From d45b6da444f211f49e25d25d26cc0241023d22c8 Mon Sep 17 00:00:00 2001 From: Jon Johnson Date: Wed, 11 Dec 2024 14:02:06 -0800 Subject: [PATCH] Make LockImageConfiguration multi-arch aware (#1432) Previously this would just return the intersection of archs with warnings for any issues (or an error for fatal issues), but that only partially locked the architectures because anything not in the intersection would fail. Instead we'll return a map of arch to config. This is a breaking change but it's used in exactly one place and I'll fix that when I bump this. Signed-off-by: Jon Johnson --- pkg/build/lock.go | 63 +++++++++++++++--- pkg/build/lock_test.go | 146 +++++++++++++++++++++++++++++++---------- 2 files changed, 166 insertions(+), 43 deletions(-) diff --git a/pkg/build/lock.go b/pkg/build/lock.go index 249937329..5619bde23 100644 --- a/pkg/build/lock.go +++ b/pkg/build/lock.go @@ -17,6 +17,7 @@ package build import ( "context" "fmt" + "maps" "reflect" "regexp" "sort" @@ -27,13 +28,19 @@ import ( "chainguard.dev/apko/pkg/build/types" ) -func LockImageConfiguration(ctx context.Context, ic types.ImageConfiguration, opts ...Option) (*types.ImageConfiguration, map[string][]string, error) { +// LockImageConfiguration returns a map of locked image configurations for each architecture, +// plus an entry for the "index" pseudo-architecture that represents the intersection of packages +// across all the real architecture configs. It also returns a map of missing packages for each +// architecture that could not be locked. Using the "index" architecture is equivalent to what +// this used to return prior to supporting per-arch locked configs. +func LockImageConfiguration(ctx context.Context, ic types.ImageConfiguration, opts ...Option) (map[string]*types.ImageConfiguration, map[string][]string, error) { mc, err := NewMultiArch(ctx, ic.Archs, append(opts, WithImageConfiguration(ic))...) if err != nil { return nil, nil, err } archs := make([]resolved, 0, len(ic.Archs)) + ics := make(map[string]*types.ImageConfiguration, len(ic.Archs)+1) // Determine the exact versions of our transitive packages and lock them // down in the "resolved" configuration, so that this build may be @@ -72,14 +79,30 @@ func LockImageConfiguration(ctx context.Context, ic types.ImageConfiguration, op archs = append(archs, r) } - l, missing, err := unify(ic.Contents.Packages, archs) + pls, missing, err := unify(ic.Contents.Packages, archs) if err != nil { return nil, missing, err } - // Set the locked package list. - ic.Contents.Packages = l - return &ic, missing, nil + // Set the locked package lists. + for arch, pl := range pls { + // Create a defensive copy of "ic". + copied := types.ImageConfiguration{} + if err := ic.MergeInto(&copied); err != nil { + return nil, nil, err + } + + copied.Contents.Packages = pl + + if arch != "index" { + // Overwrite single-arch configs with their specific arch. + copied.Archs = []types.Architecture{types.ParseArchitecture(arch)} + } + + ics[arch] = &copied + } + + return ics, missing, nil } type resolved struct { @@ -89,14 +112,22 @@ type resolved struct { provided map[string]sets.Set[string] } -func unify(originals []string, inputs []resolved) ([]string, map[string][]string, error) { +// unify returns (locked packages (per arch), missing packages (per arch), error) +func unify(originals []string, inputs []resolved) (map[string][]string, map[string][]string, error) { if len(originals) == 0 { - return nil, nil, nil + // If there are no original packages, then we can't really do anything. + // This used to return nil but multi-arch unification assumes we always + // have an "index" entry, even if it's empty, so we return this now. + // Mostly this is to satisfy some tests that have no package inputs. + return map[string][]string{"index": {}}, nil, nil } originalPackages := resolved{ packages: make(sets.Set[string], len(originals)), versions: make(map[string]string, len(originals)), } + + byArch := map[string][]string{} + for _, orig := range originals { name := orig // The function we want from go-apk is private, but these are all the @@ -114,7 +145,7 @@ func unify(originals []string, inputs []resolved) ([]string, map[string][]string // architectures. acc := resolved{ packages: inputs[0].packages.Clone(), - versions: inputs[0].versions, + versions: maps.Clone(inputs[0].versions), provided: inputs[0].provided, } for _, next := range inputs[1:] { @@ -207,6 +238,18 @@ func unify(originals []string, inputs []resolved) ([]string, map[string][]string pl = append(pl, fmt.Sprintf("%s=%s", pkg, acc.versions[pkg])) } + // "index" is a sentinel value for the intersectino of all architectures. + // This is a reference to the OCI image index we'll be producing with it. + byArch["index"] = pl + + for _, input := range inputs { + pl := make([]string, 0, len(input.packages)) + for _, pkg := range sets.List(input.packages) { + pl = append(pl, fmt.Sprintf("%s=%s", pkg, input.versions[pkg])) + } + byArch[input.arch] = pl + } + // If a particular architecture is missing additional packages from the // locked set that it produced, than warn about those as well. missingByArch := make(map[string][]string, len(inputs)) @@ -217,10 +260,10 @@ func unify(originals []string, inputs []resolved) ([]string, map[string][]string } } if len(missingByArch) > 0 { - return pl, missingByArch, nil + return byArch, missingByArch, nil } - return pl, nil, nil + return byArch, nil, nil } // Copied from go-apk's version.go diff --git a/pkg/build/lock_test.go b/pkg/build/lock_test.go index 5608948ed..b7092b812 100644 --- a/pkg/build/lock_test.go +++ b/pkg/build/lock_test.go @@ -27,15 +27,17 @@ func TestUnify(t *testing.T) { name string originals []string inputs []resolved - want []string + want map[string][]string wantMissing map[string][]string wantDiag error }{{ name: "empty", + want: map[string][]string{"index": {}}, }, { name: "simple single arch", originals: []string{"foo", "bar", "baz"}, inputs: []resolved{{ + arch: "amd64", packages: sets.New("foo", "bar", "baz"), versions: map[string]string{ "foo": "1.2.3", @@ -43,15 +45,23 @@ func TestUnify(t *testing.T) { "baz": "0.0.1", }, }}, - want: []string{ - "bar=2.4.6", - "baz=0.0.1", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, + "index": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, }, }, { name: "locked versions", originals: []string{"foo=1.2.3", "bar=2.4.6", "baz=0.0.1"}, inputs: []resolved{{ + arch: "amd64", packages: sets.New("foo", "bar", "baz"), versions: map[string]string{ "foo": "1.2.3", @@ -59,15 +69,23 @@ func TestUnify(t *testing.T) { "baz": "0.0.1", }, }}, - want: []string{ - "bar=2.4.6", - "baz=0.0.1", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, + "index": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, }, }, { name: "transitive dependency", originals: []string{"foo", "bar", "baz"}, inputs: []resolved{{ + arch: "amd64", packages: sets.New("foo", "bar", "baz", "bonus"), versions: map[string]string{ "foo": "1.2.3", @@ -76,11 +94,19 @@ func TestUnify(t *testing.T) { "bonus": "5.4.3", }, }}, - want: []string{ - "bar=2.4.6", - "baz=0.0.1", - "bonus=5.4.3", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, + "index": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, }, }, { name: "multiple matching architectures", @@ -112,11 +138,25 @@ func TestUnify(t *testing.T) { "bar": sets.New("def", "ogg"), }, }}, - want: []string{ - "bar=2.4.6", - "baz=0.0.1", - "bonus=5.4.3", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, + "arm64": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, + "index": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, }, }, { name: "mismatched transitive dependency", @@ -140,10 +180,24 @@ func TestUnify(t *testing.T) { "bonus": "5.4.3-r1", }, }}, - want: []string{ - "bar=2.4.6", - "baz=0.0.1", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3-r0", + "foo=1.2.3", + }, + "arm64": { + "bar=2.4.6", + "baz=0.0.1", + "bonus=5.4.3-r1", + "foo=1.2.3", + }, + "index": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, }, wantMissing: map[string][]string{ "amd64": {"bonus"}, @@ -175,10 +229,22 @@ func TestUnify(t *testing.T) { "bonus": sets.New("bar"), }, }}, - want: []string{ - "baz=0.0.1", - "bonus=5.4.3", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, + "arm64": { + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, + "index": { + "baz=0.0.1", + "bonus=5.4.3", + "foo=1.2.3", + }, }, }, { name: "mismatched direct dependency", @@ -202,7 +268,7 @@ func TestUnify(t *testing.T) { "bonus": "5.4.3", }, }}, - wantDiag: errors.New("unable to lock packages to a consistent version: map[bar:[ (amd64) 2.4.6-r1 (arm64)]]"), + wantDiag: errors.New("unable to lock packages to a consistent version: map[bar:[2.4.6-r0 (amd64) 2.4.6-r1 (arm64)]]"), }, { name: "mismatched direct dependency (with constraint)", originals: []string{"foo", "bar>2.4.6", "baz"}, @@ -231,7 +297,7 @@ func TestUnify(t *testing.T) { // "bonus=5.4.3", // "foo=1.2.3", // }, - wantDiag: errors.New("unable to lock packages to a consistent version: map[bar:[ (amd64) 2.4.6-r1 (arm64)]]"), + wantDiag: errors.New("unable to lock packages to a consistent version: map[bar:[2.4.6-r0 (amd64) 2.4.6-r1 (arm64)]]"), }, { name: "single-architecture resolved dependency", originals: []string{"foo", "bar", "baz"}, @@ -254,10 +320,24 @@ func TestUnify(t *testing.T) { "arm-energy-efficient-as-f-arithmetic": "9.8.7", }, }}, - want: []string{ - "bar=2.4.6", - "baz=0.0.1", - "foo=1.2.3", + want: map[string][]string{ + "amd64": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + "intel-fast-as-f-math=5.4.3", + }, + "arm64": { + "arm-energy-efficient-as-f-arithmetic=9.8.7", + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, + "index": { + "bar=2.4.6", + "baz=0.0.1", + "foo=1.2.3", + }, }, wantMissing: map[string][]string{ "amd64": {"intel-fast-as-f-math"},