Pass docker auth when retrieving bundle metadata

build/azure-pipelines.integration.yml

@@ -11,8 +11,20 @@ pr:
 pool:
   vmImage: "ubuntu-latest"
 
-variables:
+variables: # these are really constants
   GOVERSION: "1.18"
+  # Cache go modules and the results of go build/test
+  # Increment the version number prefix of the key and restoreKey to clear the cache
+  GOCACHE: $(Pipeline.Workspace)/.cache/go-build/
+  GOCACHE_KEY: 'v3 | go-build | "$(Agent.OS)" | go.sum'
+  GOCACHE_RESTOREKEYS: |
+    v3 | go-build | "$(Agent.OS)"
+    v3 | go-build | "$(Agent.OS)" | go.sum
+  GOMODCACHE: /home/vsts/go/pkg/mod
+  GOMODCACHE_KEY: 'v4 | go-pkg | "$(Agent.OS)" | go.sum'
+  GOMODCACHE_RESTOREKEYS: |
+    v4 | go-pkg | "$(Agent.OS)"
+    v4 | go-pkg | "$(Agent.OS)" | go.sum
 
 stages:
 - stage: Setup
@@ -31,10 +43,29 @@ stages:
   - job: integration_test
     displayName: "Integration Test"
     steps:
+      # We log in here because TestRegistry integration test needs a valid docker session
+      # This unfortunately means that this pipeline must be manually triggered for non-maintainers
+      - task: Docker@2
+        displayName: Docker Login
+        inputs:
+          command: login
+          containerRegistry: ghcr.io/getporter
       - task: GoTool@0
         displayName: "Set Go Version"
         inputs:
-          version: "$(GOVERSION)"
+          version: $(GOVERSION)
+      - task: Cache@2
+        displayName: Cache Go Packages
+        inputs:
+          key: "$(GOMODCACHE_KEY)"
+          restoreKeys: $(GOMODCACHE_RESTOREKEYS)
+          path: $(GOMODCACHE)
+      - task: Cache@2
+        displayName: Cache Go Build
+        inputs:
+          key: "$(GOCACHE_KEY)"
+          restoreKeys: $(GOCACHE_RESTOREKEYS)
+          path: $(GOCACHE)
       - script: go run mage.go ConfigureAgent
         displayName: "Configure Agent"
       - bash: mage build

pkg/cnab/cnab-to-oci/provider.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"get.porter.sh/porter/pkg/cnab"
+	"github.com/google/go-containerregistry/pkg/crane"
 	"github.com/opencontainers/go-digest"
 )
 
@@ -40,3 +41,12 @@ type RegistryOptions struct {
 	// InsecureRegistry allows connecting to an unsecured registry or one without verifiable certificates.
 	InsecureRegistry bool
 }
+
+func (o RegistryOptions) toCraneOptions() []crane.Option {
+	var result []crane.Option
+	if o.InsecureRegistry {
+		transport := GetInsecureRegistryTransport()
+		result = []crane.Option{crane.Insecure, crane.WithTransport(transport)}
+	}
+	return result
+}

pkg/cnab/cnab-to-oci/registry.go

@@ -20,8 +20,6 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/google/go-containerregistry/pkg/crane"
-	"github.com/google/go-containerregistry/pkg/name"
-	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 	"github.com/moby/term"
 	"github.com/opencontainers/go-digest"
