Merge pull request #2579 from carolynvs/build-ref-digest-perf

Do not pull referenced images during build

cmd/porter/bundle.go

@@ -91,6 +91,8 @@ The docker driver builds the bundle image using the local Docker host. To use a
 		"Do not use the Docker cache when building the bundle's invocation image.")
 	f.StringArrayVar(&opts.Customs, "custom", nil,
 		"Define an individual key-value pair for the custom section in the form of NAME=VALUE. Use dot notation to specify a nested custom field. May be specified multiple times. Max length is 5,000 characters when used as a build argument.")
+	f.BoolVar(&opts.InsecureRegistry, "insecure-registry", false,
+		"Don't require TLS when pulling referenced images")
 
 	// Allow configuring the --driver flag with build-driver, to avoid conflicts with other commands
 	cmd.Flag("driver").Annotations = map[string][]string{

docs/content/cli/build.md

@@ -42,6 +42,7 @@ porter build [flags]
   -d, --dir string              Path to the build context directory where all bundle assets are located. Defaults to the current directory.
   -f, --file string             Path to the Porter manifest. The path is relative to the build context directory. Defaults to porter.yaml in the current directory.
   -h, --help                    help for build
+      --insecure-registry       Don't require TLS when pulling referenced images
       --name string             Override the bundle name
       --no-cache                Do not use the Docker cache when building the bundle's invocation image.
       --no-lint                 Do not run the linter

docs/content/cli/bundles_build.md

@@ -42,6 +42,7 @@ porter bundles build [flags]
   -d, --dir string              Path to the build context directory where all bundle assets are located. Defaults to the current directory.
   -f, --file string             Path to the Porter manifest. The path is relative to the build context directory. Defaults to porter.yaml in the current directory.
   -h, --help                    help for build
+      --insecure-registry       Don't require TLS when pulling referenced images
       --name string             Override the bundle name
       --no-cache                Do not use the Docker cache when building the bundle's invocation image.
       --no-lint                 Do not run the linter

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

@@ -15,24 +15,25 @@ type TestRegistry struct {
 	MockPullBundle        func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (cnab.BundleReference, error)
 	MockPushBundle        func(ctx context.Context, ref cnab.BundleReference, opts RegistryOptions) (bundleReference cnab.BundleReference, err error)
 	MockPushImage         func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (imageDigest digest.Digest, err error)
-	MockGetCachedImage    func(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error)
+	MockGetCachedImage    func(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error)
 	MockListTags          func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) ([]string, error)
 	MockPullImage         func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) error
 	MockGetBundleMetadata func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (BundleMetadata, error)
