Add a blocking container resolver

.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

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())
+}

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
+}

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
+}

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)