Skip to content

Commit

Permalink
Make LockImageConfiguration multi-arch aware (#1432)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
jonjohnsonjr authored Dec 11, 2024
1 parent c269982 commit d45b6da
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 43 deletions.
63 changes: 53 additions & 10 deletions pkg/build/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package build
import (
"context"
"fmt"
"maps"
"reflect"
"regexp"
"sort"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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:] {
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand Down
146 changes: 113 additions & 33 deletions pkg/build/lock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,47 +27,65 @@ 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",
"bar": "2.4.6",
"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",
"bar": "2.4.6",
"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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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"},
Expand Down Expand Up @@ -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",
Expand All @@ -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"},
Expand Down Expand Up @@ -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"},
Expand All @@ -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"},
Expand Down

0 comments on commit d45b6da

Please sign in to comment.