-	cache                 map[string]ImageSummary
+	MockGetImageMetadata  func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error)
+	cache                 map[string]ImageMetadata
 }
 
 func NewTestRegistry() *TestRegistry {
 	return &TestRegistry{
-		cache: make(map[string]ImageSummary),
+		cache: make(map[string]ImageMetadata),
 	}
 }
 
 func (t TestRegistry) PullBundle(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (cnab.BundleReference, error) {
 	if t.MockPullBundle != nil {
 		return t.MockPullBundle(ctx, ref, opts)
 	}
-	sum, _ := NewImageSummary(ref.String(), types.ImageInspect{ID: cnab.NewULID()})
+	sum, _ := NewImageSummaryFromInspect(ref, types.ImageInspect{ID: cnab.NewULID()})
 	t.cache[ref.String()] = sum
 
 	return cnab.BundleReference{Reference: ref}, nil
@@ -53,19 +54,35 @@ func (t *TestRegistry) PushImage(ctx context.Context, ref cnab.OCIReference, opt
 	return "sha256:75c495e5ce9c428d482973d72e3ce9925e1db304a97946c9aa0b540d7537e041", nil
 }
 
-func (t *TestRegistry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error) {
+func (t *TestRegistry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error) {
 	if t.MockGetCachedImage != nil {
 		return t.MockGetCachedImage(ctx, ref)
 	}
 
 	img := ref.String()
 	sum, ok := t.cache[img]
 	if !ok {
-		return ImageSummary{}, fmt.Errorf("image %s not found in cache", img)
+		return ImageMetadata{}, fmt.Errorf("failed to find image in docker cache: %w", ErrNotFound{Reference: ref})
 	}
 	return sum, nil
 }
 
+func (t TestRegistry) GetImageMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error) {
+	meta, err := t.GetCachedImage(ctx, ref)
+	if err != nil {
+		if t.MockGetImageMetadata != nil {
+			return t.MockGetImageMetadata(ctx, ref, opts)
+		} else {
+			mockImg := ImageMetadata{
+				Reference:   ref,
+				RepoDigests: []string{fmt.Sprintf("%s@sha256:75c495e5ce9c428d482973d72e3ce9925e1db304a97946c9aa0b540d7537e041", ref.Repository())},
+			}
+			return mockImg, nil
+		}
+	}
+	return meta, nil
+}
+
 func (t *TestRegistry) ListTags(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) ([]string, error) {
 	if t.MockListTags != nil {
 		return t.MockListTags(ctx, ref, opts)
@@ -80,7 +97,7 @@ func (t *TestRegistry) PullImage(ctx context.Context, ref cnab.OCIReference, opt
 	}
 
 	image := ref.String()
-	sum, err := NewImageSummary(image, types.ImageInspect{
+	sum, err := NewImageSummaryFromInspect(ref, types.ImageInspect{
 		ID:          cnab.NewULID(),
 		RepoDigests: []string{fmt.Sprintf("%s@sha256:75c495e5ce9c428d482973d72e3ce9925e1db304a97946c9aa0b540d7537e041", image)},
 	})

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

@@ -23,14 +23,18 @@ type RegistryProvider interface {
 
 	// GetCachedImage returns a particular image from the local image cache.
 	// Use ErrNotFound to detect if the failure is because the image is not in the local Docker cache.
-	GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error)
+	GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error)
 
 	// ListTags returns all tags defined on the specified repository.
 	ListTags(ctx context.Context, repo cnab.OCIReference, opts RegistryOptions) ([]string, error)
 
 	// PullImage pulls an image from an OCI registry and returns the image's digest
 	PullImage(ctx context.Context, image cnab.OCIReference, opts RegistryOptions) error
 
+	// GetImageMetadata returns information about an image in a registry
+	// Use ErrNotFound to detect if the error is because the image is not in the registry.
+	GetImageMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error)
+
 	// GetBundleMetadata returns information about a bundle in a registry
 	// Use ErrNotFound to detect if the error is because the bundle is not in the registry.
 	GetBundleMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (BundleMetadata, error)

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

@@ -266,24 +266,27 @@ func (r *Registry) displayEvent(ev remotes.FixupEvent) {
 }
 
 // GetCachedImage returns information about an image from local docker cache.
-func (r *Registry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error) {
+func (r *Registry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error) {
 	image := ref.String()
 	ctx, log := tracing.StartSpan(ctx, attribute.String("reference", image))
 	defer log.EndSpan()
 
 	cli, err := docker.GetDockerClient()
 	if err != nil {
-		return ImageSummary{}, log.Error(err)
+		return ImageMetadata{}, log.Error(err)
 	}
 
 	result, _, err := cli.Client().ImageInspectWithRaw(ctx, image)
 	if err != nil {
-		return ImageSummary{}, log.Error(fmt.Errorf("failed to find image in docker cache: %w", ErrNotFound{Reference: ref}))
+		err = fmt.Errorf("failed to find image in docker cache: %w", ErrNotFound{Reference: ref})
+		// log as debug because this isn't a terminal error
+		log.Debugf(err.Error())
+		return ImageMetadata{}, err
 	}
 
-	summary, err := NewImageSummary(image, result)
+	summary, err := NewImageSummaryFromInspect(ref, result)
 	if err != nil {
-		return ImageSummary{}, log.Error(fmt.Errorf("failed to extract image %s in docker cache: %w", image, err))
+		return ImageMetadata{}, log.Error(fmt.Errorf("failed to extract image %s in docker cache: %w", image, err))
 	}
 
 	return summary, nil
@@ -331,6 +334,41 @@ func (r *Registry) GetBundleMetadata(ctx context.Context, ref cnab.OCIReference,
 	}, nil
 }
 
+// GetImageMetadata returns information about an image in a registry
+// Use ErrNotFound to detect if the error is because the image is not in the registry.
+func (r *Registry) GetImageMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error) {
+	ctx, span := tracing.StartSpan(ctx, attribute.String("reference", ref.String()))
+	defer span.EndSpan()
+
+	// Check if we already have the image in the Docker cache
+	cachedResult, err := r.GetCachedImage(ctx, ref)
+	if err != nil {
+		if !errors.Is(err, ErrNotFound{}) {
+			return ImageMetadata{}, err
+		}
+	}
+
+	// Check if we have the repository digest cached for the referenced image
+	if cachedDigest, err := cachedResult.GetRepositoryDigest(); err == nil {
+		span.SetAttributes(attribute.String("cached-digest", cachedDigest.String()))
+		return cachedResult, nil
+	}
+
+	// Do a HEAD against the registry to retrieve image metadata without pulling the entire image contents
+	desc, err := crane.Head(ref.String(), opts.toCraneOptions()...)
+	if err != nil {
+		if notFoundErr := asNotFoundError(err, ref); notFoundErr != nil {
+			return ImageMetadata{}, span.Error(notFoundErr)
+		}
+		return ImageMetadata{}, span.Errorf("error fetching image metadata for %s: %w", ref, err)
+	}
+
+	repoDigest := digest.NewDigestFromHex(desc.Digest.Algorithm, desc.Digest.Hex)
+	span.SetAttributes(attribute.String("fetched-digest", repoDigest.String()))
+
+	return NewImageSummaryFromDigest(ref, repoDigest)
+}
+
 // 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
@@ -343,49 +381,57 @@ func asNotFoundError(err error, ref cnab.OCIReference) error {
 	return nil
 }
 
-// ImageSummary contains information about an OCI image.
-type ImageSummary struct {
-	types.ImageInspect
-	imageRef cnab.OCIReference
+// ImageMetadata contains information about an OCI image.
+type ImageMetadata struct {
+	Reference   cnab.OCIReference
+	RepoDigests []string
 }
 
-func NewImageSummary(imageRef string, sum types.ImageInspect) (ImageSummary, error) {
-	ref, err := cnab.ParseOCIReference(imageRef)
-	if err != nil {
-		return ImageSummary{}, err
-	}
-
-	img := ImageSummary{
-		imageRef:     ref,
-		ImageInspect: sum,
+func NewImageSummaryFromInspect(ref cnab.OCIReference, sum types.ImageInspect) (ImageMetadata, error) {
+	img := ImageMetadata{
+		Reference:   ref,
+		RepoDigests: sum.RepoDigests,
 	}
 	if img.IsZero() {
-		return ImageSummary{}, fmt.Errorf("invalid image summary for image reference %s", imageRef)
+		return ImageMetadata{}, fmt.Errorf("invalid image summary for image reference %s", ref)
 	}
 
 	return img, nil
 }
 
-func (i ImageSummary) GetImageReference() cnab.OCIReference {
-	return i.imageRef
+func NewImageSummaryFromDigest(ref cnab.OCIReference, repoDigest digest.Digest) (ImageMetadata, error) {
+	digestedRef, err := ref.WithDigest(repoDigest)
+	if err != nil {
+		return ImageMetadata{}, fmt.Errorf("error building an OCI reference from image %s and digest %s", ref.Repository(), ref.Digest())
+	}
+
+	return ImageMetadata{
+		Reference:   ref,
+		RepoDigests: []string{digestedRef.String()},
+	}, nil
+}
+
+func (i ImageMetadata) String() string {
+	return i.Reference.String()
 }
 
-func (i ImageSummary) IsZero() bool {
-	return i.ID == ""
+func (i ImageMetadata) IsZero() bool {
+	return i.String() == ""
 }
 
-// Digest returns the image digest for the image reference.
-func (i ImageSummary) Digest() (digest.Digest, error) {
+// GetRepositoryDigest finds the repository digest associated with the original
+// image reference used to create this ImageMetadata.
+func (i ImageMetadata) GetRepositoryDigest() (digest.Digest, error) {
 	if len(i.RepoDigests) == 0 {
-		return "", fmt.Errorf("failed to get digest for image: %s", i.imageRef.String())
+		return "", fmt.Errorf("failed to get digest for image: %s", i)
 	}
 	var imgDigest digest.Digest
 	for _, rd := range i.RepoDigests {
 		imgRef, err := cnab.ParseOCIReference(rd)
 		if err != nil {
 			return "", err
 		}
-		if imgRef.Repository() != i.imageRef.Repository() {
+		if imgRef.Repository() != i.Reference.Repository() {
 			continue
 		}
 
@@ -398,7 +444,7 @@ func (i ImageSummary) Digest() (digest.Digest, error) {
 	}
 
 	if imgDigest == "" {
-		return "", fmt.Errorf("cannot find image digest for desired repo %s", i.imageRef.String())
+		return "", fmt.Errorf("cannot find image digest for desired repo %s", i)
 	}
 
 	if err := imgDigest.Validate(); err != nil {

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

@@ -7,6 +7,8 @@ import (
 	"get.porter.sh/porter/pkg/cnab"
 	"github.com/docker/docker/api/types"
 	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
+	"github.com/opencontainers/go-digest"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -31,13 +33,6 @@ func TestImageSummary(t *testing.T) {
 			expected:     expectedOutput{imageRef: "test/image:latest", digest: "sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687"},
 			expectedErr:  "",
 		},
-		{
-			name:         "invalid image reference",
-			imgRef:       "test-",
-			imageSummary: types.ImageInspect{ID: "test", RepoDigests: []string{"test/image@sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687"}},
-			expected:     expectedOutput{hasInitErr: true},
-			expectedErr:  "invalid reference format",
-		},
 		{
 			name:         "empty repo digests",
 			imgRef:       "test/image:latest",
@@ -61,13 +56,13 @@ func TestImageSummary(t *testing.T) {
 	for _, tt := range testcases {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			sum, err := NewImageSummary(tt.imgRef, tt.imageSummary)
+			sum, err := NewImageSummaryFromInspect(cnab.MustParseOCIReference(tt.imgRef), tt.imageSummary)
 			if tt.expected.hasInitErr {
 				require.ErrorContains(t, err, tt.expectedErr)
 				return
 			}
-			require.Equal(t, sum.GetImageReference().String(), tt.expected.imageRef)
-			digest, err := sum.Digest()
+			require.Equal(t, sum.Reference.String(), tt.expected.imageRef)
+			digest, err := sum.GetRepositoryDigest()
 			if tt.expected.digest == "" {
 				require.ErrorContains(t, err, tt.expectedErr)
 				return
@@ -77,6 +72,19 @@ func TestImageSummary(t *testing.T) {
 	}
 }
 
+func TestNewImageSummaryFromDescriptor(t *testing.T) {
+	ref := cnab.MustParseOCIReference("localhost:5000/whalesayd:latest")
+	origRepoDigest := digest.Digest("sha256:499f71eec2e3bd78f26c268bbf5b2a65f73b96216fac4a89b86b5ebf115527b6")
+
+	s, err := NewImageSummaryFromDigest(ref, origRepoDigest)
+	require.NoError(t, err, "NewImageSummaryFromDigest failed")
+
+	// Locate the repository digest associated with the reference and validate that it matches what we input
+	repoDigest, err := s.GetRepositoryDigest()
+	require.NoError(t, err, "failed to get repository digest for image summary")
+	assert.Equal(t, origRepoDigest.String(), repoDigest.String())
+}
+
 func TestAsNotFoundError(t *testing.T) {
 	ref := cnab.MustParseOCIReference("example.com/mybuns:v1.2.3")
 	t.Run("404", func(t *testing.T) {

pkg/porter/build.go

@@ -35,6 +35,9 @@ type BuildOptions struct {
 	// Custom is the unparsed list of NAME=VALUE custom inputs set on the command line.
 	Customs []string
 
+	// InsecureRegistry allows connecting to an unsecured registry or one without verifiable certificates.
+	InsecureRegistry bool
+
 	// parsedCustoms is the parsed set of custom inputs from Customs.
 	parsedCustoms map[string]string
 }