From 36ed299501ae5ecfcc3a89999777f83b8072f9df Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Tue, 14 Feb 2023 14:05:25 -0600 Subject: [PATCH 1/2] Do not pull referenced images during build At build time, Porter needs the repository digest of each referenced bundle from porter.yaml. We update the referenced images in the final porter.yaml generated to .cnab/app/porter.yaml with the digest, so that the bundle is "pinned" to a specific image that can't be messed up by a force push over an existing tag for example. I have updated how we do this so that instead of pulling the entire referenced image, we just call HEAD on the image to get its repository digest. Signed-off-by: Carolyn Van Slyck --- pkg/cnab/cnab-to-oci/helpers.go | 31 ++++++-- pkg/cnab/cnab-to-oci/provider.go | 6 +- pkg/cnab/cnab-to-oci/registry.go | 102 +++++++++++++++++++------- pkg/cnab/cnab-to-oci/registry_test.go | 28 ++++--- pkg/porter/generateManifest.go | 20 ++--- pkg/porter/generateManifest_test.go | 78 ++++++++++++-------- pkg/porter/stamp.go | 13 ++-- tests/smoke/airgap_test.go | 17 ++++- 8 files changed, 200 insertions(+), 95 deletions(-) diff --git a/pkg/cnab/cnab-to-oci/helpers.go b/pkg/cnab/cnab-to-oci/helpers.go index 10af7d5e4..9f1bd15d4 100644 --- a/pkg/cnab/cnab-to-oci/helpers.go +++ b/pkg/cnab/cnab-to-oci/helpers.go @@ -15,16 +15,17 @@ 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), } } @@ -32,7 +33,7 @@ func (t TestRegistry) PullBundle(ctx context.Context, ref cnab.OCIReference, opt 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,7 +54,7 @@ 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) } @@ -61,11 +62,27 @@ func (t *TestRegistry) GetCachedImage(ctx context.Context, ref cnab.OCIReference 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)}, }) diff --git a/pkg/cnab/cnab-to-oci/provider.go b/pkg/cnab/cnab-to-oci/provider.go index bd1352393..fd59d234d 100644 --- a/pkg/cnab/cnab-to-oci/provider.go +++ b/pkg/cnab/cnab-to-oci/provider.go @@ -23,7 +23,7 @@ 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) @@ -31,6 +31,10 @@ type RegistryProvider interface { // 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) diff --git a/pkg/cnab/cnab-to-oci/registry.go b/pkg/cnab/cnab-to-oci/registry.go index f724c8961..aa94524eb 100644 --- a/pkg/cnab/cnab-to-oci/registry.go +++ b/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,41 +381,49 @@ 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 { @@ -385,7 +431,7 @@ func (i ImageSummary) Digest() (digest.Digest, error) { 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 { diff --git a/pkg/cnab/cnab-to-oci/registry_test.go b/pkg/cnab/cnab-to-oci/registry_test.go index 5fc6161a3..f205f9a72 100644 --- a/pkg/cnab/cnab-to-oci/registry_test.go +++ b/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) { diff --git a/pkg/porter/generateManifest.go b/pkg/porter/generateManifest.go index 6198d4d92..6aa926b4b 100644 --- a/pkg/porter/generateManifest.go +++ b/pkg/porter/generateManifest.go @@ -88,10 +88,11 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions return span.Errorf("failed to parse image %s reference: %w", img.Repository, err) } - digest, err := p.getImageLatestDigest(ctx, ref) + digest, err := p.getImageDigest(ctx, ref) if err != nil { return span.Error(err) } + span.SetAttributes(attribute.String("digest", digest.Encoded())) var path string for _, p := range nc.PathStack { @@ -115,8 +116,9 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions return e.WriteFile(build.LOCAL_MANIFEST) } -func (p *Porter) getImageLatestDigest(ctx context.Context, img cnab.OCIReference) (digest.Digest, error) { - ctx, span := tracing.StartSpan(ctx) +// getImageDigest retrieves the repository digest associated with the specified image reference. +func (p *Porter) getImageDigest(ctx context.Context, img cnab.OCIReference) (digest.Digest, error) { + ctx, span := tracing.StartSpan(ctx, attribute.String("image", img.String())) defer span.EndSpan() // if no image tag is specified, default to use latest @@ -128,17 +130,17 @@ func (p *Porter) getImageLatestDigest(ctx context.Context, img cnab.OCIReference img = refWithTag } - // Right now there isn't a way to specify --insecure-registry for build - // because the underlying implementation in PullImage doesn't support it. - err := p.Registry.PullImage(ctx, img, cnabtooci.RegistryOptions{}) + regOpts := cnabtooci.RegistryOptions{} + imgSummary, err := p.Registry.GetImageMetadata(ctx, img, regOpts) if err != nil { return "", err } - imgSummary, err := p.Registry.GetCachedImage(ctx, img) + imgDigest, err := imgSummary.GetRepositoryDigest() if err != nil { - return "", err + return "", span.Error(err) } - return imgSummary.Digest() + span.SetAttributes(attribute.String("digest", imgDigest.String())) + return imgDigest, nil } diff --git a/pkg/porter/generateManifest_test.go b/pkg/porter/generateManifest_test.go index 661798727..f768d8497 100644 --- a/pkg/porter/generateManifest_test.go +++ b/pkg/porter/generateManifest_test.go @@ -51,20 +51,19 @@ func Test_generateInternalManifest(t *testing.T) { opts: BuildOptions{Customs: []string{"key1=editedValue1", "key2.nestedKey2=editedValue2"}}, wantManifest: "custom-input.yaml", }, { - name: "failed to pull image reference", + name: "failed to fetch image metadata", opts: BuildOptions{}, - wantErr: "failed to pull image", + wantErr: "failed to fetch image metadata", wantManifest: "expected-result.yaml", }, } - p := NewTestPorter(t) - defer p.Close() - p.TestRegistry.MockGetCachedImage = mockGetCachedImage - for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + manifest := config.Name if tc.opts.File != "" { manifest = tc.opts.File @@ -74,8 +73,10 @@ func Test_generateInternalManifest(t *testing.T) { err := tc.opts.Validate(p.Porter) require.NoError(t, err) - if tc.wantErr != "" { - p.TestRegistry.MockPullImage = mockPullImageFailure + if tc.wantErr == "" { + p.TestRegistry.MockGetCachedImage = mockGetCachedImage + } else { + p.TestRegistry.MockGetImageMetadata = mockGetImageMetadataFailure } err = p.generateInternalManifest(context.Background(), tc.opts) @@ -94,34 +95,34 @@ func Test_generateInternalManifest(t *testing.T) { } } -func mockPullImageFailure(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) error { - return fmt.Errorf("failed to pull image %s", ref) +func mockGetImageMetadataFailure(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) (cnabtooci.ImageMetadata, error) { + return cnabtooci.ImageMetadata{}, fmt.Errorf("failed to fetch image metadata %s", ref) } -func mockGetCachedImage(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageSummary, error) { +func mockGetCachedImage(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageMetadata, error) { sum := types.ImageInspect{ ID: "test-id", RepoDigests: []string{"test/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f"}, } - return cnabtooci.NewImageSummary(ref.String(), sum) + return cnabtooci.NewImageSummaryFromInspect(ref, sum) } func Test_getImageLatestDigest(t *testing.T) { - defaultMockGetCachedImage := func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageSummary, error) { + defaultMockGetCachedImage := func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageMetadata, error) { sum := types.ImageInspect{ ID: "test-id", RepoDigests: []string{"test/repo@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f"}, } - return cnabtooci.NewImageSummary(ref.String(), sum) + return cnabtooci.NewImageSummaryFromInspect(ref, sum) } testcases := []struct { - name string - imgRef string - mockGetCachedImage func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageSummary, error) - mockPullImage func(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) error - wantErr string - wantDigest string + name string + imgRef string + mockGetCachedImage func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageMetadata, error) + mockGetImageMetadata func(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) (cnabtooci.ImageMetadata, error) + wantErr string + wantDigest string }{{ name: "success", imgRef: "test/repo", @@ -129,35 +130,50 @@ func Test_getImageLatestDigest(t *testing.T) { }, { name: "non-default image tag", imgRef: "test/repo:v0.1.0", - mockPullImage: func(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) error { + mockGetCachedImage: func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageMetadata, error) { + return cnabtooci.ImageMetadata{}, fmt.Errorf("not in cache") + }, + mockGetImageMetadata: func(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) (cnabtooci.ImageMetadata, error) { require.True(t, ref.HasTag()) require.Equal(t, "v0.1.0", ref.Tag()) - return nil + return cnabtooci.ImageMetadata{ + Reference: ref, + RepoDigests: []string{ + "test/repo@sha256:d2134b0be91e9e293bc872b719538440e5f933e9828cd96430c85d904afb5aa9", + }, + }, nil }, - wantDigest: "sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f", + wantDigest: "sha256:d2134b0be91e9e293bc872b719538440e5f933e9828cd96430c85d904afb5aa9", }, { name: "failure", imgRef: "test/repo", - mockGetCachedImage: func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageSummary, error) { - return cnabtooci.ImageSummary{}, errors.New("failed to get cached image") + mockGetCachedImage: func(ctx context.Context, ref cnab.OCIReference) (cnabtooci.ImageMetadata, error) { + return cnabtooci.ImageMetadata{}, errors.New("failed to get cached image") }, - wantErr: "failed to get cached image", + mockGetImageMetadata: mockGetImageMetadataFailure, + wantErr: "failed to fetch image metadata", }, } - p := NewTestPorter(t) - defer p.Close() - p.TestRegistry.MockGetCachedImage = defaultMockGetCachedImage - for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + ref, err := cnab.ParseOCIReference(tc.imgRef) require.NoError(t, err) + if tc.mockGetCachedImage != nil { p.TestRegistry.MockGetCachedImage = tc.mockGetCachedImage + } else { + p.TestRegistry.MockGetCachedImage = defaultMockGetCachedImage } - digest, err := p.getImageLatestDigest(context.Background(), ref) + if tc.mockGetImageMetadata != nil { + p.TestRegistry.MockGetImageMetadata = tc.mockGetImageMetadata + } + + digest, err := p.getImageDigest(context.Background(), ref) if tc.wantErr != "" { require.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/porter/stamp.go b/pkg/porter/stamp.go index bda768fbb..18ae762fa 100644 --- a/pkg/porter/stamp.go +++ b/pkg/porter/stamp.go @@ -9,6 +9,7 @@ import ( "get.porter.sh/porter/pkg/build" "get.porter.sh/porter/pkg/cnab" + cnabtooci "get.porter.sh/porter/pkg/cnab/cnab-to-oci" configadapter "get.porter.sh/porter/pkg/cnab/config-adapter" "get.porter.sh/porter/pkg/manifest" "get.porter.sh/porter/pkg/tracing" @@ -82,7 +83,7 @@ func (p *Porter) IsBundleUpToDate(ctx context.Context, opts BundleDefinitionOpti // Check whether invocation images exist in host registry. for _, invocationImage := range bun.InvocationImages { - // if the invovationImage is built before using a random string tag, + // if the invocationImage is built before using a random string tag, // we should rebuild it with the new format if strings.HasSuffix(invocationImage.Image, "-installer") { return false, nil @@ -93,14 +94,14 @@ func (p *Porter) IsBundleUpToDate(ctx context.Context, opts BundleDefinitionOpti return false, span.Errorf("error parsing %s as an OCI image reference: %w", invocationImage.Image, err) } - cachedImg, err := p.Registry.GetCachedImage(ctx, imgRef) + _, err = p.Registry.GetCachedImage(ctx, imgRef) if err != nil { + if errors.Is(err, cnabtooci.ErrNotFound{}) { + span.Debugf("Invocation image %s doesn't exist in the local image cache, will need to build first", invocationImage.Image) + return false, nil + } return false, err - } - if cachedImg.IsZero() { - span.Debugf("Invocation image %s doesn't exist in the local image cache, will need to build first", invocationImage.Image) - return false, nil } } diff --git a/tests/smoke/airgap_test.go b/tests/smoke/airgap_test.go index 2e67285e4..73207e9a6 100644 --- a/tests/smoke/airgap_test.go +++ b/tests/smoke/airgap_test.go @@ -48,6 +48,7 @@ func TestAirgappedEnvironment(t *testing.T) { // Start a temporary insecure test registry reg1 := test.StartTestRegistry(tester.TestRegistryOptions{UseTLS: tc.useTLS, UseAlias: tc.useAlias}) + t.Cleanup(reg1.Close) // Publish referenced image to the insecure registry // This helps test that we can publish a bundle that references images from multiple registries @@ -55,15 +56,16 @@ func TestAirgappedEnvironment(t *testing.T) { referencedImg := "carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f" localRegRepo := fmt.Sprintf("%s/whalesayd", reg1) localRegRef := localRegRepo + ":latest" - require.NoError(t, shx.RunE("docker", "pull", referencedImg)) - require.NoError(t, shx.RunE("docker", "tag", referencedImg, localRegRef)) - output, err := shx.OutputE("docker", "push", localRegRef) + require.NoError(t, shx.RunV("docker", "pull", referencedImg)) + require.NoError(t, shx.RunV("docker", "tag", referencedImg, localRegRef)) + output, err := shx.OutputV("docker", "push", localRegRef) require.NoError(t, err) digest := getDigestFromDockerOutput(test.T, output) localRefWithDigest := fmt.Sprintf("%s@%s", localRegRepo, digest) // Start a second insecure test registry reg2 := test.StartTestRegistry(tester.TestRegistryOptions{UseTLS: tc.useTLS, UseAlias: tc.useAlias}) + t.Cleanup(reg2.Close) // Edit the bundle so that it's referencing the image on the temporary registry // make sure the referenced image is not in local image cache @@ -80,6 +82,15 @@ func TestAirgappedEnvironment(t *testing.T) { return yq.SetValue("images.whalesayd.repository", fmt.Sprintf("%s/whalesayd", reg1)) }) + // Build the test bundle separate from publish so we best validate that we aren't pulling referenced images during build anymore + test.RequirePorter("build") + + // Validate that the referenced bundle is not in the local docker cache and that build did not pull it + err = shx.RunE("docker", "image", "inspect", localRefWithDigest) + if err == nil { + t.Fatalf("expected %s to not be in the local docker cache after building the bundle. Build should no longer pull referenced images.", localRefWithDigest) + } + // Publish a test bundle that references the image from the temp registry, and push to another insecure registry test.RequirePorter("publish", "--registry", reg2.String(), insecureFlag) From 74e196eafefcc943e8f4809d595fb88b35ff25ce Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Fri, 7 Apr 2023 16:27:26 -0500 Subject: [PATCH 2/2] Add --insecure-registry to porter build command When porter builds a bundle, we lookup the repository digest of any referenced images. Previously we did that with Pull, which always allowed connections to insecure registries. Now that we are executing a HEAD request instead to get the digest, instead of pulling the image with PullImage, we can be more explicit like we are with the publish commnad. I have added --insecure-registry to porter build, so that the bundle author can decide when building if they want to allow connections to an insecure registry (http or self-signed certificates). Signed-off-by: Carolyn Van Slyck --- cmd/porter/bundle.go | 2 ++ docs/content/cli/build.md | 1 + docs/content/cli/bundles_build.md | 1 + pkg/porter/build.go | 3 +++ pkg/porter/generateManifest.go | 9 ++++++--- pkg/porter/generateManifest_test.go | 3 ++- tests/smoke/airgap_test.go | 2 +- 7 files changed, 16 insertions(+), 5 deletions(-) diff --git a/cmd/porter/bundle.go b/cmd/porter/bundle.go index 6f998be24..85906f4ac 100644 --- a/cmd/porter/bundle.go +++ b/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{ diff --git a/docs/content/cli/build.md b/docs/content/cli/build.md index 93cc6a60c..e03550a5d 100644 --- a/docs/content/cli/build.md +++ b/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 diff --git a/docs/content/cli/bundles_build.md b/docs/content/cli/bundles_build.md index 6fe46549f..1e6346752 100644 --- a/docs/content/cli/bundles_build.md +++ b/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 diff --git a/pkg/porter/build.go b/pkg/porter/build.go index d998ca552..11778bc2d 100644 --- a/pkg/porter/build.go +++ b/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 } diff --git a/pkg/porter/generateManifest.go b/pkg/porter/generateManifest.go index 6aa926b4b..abcc52cbc 100644 --- a/pkg/porter/generateManifest.go +++ b/pkg/porter/generateManifest.go @@ -64,6 +64,10 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions } } + regOpts := cnabtooci.RegistryOptions{ + InsecureRegistry: opts.InsecureRegistry, + } + // find all referenced images that does not have digest specified // get the image digest for all of them and update the manifest with the digest err = e.WalkNodes(ctx, "images.*", func(ctx context.Context, nc *yqlib.NodeContext) error { @@ -88,7 +92,7 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions return span.Errorf("failed to parse image %s reference: %w", img.Repository, err) } - digest, err := p.getImageDigest(ctx, ref) + digest, err := p.getImageDigest(ctx, ref, regOpts) if err != nil { return span.Error(err) } @@ -117,7 +121,7 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions } // getImageDigest retrieves the repository digest associated with the specified image reference. -func (p *Porter) getImageDigest(ctx context.Context, img cnab.OCIReference) (digest.Digest, error) { +func (p *Porter) getImageDigest(ctx context.Context, img cnab.OCIReference, regOpts cnabtooci.RegistryOptions) (digest.Digest, error) { ctx, span := tracing.StartSpan(ctx, attribute.String("image", img.String())) defer span.EndSpan() @@ -130,7 +134,6 @@ func (p *Porter) getImageDigest(ctx context.Context, img cnab.OCIReference) (dig img = refWithTag } - regOpts := cnabtooci.RegistryOptions{} imgSummary, err := p.Registry.GetImageMetadata(ctx, img, regOpts) if err != nil { return "", err diff --git a/pkg/porter/generateManifest_test.go b/pkg/porter/generateManifest_test.go index f768d8497..a389181fc 100644 --- a/pkg/porter/generateManifest_test.go +++ b/pkg/porter/generateManifest_test.go @@ -173,7 +173,8 @@ func Test_getImageLatestDigest(t *testing.T) { p.TestRegistry.MockGetImageMetadata = tc.mockGetImageMetadata } - digest, err := p.getImageDigest(context.Background(), ref) + regOpts := cnabtooci.RegistryOptions{} + digest, err := p.getImageDigest(context.Background(), ref, regOpts) if tc.wantErr != "" { require.ErrorContains(t, err, tc.wantErr) return diff --git a/tests/smoke/airgap_test.go b/tests/smoke/airgap_test.go index 73207e9a6..4706e6258 100644 --- a/tests/smoke/airgap_test.go +++ b/tests/smoke/airgap_test.go @@ -83,7 +83,7 @@ func TestAirgappedEnvironment(t *testing.T) { }) // Build the test bundle separate from publish so we best validate that we aren't pulling referenced images during build anymore - test.RequirePorter("build") + test.RequirePorter("build", insecureFlag) // Validate that the referenced bundle is not in the local docker cache and that build did not pull it err = shx.RunE("docker", "image", "inspect", localRefWithDigest)