@@ -299,15 +297,12 @@ func (r *Registry) ListTags(ctx context.Context, ref cnab.OCIReference, opts Reg
 	ctx, span := tracing.StartSpan(ctx, attribute.String("repository", repository))
 	defer span.EndSpan()
 
-	var listOpts []crane.Option
-	if opts.InsecureRegistry {
-		transport := GetInsecureRegistryTransport()
-		listOpts = append(listOpts, crane.WithTransport(transport))
-	}
-
-	tags, err := crane.ListTags(repository, listOpts...)
+	tags, err := crane.ListTags(repository, opts.toCraneOptions()...)
 	if err != nil {
-		return nil, span.Errorf("error listing tags for %s: %w", repository, err)
+		if notFoundErr := asNotFoundError(err, ref); notFoundErr != nil {
+			return nil, span.Error(notFoundErr)
+		}
+		return nil, span.Errorf("error listing tags for %s: %w", ref.String(), err)
 	}
 
 	return tags, nil
@@ -320,43 +315,34 @@ func (r *Registry) GetBundleMetadata(ctx context.Context, ref cnab.OCIReference,
 	ctx, span := tracing.StartSpan(ctx, attribute.String("reference", ref.String()))
 	defer span.EndSpan()
 
-	var remoteOpts []remote.Option
-	var nameOpts []name.Option
-	if opts.InsecureRegistry {
-		transport := GetInsecureRegistryTransport()
-		remoteOpts = append(remoteOpts, remote.WithTransport(transport))
-		nameOpts = append(nameOpts, name.Insecure)
-	}
-
-	remoteRef, err := name.ParseReference(ref.String(), nameOpts...)
-	if err != nil {
-		return BundleMetadata{}, span.Errorf("error parsing the remote bundle reference %s: %w", ref, err)
-	}
-
-	bundleIndex, err := remote.Index(remoteRef, remoteOpts...)
+	bundleDigest, err := crane.Digest(ref.String(), opts.toCraneOptions()...)
 	if err != nil {
-		var httpError *transport.Error
-		if errors.As(err, &httpError) {
-			if httpError.StatusCode == http.StatusNotFound {
-				return BundleMetadata{}, span.Error(ErrNotFound{Reference: ref})
-			}
+		if notFoundErr := asNotFoundError(err, ref); notFoundErr != nil {
+			return BundleMetadata{}, span.Error(notFoundErr)
 		}
 		return BundleMetadata{}, span.Errorf("error retrieving bundle metadata for %s: %w", ref.String(), err)
 	}
 
-	bundleDigest, err := bundleIndex.Digest()
-	if err != nil {
-		return BundleMetadata{}, span.Errorf("error reading the remote bundle digest for %s: %w", ref.String(), err)
-	}
-
 	return BundleMetadata{
 		BundleReference: cnab.BundleReference{
 			Reference: ref,
-			Digest:    digest.Digest(bundleDigest.String()),
+			Digest:    digest.Digest(bundleDigest),
 		},
 	}, nil
 }
 
+// asNotFoundError checks if the error is an HTTP 404 not found error, and if so returns a corresponding ErrNotFound instance.
+func asNotFoundError(err error, ref cnab.OCIReference) error {
+	var httpError *transport.Error
+	if errors.As(err, &httpError) {
+		if httpError.StatusCode == http.StatusNotFound {
+			return ErrNotFound{Reference: ref}
+		}
+	}
+
+	return nil
+}
+
 // ImageSummary contains information about an OCI image.
 type ImageSummary struct {
 	types.ImageInspect

pkg/cnab/cnab-to-oci/registry_test.go

@@ -1,10 +1,12 @@
-package cnabtooci_test
+package cnabtooci
 
 import (
+	"net/http"
 	"testing"
 
-	cnabtooci "get.porter.sh/porter/pkg/cnab/cnab-to-oci"
+	"get.porter.sh/porter/pkg/cnab"
 	"github.com/docker/docker/api/types"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 	"github.com/stretchr/testify/require"
 )
 
@@ -59,7 +61,7 @@ func TestImageSummary(t *testing.T) {
 	for _, tt := range testcases {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			sum, err := cnabtooci.NewImageSummary(tt.imgRef, tt.imageSummary)
+			sum, err := NewImageSummary(tt.imgRef, tt.imageSummary)
 			if tt.expected.hasInitErr {
 				require.ErrorContains(t, err, tt.expectedErr)
 				return
@@ -74,3 +76,22 @@ func TestImageSummary(t *testing.T) {
 		})
 	}
 }
+
+func TestAsNotFoundError(t *testing.T) {
+	ref := cnab.MustParseOCIReference("example.com/mybuns:v1.2.3")
+	t.Run("404", func(t *testing.T) {
+		srcErr := &transport.Error{
+			StatusCode: http.StatusNotFound,
+		}
+		result := asNotFoundError(srcErr, ref)
+		require.NotNil(t, result)
+		require.Equal(t, ErrNotFound{Reference: ref}, result)
+	})
+	t.Run("401", func(t *testing.T) {
+		srcErr := &transport.Error{
+			StatusCode: http.StatusUnauthorized,
+		}
+		result := asNotFoundError(srcErr, ref)
+		require.Nil(t, result)
+	})
+}

tests/integration/registry_integration_test.go

@@ -10,23 +10,32 @@ import (
 	"get.porter.sh/porter/pkg/cnab"
 	cnabtooci "get.porter.sh/porter/pkg/cnab/cnab-to-oci"
 	"get.porter.sh/porter/pkg/portercontext"
-	"get.porter.sh/porter/tests"
 	"get.porter.sh/porter/tests/tester"
 	"github.com/stretchr/testify/require"
 )
 
-func TestRegistry_ListTags(t *testing.T) {
+func TestRegistry(t *testing.T) {
 	ctx := context.Background()
 	c := portercontext.NewTestContext(t)
 	r := cnabtooci.NewRegistry(c.Context)
 
 	t.Run("secure registry, existing bundle", func(t *testing.T) {
-		ref := cnab.MustParseOCIReference("docker.io/carolynvs/porter-hello-nonroot")
-		tags, err := r.ListTags(ctx, ref, cnabtooci.RegistryOptions{})
+		repo := cnab.MustParseOCIReference("ghcr.io/getporter/examples/porter-hello")
+		ref, err := repo.WithTag("v0.2.0")
+		require.NoError(t, err)
+
+		// List Tags
+		regOpts := cnabtooci.RegistryOptions{}
+		tags, err := r.ListTags(ctx, repo, regOpts)
 		require.NoError(t, err, "ListTags failed")
+		require.Contains(t, tags, "v0.2.0", "expected a tag for the bundle version")
+		require.Contains(t, tags, "3cb284ae76addb8d56b52bb7d6838351", "expected a tag for the invocation image")
 
-		require.Contains(t, tags, "v0.1.0", "expected a tag for the bundle version")
-		require.Contains(t, tags, "0540db3f2c70103816cc91e9c4207447", "expected a tag for the invocation image")
+		// GetBundleMetadata
+		// Validates that we are passing auth when querying the registry
+		meta, err := r.GetBundleMetadata(ctx, ref, regOpts)
+		require.NoError(t, err, "GetBundleMetadata failed")
+		require.Equal(t, "sha256:276b44be3f478b4c8d1f99c1925386d45a878a853f22436ece5589f32e9df384", meta.Digest.String(), "incorrect bundle digest")
 	})
 
 	t.Run("insecure registry, existing bundle", func(t *testing.T) {
@@ -37,20 +46,35 @@ func TestRegistry_ListTags(t *testing.T) {
 		reg := testr.StartTestRegistry(tester.TestRegistryOptions{UseTLS: true})
 
 		// Copy a test bundle to the registry
-		testRef := fmt.Sprintf("%s/porter-hello-nonroot:v0.1.0", reg)
-		testr.RunPorter("copy", "--source=docker.io/carolynvs/porter-hello-nonroot:v0.1.0", "--destination", testRef, "--insecure-registry")
+		testRef := fmt.Sprintf("%s/porter-hello-nonroot:v0.2.0", reg)
+		testr.RunPorter("copy", "--source=ghcr.io/getporter/examples/porter-hello:v0.2.0", "--destination", testRef, "--insecure-registry")
 
+		// List Tags
 		ref := cnab.MustParseOCIReference(testRef)
-		tags, err := r.ListTags(ctx, ref, cnabtooci.RegistryOptions{InsecureRegistry: true})
+		regOpts := cnabtooci.RegistryOptions{InsecureRegistry: true}
+		tags, err := r.ListTags(ctx, ref, regOpts)
 		require.NoError(t, err, "ListTags failed")
+		require.Contains(t, tags, "v0.2.0", "expected a tag for the bundle version")
 
-		require.Contains(t, tags, "v0.1.0", "expected a tag for the bundle version")
+		// GetBundleMetadata
+		// Validate that call works when no auth is needed (since it's the local test registry, there is no auth)
+		meta, err := r.GetBundleMetadata(ctx, ref, regOpts)
+		require.NoError(t, err, "GetBundleMetadata failed")
+		require.Equal(t, "sha256:276b44be3f478b4c8d1f99c1925386d45a878a853f22436ece5589f32e9df384", meta.Digest.String(), "incorrect bundle digest")
 	})
 
 	t.Run("nonexistant bundle", func(t *testing.T) {
-		ref := cnab.MustParseOCIReference("docker.io/carolynvs/oops-i-dont-exist")
-		_, err := r.ListTags(ctx, ref, cnabtooci.RegistryOptions{})
-		tests.RequireErrorContains(t, err, "error listing tags for docker.io/carolynvs/oops-i-dont-exist")
+		ref := cnab.MustParseOCIReference("ghcr.io/getporter/oops-i-dont-exist")
+
+		// List Tags
 		// Note that listing tags on a nonexistent repo will always yield an authentication error, instead of a not found, to avoid leaking information about private repositories
+		regOpts := cnabtooci.RegistryOptions{}
+		_, err := r.ListTags(ctx, ref, regOpts)
+		require.ErrorIs(t, cnabtooci.ErrNotFound{}, err)
+
+		// GetBundleMetadata
+		// Validates that we are passing auth when querying the registry
+		_, err = r.GetBundleMetadata(ctx, ref, regOpts)
+		require.ErrorIs(t, cnabtooci.ErrNotFound{}, err)
 	})
 }