Merge branch 'version/0-47-0-RC1'

pkg/buildplan/buildplan.go

@@ -37,9 +37,10 @@ func Unmarshal(data []byte) (*BuildPlan, error) {
 		return nil, errs.Wrap(err, "error hydrating build plan")
 	}
 
-	if len(b.artifacts) == 0 || len(b.ingredients) == 0 || len(b.platforms) == 0 {
-		return nil, errs.New("Buildplan unmarshalling failed as it got zero artifacts (%d), ingredients (%d) and or platforms (%d).",
-			len(b.artifacts), len(b.ingredients), len(b.platforms))
+	if len(b.artifacts) == 0 || len(b.platforms) == 0 {
+		// Ingredients are not considered here because certain builds (eg. camel) won't be able to relate to a single ingredient
+		return nil, errs.New("Buildplan unmarshalling failed as it got zero artifacts (%d) and/or platforms (%d).",
+			len(b.artifacts), len(b.platforms))
 	}
 
 	return b, nil

pkg/buildplan/filters.go

@@ -47,7 +47,7 @@ func FilterStateArtifacts() FilterArtifact {
 		internalIngredients := sliceutils.Filter(a.Ingredients, func(i *Ingredient) bool {
 			return i.Namespace == NamespaceInternal
 		})
-		if len(a.Ingredients) == len(internalIngredients) {
+		if len(a.Ingredients) > 0 && len(a.Ingredients) == len(internalIngredients) {
 			return false
 		}
 		if strings.Contains(a.URL, "as-builds/noop") {

pkg/buildplan/hydrate.go

@@ -55,7 +55,9 @@ func (b *BuildPlan) hydrate() error {
 		}
 		ingredient, ok := ingredientLookup[source.IngredientID]
 		if !ok {
-			return errs.New("missing ingredient for source ID: %s", req.Source)
+			// It's possible that we haven't associated a source to an artifact if that source links to multiple artifacts.
+			// In this case we cannot determine which artifact relates to which source.
+			continue
 		}
 		b.requirements = append(b.requirements, &Requirement{
 			Requirement: req.Requirement,
@@ -80,7 +82,7 @@ func (b *BuildPlan) hydrate() error {
 }
 
 func (b *BuildPlan) hydrateWithBuildClosure(nodeIDs []strfmt.UUID, platformID *strfmt.UUID, artifactLookup map[strfmt.UUID]*Artifact) error {
-	err := b.raw.WalkViaSteps(nodeIDs, raw.TagDependency, func(node interface{}, parent *raw.Artifact) error {
+	err := b.raw.WalkViaSteps(nodeIDs, raw.WalkViaDeps, func(node interface{}, parent *raw.Artifact) error {
 		switch v := node.(type) {
 		case *raw.Artifact:
 			// logging.Debug("Walking build closure artifact '%s (%s)'", v.DisplayName, v.NodeID)
@@ -151,51 +153,44 @@ func (b *BuildPlan) hydrateWithRuntimeClosure(nodeIDs []strfmt.UUID, platformID
 }
 
 func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfmt.UUID, ingredientLookup map[strfmt.UUID]*Ingredient) error {
-	err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.TagSource,
+	err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.WalkViaSingleSource,
 		func(node interface{}, parent *raw.Artifact) error {
-			switch v := node.(type) {
-			case *raw.Source:
-				// logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID)
-
-				// Ingredients aren't explicitly represented in buildplans. Technically all sources are ingredients
-				// but this may not always be true in the future. For our purposes we will initialize our own ingredients
-				// based on the source information, but we do not want to make the assumption in our logic that all
-				// sources are ingredients.
-				ingredient, ok := ingredientLookup[v.IngredientID]
-				if !ok {
-					ingredient = &Ingredient{
-						IngredientSource: &v.IngredientSource,
-						platforms:        []strfmt.UUID{},
-						Artifacts:        []*Artifact{},
-					}
-					b.ingredients = append(b.ingredients, ingredient)
-					ingredientLookup[v.IngredientID] = ingredient
-				}
+			// logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID)
+			v, ok := node.(*raw.Source)
+			if !ok {
+				return nil // continue
+			}
 
-				// With multiple terminals it's possible we encounter the same combination multiple times.
-				// And an artifact usually only has one ingredient, so this is the cheapest lookup.
-				if !sliceutils.Contains(artifact.Ingredients, ingredient) {
-					artifact.Ingredients = append(artifact.Ingredients, ingredient)
-					ingredient.Artifacts = append(ingredient.Artifacts, artifact)
-				}
-				if platformID != nil {
-					ingredient.platforms = append(ingredient.platforms, *platformID)
+			// Ingredients aren't explicitly represented in buildplans. Technically all sources are ingredients
+			// but this may not always be true in the future. For our purposes we will initialize our own ingredients
+			// based on the source information, but we do not want to make the assumption in our logic that all
+			// sources are ingredients.
+			ingredient, ok := ingredientLookup[v.IngredientID]
+			if !ok {
+				ingredient = &Ingredient{
+					IngredientSource: &v.IngredientSource,
+					platforms:        []strfmt.UUID{},
+					Artifacts:        []*Artifact{},
 				}
+				b.ingredients = append(b.ingredients, ingredient)
+				ingredientLookup[v.IngredientID] = ingredient
+			}
 
-				if artifact.IsBuildtimeDependency {
-					ingredient.IsBuildtimeDependency = true
-				}
-				if artifact.IsRuntimeDependency {
-					ingredient.IsRuntimeDependency = true
-				}
+			// With multiple terminals it's possible we encounter the same combination multiple times.
+			// And an artifact usually only has one ingredient, so this is the cheapest lookup.
+			if !sliceutils.Contains(artifact.Ingredients, ingredient) {
+				artifact.Ingredients = append(artifact.Ingredients, ingredient)
+				ingredient.Artifacts = append(ingredient.Artifacts, artifact)
+			}
+			if platformID != nil {
+				ingredient.platforms = append(ingredient.platforms, *platformID)
+			}
 
-				return nil
-			default:
-				if a, ok := v.(*raw.Artifact); ok && a.NodeID == artifact.ArtifactID {
-					return nil // continue
-				}
-				// Source ingredients are only relevant when they link DIRECTLY to the artifact
-				return raw.WalkInterrupt{}
+			if artifact.IsBuildtimeDependency {
+				ingredient.IsBuildtimeDependency = true
+			}
+			if artifact.IsRuntimeDependency {
+				ingredient.IsRuntimeDependency = true
 			}
 
 			return nil
@@ -211,14 +206,6 @@ func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfm
 // If there are duplicates we're likely to see failures down the chain if live, though that's by no means guaranteed.
 // Surfacing it here will make it easier to reason about the failure.
 func (b *BuildPlan) sanityCheck() error {
-	// Ensure all artifacts have an associated ingredient
-	// If this fails either the API is bugged or the hydrate logic is bugged
-	for _, a := range b.Artifacts() {
-		if raw.IsStateToolMimeType(a.MimeType) && len(a.Ingredients) == 0 {
-			return errs.New("artifact '%s (%s)' does not have an ingredient", a.ArtifactID, a.DisplayName)
-		}
-	}
-
 	// The remainder of sanity checks aren't checking for error conditions so much as they are checking for smoking guns
 	// If these fail then it's likely the API has changed in a backward incompatible way, or we broke something.
 	// In any case it does not necessarily mean runtime sourcing is broken.

pkg/buildplan/hydrate_test.go

@@ -18,27 +18,40 @@ func TestBuildPlan_hydrateWithIngredients(t *testing.T) {
 	}{
 		{
 			"Ingredient solves for simple artifact > src hop",
-			&BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc},
+			&BuildPlan{raw: mock.BuildWithInstallerDepsViaSrc},
 			&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000007"},
 			"00000000-0000-0000-0000-000000000009",
 		},
 		{
 			"Installer should not resolve to an ingredient as it doesn't have a direct source",
-			&BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc},
+			&BuildPlan{raw: mock.BuildWithInstallerDepsViaSrc},
 			&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"},
 			"",
 		},
+		{
+			"State artifact should resolve to source even when hopping through a python wheel",
+			&BuildPlan{raw: mock.BuildWithStateArtifactThroughPyWheel},
+			&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"},
+			"00000000-0000-0000-0000-000000000009",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			b := tt.buildplan
 			if err := b.hydrateWithIngredients(tt.inputArtifact, nil, map[strfmt.UUID]*Ingredient{}); err != nil {
 				t.Fatalf("hydrateWithIngredients() error = %v", errs.JoinMessage(err))
 			}
+
+			// Use string slice so testify doesn't just dump a bunch of pointer addresses on failure -.-
+			ingredients := []string{}
+			for _, i := range tt.inputArtifact.Ingredients {
+				ingredients = append(ingredients, i.IngredientID.String())
+			}
 			if tt.wantIngredient == "" {
-				require.Empty(t, tt.inputArtifact.Ingredients)
+				require.Empty(t, ingredients)
 				return
 			}
+
 			if len(tt.inputArtifact.Ingredients) != 1 {
 				t.Fatalf("expected 1 ingredient resolution, got %d", len(tt.inputArtifact.Ingredients))
 			}

pkg/buildplan/mock/mock.go

@@ -153,7 +153,8 @@ var BuildWithRuntimeDeps = &raw.Build{
 	},
 }
 
-var BuildWithRuntimeDepsViaSrc = &raw.Build{
+// BuildWithInstallerDepsViaSrc is a build that includes an installer which has two artifacts as its dependencies.
+var BuildWithInstallerDepsViaSrc = &raw.Build{
 	Terminals: []*raw.NamedTarget{
 		{
 			Tag:     "platform:00000000-0000-0000-0000-000000000001",
@@ -165,7 +166,12 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{
 			StepID:  "00000000-0000-0000-0000-000000000003",
 			Outputs: []string{"00000000-0000-0000-0000-000000000002"},
 			Inputs: []*raw.NamedTarget{
-				{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000007"}},
+				{
+					Tag: "src", NodeIDs: []strfmt.UUID{
+						"00000000-0000-0000-0000-000000000007",
+						"00000000-0000-0000-0000-000000000010",
+					},
+				},
 			},
 		},
 		{
@@ -175,6 +181,13 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{
 				{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000009"}},
 			},
 		},
+		{
+			StepID:  "00000000-0000-0000-0000-000000000011",
+			Outputs: []string{"00000000-0000-0000-0000-000000000010"},
+			Inputs: []*raw.NamedTarget{
+				{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000012"}},
+			},
+		},
 	},
 	Artifacts: []*raw.Artifact{
 		{
@@ -190,6 +203,66 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{
 			MimeType:    types.XActiveStateArtifactMimeType,
 			GeneratedBy: "00000000-0000-0000-0000-000000000008",
 		},
+		{
+			NodeID:      "00000000-0000-0000-0000-000000000010",
+			DisplayName: "pkgTwo",
+			MimeType:    types.XActiveStateArtifactMimeType,
+			GeneratedBy: "00000000-0000-0000-0000-000000000011",
+		},
+	},
+	Sources: []*raw.Source{
+		{
+			"00000000-0000-0000-0000-000000000009",
+			raw.IngredientSource{
+				IngredientID: "00000000-0000-0000-0000-000000000009",
+			},
+		},
+		{
+			"00000000-0000-0000-0000-000000000012",
+			raw.IngredientSource{
+				IngredientID: "00000000-0000-0000-0000-000000000012",
+			},
+		},
+	},
+}
+
+// BuildWithStateArtifactThroughPyWheel is a build with a state tool artifact that has a python wheel as its dependency
+var BuildWithStateArtifactThroughPyWheel = &raw.Build{
+	Terminals: []*raw.NamedTarget{
+		{
+			Tag:     "platform:00000000-0000-0000-0000-000000000001",
+			NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000002"},
+		},
+	},
+	Steps: []*raw.Step{
+		{
+			StepID:  "00000000-0000-0000-0000-000000000003",
+			Outputs: []string{"00000000-0000-0000-0000-000000000002"},
+			Inputs: []*raw.NamedTarget{
+				{
+					Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000007"},
+				},
+			},
+		},
+		{
+			StepID:  "00000000-0000-0000-0000-000000000008",
+			Outputs: []string{"00000000-0000-0000-0000-000000000007"},
+			Inputs: []*raw.NamedTarget{
+				{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000009"}},
+			},
+		},
+	},
+	Artifacts: []*raw.Artifact{
+		{
+			NodeID:      "00000000-0000-0000-0000-000000000002",
+			DisplayName: "pkgStateArtifact",
+			GeneratedBy: "00000000-0000-0000-0000-000000000003",
+		},
+		{
+			NodeID:      "00000000-0000-0000-0000-000000000007",
+			DisplayName: "pkgPyWheel",
+			GeneratedBy: "00000000-0000-0000-0000-000000000008",
+		},
 	},
 	Sources: []*raw.Source{
 		{

pkg/buildplan/raw/walk.go

@@ -1,8 +1,6 @@
 package raw
 
 import (
-	"errors"
-
 	"github.com/go-openapi/strfmt"
 
 	"github.com/ActiveState/cli/internal/errs"
@@ -11,45 +9,49 @@ import (
 
 type walkFunc func(node interface{}, parent *Artifact) error
 
-type WalkNodeContext struct {
-	Node           interface{}
-	ParentArtifact *Artifact
-	tag            StepInputTag
-	lookup         map[strfmt.UUID]interface{}
+type WalkStrategy struct {
+	tag               StepInputTag
+	stopAtMultiSource bool // If true, we will stop walking if the artifact relates to multiple sources (eg. installer, docker img)
 }
 
-type WalkInterrupt struct{}
-
-func (w WalkInterrupt) Error() string {
-	return "interrupt walk"
-}
+var (
+	WalkViaSingleSource = WalkStrategy{
+		TagSource,
+		true,
+	}
+	WalkViaMultiSource = WalkStrategy{
+		TagSource,
+		false,
+	}
+	WalkViaDeps = WalkStrategy{
+		TagDependency,
+		false,
+	}
+)
 
 // WalkViaSteps walks the graph and reports on nodes it encounters
 // Note that the same node can be encountered multiple times if it is referenced in the graph multiple times.
 // In this case the context around the node may be different, even if the node itself isn't.
-func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, inputTag StepInputTag, walk walkFunc) error {
+func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, strategy WalkStrategy, walk walkFunc) error {
 	lookup := b.LookupMap()
 
 	for _, nodeID := range nodeIDs {
 		node, ok := lookup[nodeID]
 		if !ok {
 			return errs.New("node ID '%s' does not exist in lookup table", nodeID)
 		}
-		if err := b.walkNodeViaSteps(node, nil, inputTag, walk); err != nil {
+		if err := b.walkNodeViaSteps(node, nil, strategy, walk); err != nil {
 			return errs.Wrap(err, "could not recurse over node IDs")
 		}
 	}
 
 	return nil
 }
 
-func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, tag StepInputTag, walk walkFunc) error {
+func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, strategy WalkStrategy, walk walkFunc) error {
 	lookup := b.LookupMap()
 
 	if err := walk(node, parent); err != nil {
-		if errors.As(err, &WalkInterrupt{}) {
-			return nil
-		}
 		return errs.Wrap(err, "error walking over node")
 	}
 
@@ -74,24 +76,29 @@ func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, tag StepInp
 	// tool, but it's technically possible to happen if someone requested a builder as part of their order.
 	_, isSource = generatedByNode.(*Source)
 	if isSource {
-		if err := b.walkNodeViaSteps(generatedByNode, ar, tag, walk); err != nil {
+		if err := b.walkNodeViaSteps(generatedByNode, ar, strategy, walk); err != nil {
 			return errs.Wrap(err, "error walking source from generatedBy")
 		}
 		return nil // Sources are at the end of the recursion.
 	}
 
-	nodeIDs, err := b.inputNodeIDsFromStep(ar, tag)
+	nodeIDs, err := b.inputNodeIDsFromStep(ar, strategy.tag)
 	if err != nil {
 		return errs.Wrap(err, "error walking over step inputs")
 	}
 
+	// Stop if the next step has multiple input node ID's; this means we cannot determine a single source
+	if strategy.stopAtMultiSource && len(nodeIDs) > 1 {
+		return nil
+	}
+
 	for _, id := range nodeIDs {
 		// Grab subnode that we want to iterate over next
 		subNode, ok := lookup[id]
 		if !ok {
 			return errs.New("node ID '%s' does not exist in lookup table", id)
 		}
-		if err := b.walkNodeViaSteps(subNode, ar, tag, walk); err != nil {
+		if err := b.walkNodeViaSteps(subNode, ar, strategy, walk); err != nil {
 			return errs.Wrap(err, "error iterating over %s", id)
 		}
 	}