Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map to ingredient when its source can be identified as a single straight path #3508

Merged
merged 9 commits into from
Sep 26, 2024
7 changes: 4 additions & 3 deletions pkg/buildplan/buildplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/buildplan/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
87 changes: 37 additions & 50 deletions pkg/buildplan/hydrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
19 changes: 16 additions & 3 deletions pkg/buildplan/hydrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
77 changes: 75 additions & 2 deletions pkg/buildplan/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
},
},
},
},
{
Expand All @@ -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{
{
Expand All @@ -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{
{
Expand Down
49 changes: 28 additions & 21 deletions pkg/buildplan/raw/walk.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package raw

import (
"errors"

"github.com/go-openapi/strfmt"

"github.com/ActiveState/cli/internal/errs"
Expand All @@ -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")
}

Expand All @@ -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)
}
}
Expand Down
Loading
Loading