diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9c0f6a9755..b285fa52bc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -48,9 +48,10 @@ jobs: run: git config --global --add safe.directory "$(pwd)" - name: Run unit tests - # skip the TestResolverLocalManifest test. It is tested separately + # skip the TestResolverLocalManifest and + # TestBlockingResolverLocalManifest tests. They are tested separately # (see below: requires root) - run: go test -race ./... -test.skip TestResolverLocalManifest + run: go test -race ./... -test.skip 'TestBlockingResolverLocalManifest|TestResolverLocalManifest' - name: Run depsolver tests with force-dnf to make sure it's not skipped run: go test -race ./pkg/dnfjson/... -force-dnf diff --git a/pkg/container/blocking_resolver_test.go b/pkg/container/blocking_resolver_test.go new file mode 100644 index 0000000000..fe907f05fe --- /dev/null +++ b/pkg/container/blocking_resolver_test.go @@ -0,0 +1,200 @@ +package container_test + +import ( + "fmt" + "os" + "os/exec" + "os/user" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/osbuild/images/internal/common" + "github.com/osbuild/images/internal/testregistry" + "github.com/osbuild/images/pkg/arch" + "github.com/osbuild/images/pkg/container" +) + +func TestBlockingResolver(t *testing.T) { + registry := testregistry.New() + defer registry.Close() + repo := registry.AddRepo("library/osbuild") + ref := registry.GetRef("library/osbuild") + + refs := make([]string, 10) + for i := 0; i < len(refs); i++ { + checksum := repo.AddImage( + []testregistry.Blob{testregistry.NewDataBlobFromBase64(testregistry.RootLayer)}, + []string{"amd64", "ppc64le"}, + fmt.Sprintf("image %d", i), + time.Time{}) + + tag := fmt.Sprintf("%d", i) + repo.AddTag(checksum, tag) + refs[i] = fmt.Sprintf("%s:%s", ref, tag) + } + + resolver := container.NewBlockingResolver("amd64") + + for _, r := range refs { + resolver.Add(container.SourceSpec{ + Source: r, + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: false, + }) + } + + have, err := resolver.Finish() + assert.NoError(t, err) + assert.NotNil(t, have) + + assert.Len(t, have, len(refs)) + + want := make([]container.Spec, len(refs)) + for i, r := range refs { + spec, err := registry.Resolve(r, arch.ARCH_X86_64) + assert.NoError(t, err) + want[i] = spec + } + + assert.ElementsMatch(t, have, want) +} + +func TestBlockingResolverFail(t *testing.T) { + resolver := container.NewBlockingResolver("amd64") + + resolver.Add(container.SourceSpec{ + Source: "invalid-reference@${IMAGE_DIGEST}", + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: false, + }) + specs, err := resolver.Finish() + assert.Error(t, err) + assert.Len(t, specs, 0) + + registry := testregistry.New() + defer registry.Close() + + resolver.Add(container.SourceSpec{ + Source: registry.GetRef("repo"), + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: false, + }) + specs, err = resolver.Finish() + assert.Error(t, err) + assert.Len(t, specs, 0) + + resolver.Add(container.SourceSpec{ + Source: registry.GetRef("repo"), + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: false, + }) + specs, err = resolver.Finish() + assert.Error(t, err) + assert.Len(t, specs, 0) + + resolver.Add(container.SourceSpec{ + Source: registry.GetRef("repo"), + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: false, + }) + specs, err = resolver.Finish() + assert.Error(t, err) + assert.Len(t, specs, 0) +} + +func TestBlockingResolverLocalManifest(t *testing.T) { + currentUser, err := user.Current() + assert.NoError(t, err) + + if !*forceLocal { + // local resolver tests aren't forced, so we can skip + // them if the user is not root or the podman executable + // is not installed + if currentUser.Uid != "0" { + t.Skip("User is not root, skipping test") + } + + _, err = exec.LookPath("podman") + if err != nil { + t.Skip("Podman not available, skipping test") + } + } + + containerFile, err := os.CreateTemp(t.TempDir(), "Containerfile") + assert.NoError(t, err) + + tmpStorage := t.TempDir() + + _, err = containerFile.Write([]byte("FROM scratch")) + assert.NoError(t, err) + + cmd := exec.Command( //nolint:gosec + "podman", + "--root", tmpStorage, // don't dirty the default store + "build", + "--platform", "linux/amd64,linux/arm64", + "--manifest", "multi-arch", + "-f", containerFile.Name(), + ".", + ) + // cleanup the containers + defer func() { + cmd := exec.Command("podman", "--root", tmpStorage, "system", "prune", "-f") + err := cmd.Run() + assert.NoError(t, err) + }() + + cmd.Stderr = os.Stderr + cmd.Stdout = os.Stdout + + err = cmd.Run() + assert.NoError(t, err) + + // try resolve an x86_64 container using a local manifest list + resolver := container.NewBlockingResolverWithTestClient("amd64", func(target string) (*container.Client, error) { + return container.NewClientWithTestStorage(target, tmpStorage) + }) + + resolver.Add(container.SourceSpec{ + Source: "localhost/multi-arch", + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: true, + }) + specs, err := resolver.Finish() + assert.NoError(t, err) + assert.Len(t, specs, 1) + assert.Equal(t, specs[0].LocalName, "localhost/multi-arch:latest") + assert.Equal(t, specs[0].Arch.String(), arch.ARCH_X86_64.String()) + + // try resolve an aarch64 container using a local manifest list + resolver = container.NewBlockingResolverWithTestClient("arm64", func(target string) (*container.Client, error) { + return container.NewClientWithTestStorage(target, tmpStorage) + }) + + resolver.Add(container.SourceSpec{ + Source: "localhost/multi-arch", + Name: "", + Digest: common.ToPtr(""), + TLSVerify: common.ToPtr(false), + Local: true, + }) + specs, err = resolver.Finish() + assert.NoError(t, err) + assert.Len(t, specs, 1) + assert.Equal(t, specs[0].LocalName, "localhost/multi-arch:latest") + assert.Equal(t, specs[0].Arch.String(), arch.ARCH_AARCH64.String()) +} diff --git a/pkg/container/export_test.go b/pkg/container/export_test.go index 43af1f836c..1fb1a8b36d 100644 --- a/pkg/container/export_test.go +++ b/pkg/container/export_test.go @@ -1,6 +1,6 @@ package container -func NewResolverWithTestClient(arch string, f func(string) (*Client, error)) *Resolver { +func NewResolverWithTestClient(arch string, f func(string) (*Client, error)) *asyncResolver { resolver := NewResolver(arch) resolver.newClient = f return resolver @@ -11,3 +11,9 @@ func NewClientWithTestStorage(target, storage string) (*Client, error) { client.store = storage return client, err } + +func NewBlockingResolverWithTestClient(arch string, f func(string) (*Client, error)) Resolver { + resolver := NewBlockingResolver(arch) + resolver.(*blockingResolver).newClient = f + return resolver +} diff --git a/pkg/container/resolver.go b/pkg/container/resolver.go index 054cd8bd9a..04d8d5973e 100644 --- a/pkg/container/resolver.go +++ b/pkg/container/resolver.go @@ -12,7 +12,12 @@ type resolveResult struct { err error } -type Resolver struct { +type Resolver interface { + Add(spec SourceSpec) + Finish() ([]Spec, error) +} + +type asyncResolver struct { jobs int queue chan resolveResult @@ -33,8 +38,10 @@ type SourceSpec struct { } // XXX: use arch.Arch here? -func NewResolver(arch string) *Resolver { - return &Resolver{ +func NewResolver(arch string) *asyncResolver { + // NOTE: this should return the Resolver interface, but osbuild-composer + // sets the AuthFilePath and for now we don't want to break the API. + return &asyncResolver{ ctx: context.Background(), queue: make(chan resolveResult, 2), Arch: arch, @@ -43,7 +50,7 @@ func NewResolver(arch string) *Resolver { } } -func (r *Resolver) Add(spec SourceSpec) { +func (r *asyncResolver) Add(spec SourceSpec) { client, err := r.newClient(spec.Source) r.jobs += 1 @@ -67,7 +74,7 @@ func (r *Resolver) Add(spec SourceSpec) { }() } -func (r *Resolver) Finish() ([]Spec, error) { +func (r *asyncResolver) Finish() ([]Spec, error) { specs := make([]Spec, 0, r.jobs) errs := make([]string, 0, r.jobs) @@ -92,3 +99,64 @@ func (r *Resolver) Finish() ([]Spec, error) { return specs, nil } + +type blockingResolver struct { + Arch string + AuthFilePath string + + newClient func(string) (*Client, error) + + results []resolveResult +} + +// NewBlockingResolver returns a [asyncResolver] that resolves container refs +// synchronously (blocking). +// TODO: Make this the only resolver after all clients have migrated to this. +func NewBlockingResolver(arch string) Resolver { + return &blockingResolver{ + Arch: arch, + newClient: NewClient, + } +} + +func (r *blockingResolver) Add(src SourceSpec) { + client, err := r.newClient(src.Source) + if err != nil { + r.results = append(r.results, resolveResult{err: err}) + return + } + + client.SetTLSVerify(src.TLSVerify) + client.SetArchitectureChoice(r.Arch) + if r.AuthFilePath != "" { + client.SetAuthFilePath(r.AuthFilePath) + } + + spec, err := client.Resolve(context.TODO(), src.Name, src.Local) + if err != nil { + err = fmt.Errorf("'%s': %w", src.Source, err) + } + r.results = append(r.results, resolveResult{spec: spec, err: err}) +} + +func (r *blockingResolver) Finish() ([]Spec, error) { + specs := make([]Spec, 0, len(r.results)) + errs := make([]string, 0, len(r.results)) + for _, result := range r.results { + if result.err == nil { + specs = append(specs, result.spec) + } else { + errs = append(errs, result.err.Error()) + } + } + + if len(errs) > 0 { + detail := strings.Join(errs, "; ") + return specs, fmt.Errorf("failed to resolve container: %s", detail) + } + + // Return a stable result, sorted by Digest + sort.Slice(specs, func(i, j int) bool { return specs[i].Digest < specs[j].Digest }) + + return specs, nil +} diff --git a/pkg/manifestgen/manifestgen.go b/pkg/manifestgen/manifestgen.go index 865eb73230..8029fea59b 100644 --- a/pkg/manifestgen/manifestgen.go +++ b/pkg/manifestgen/manifestgen.go @@ -223,7 +223,7 @@ func DefaultDepsolver(cacheDir string, packageSets map[string][]rpmmd.PackageSet } func resolveContainers(containers []container.SourceSpec, archName string) ([]container.Spec, error) { - resolver := container.NewResolver(archName) + resolver := container.NewBlockingResolver(archName) for _, c := range containers { resolver.Add(c)