Skip to content

Commit

Permalink
Merge branch 'version/0-47-0-RC1'
Browse files Browse the repository at this point in the history
  • Loading branch information
Naatan committed Sep 26, 2024
2 parents 596b29e + f423ba8 commit bc16a0b
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 87 deletions.
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

0 comments on commit bc16a0b

Please sign in to comment.