Skip to content

Commit 13a45d2

Browse files
committed
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 <[email protected]>
1 parent 62ed781 commit 13a45d2

File tree

8 files changed

+197
-93
lines changed

8 files changed

+197
-93
lines changed

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,25 @@ type TestRegistry struct {
1515
MockPullBundle func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (cnab.BundleReference, error)
1616
MockPushBundle func(ctx context.Context, ref cnab.BundleReference, opts RegistryOptions) (bundleReference cnab.BundleReference, err error)
1717
MockPushImage func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (imageDigest digest.Digest, err error)
18-
MockGetCachedImage func(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error)
18+
MockGetCachedImage func(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error)
1919
MockListTags func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) ([]string, error)
2020
MockPullImage func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) error
2121
MockGetBundleMetadata func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (BundleMetadata, error)
22-
cache map[string]ImageSummary
22+
MockGetImageMetadata func(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error)
23+
cache map[string]ImageMetadata
2324
}
2425

2526
func NewTestRegistry() *TestRegistry {
2627
return &TestRegistry{
27-
cache: make(map[string]ImageSummary),
28+
cache: make(map[string]ImageMetadata),
2829
}
2930
}
3031

3132
func (t TestRegistry) PullBundle(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (cnab.BundleReference, error) {
3233
if t.MockPullBundle != nil {
3334
return t.MockPullBundle(ctx, ref, opts)
3435
}
35-
sum, _ := NewImageSummary(ref.String(), types.ImageInspect{ID: cnab.NewULID()})
36+
sum, _ := NewImageSummaryFromInspect(ref, types.ImageInspect{ID: cnab.NewULID()})
3637
t.cache[ref.String()] = sum
3738

3839
return cnab.BundleReference{Reference: ref}, nil
@@ -53,19 +54,35 @@ func (t *TestRegistry) PushImage(ctx context.Context, ref cnab.OCIReference, opt
5354
return "sha256:75c495e5ce9c428d482973d72e3ce9925e1db304a97946c9aa0b540d7537e041", nil
5455
}
5556

56-
func (t *TestRegistry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error) {
57+
func (t *TestRegistry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error) {
5758
if t.MockGetCachedImage != nil {
5859
return t.MockGetCachedImage(ctx, ref)
5960
}
6061

6162
img := ref.String()
6263
sum, ok := t.cache[img]
6364
if !ok {
64-
return ImageSummary{}, fmt.Errorf("image %s not found in cache", img)
65+
return ImageMetadata{}, fmt.Errorf("failed to find image in docker cache: %w", ErrNotFound{Reference: ref})
6566
}
6667
return sum, nil
6768
}
6869

70+
func (t TestRegistry) GetImageMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error) {
71+
meta, err := t.GetCachedImage(ctx, ref)
72+
if err != nil {
73+
if t.MockGetImageMetadata != nil {
74+
return t.MockGetImageMetadata(ctx, ref, opts)
75+
} else {
76+
mockImg := ImageMetadata{
77+
Reference: ref,
78+
RepoDigests: []string{fmt.Sprintf("%s@sha256:75c495e5ce9c428d482973d72e3ce9925e1db304a97946c9aa0b540d7537e041", ref.Repository())},
79+
}
80+
return mockImg, nil
81+
}
82+
}
83+
return meta, nil
84+
}
85+
6986
func (t *TestRegistry) ListTags(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) ([]string, error) {
7087
if t.MockListTags != nil {
7188
return t.MockListTags(ctx, ref, opts)
@@ -80,7 +97,7 @@ func (t *TestRegistry) PullImage(ctx context.Context, ref cnab.OCIReference, opt
8097
}
8198

8299
image := ref.String()
83-
sum, err := NewImageSummary(image, types.ImageInspect{
100+
sum, err := NewImageSummaryFromInspect(ref, types.ImageInspect{
84101
ID: cnab.NewULID(),
85102
RepoDigests: []string{fmt.Sprintf("%s@sha256:75c495e5ce9c428d482973d72e3ce9925e1db304a97946c9aa0b540d7537e041", image)},
86103
})

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,18 @@ type RegistryProvider interface {
2323

2424
// GetCachedImage returns a particular image from the local image cache.
2525
// Use ErrNotFound to detect if the failure is because the image is not in the local Docker cache.
26-
GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error)
26+
GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error)
2727

2828
// ListTags returns all tags defined on the specified repository.
2929
ListTags(ctx context.Context, repo cnab.OCIReference, opts RegistryOptions) ([]string, error)
3030

3131
// PullImage pulls an image from an OCI registry and returns the image's digest
3232
PullImage(ctx context.Context, image cnab.OCIReference, opts RegistryOptions) error
3333

34+
// GetImageMetadata returns information about an image in a registry
35+
// Use ErrNotFound to detect if the error is because the image is not in the registry.
36+
GetImageMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error)
37+
3438
// GetBundleMetadata returns information about a bundle in a registry
3539
// Use ErrNotFound to detect if the error is because the bundle is not in the registry.
3640
GetBundleMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (BundleMetadata, error)

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

Lines changed: 71 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/docker/docker/api/types"
2121
"github.com/docker/docker/pkg/jsonmessage"
2222
"github.com/google/go-containerregistry/pkg/crane"
23+
oci "github.com/google/go-containerregistry/pkg/v1"
2324
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
2425
"github.com/moby/term"
2526
"github.com/opencontainers/go-digest"
@@ -266,24 +267,27 @@ func (r *Registry) displayEvent(ev remotes.FixupEvent) {
266267
}
267268

268269
// GetCachedImage returns information about an image from local docker cache.
269-
func (r *Registry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageSummary, error) {
270+
func (r *Registry) GetCachedImage(ctx context.Context, ref cnab.OCIReference) (ImageMetadata, error) {
270271
image := ref.String()
271272
ctx, log := tracing.StartSpan(ctx, attribute.String("reference", image))
272273
defer log.EndSpan()
273274

274275
cli, err := docker.GetDockerClient()
275276
if err != nil {
276-
return ImageSummary{}, log.Error(err)
277+
return ImageMetadata{}, log.Error(err)
277278
}
278279

279280
result, _, err := cli.Client().ImageInspectWithRaw(ctx, image)
280281
if err != nil {
281-
return ImageSummary{}, log.Error(fmt.Errorf("failed to find image in docker cache: %w", ErrNotFound{Reference: ref}))
282+
err = fmt.Errorf("failed to find image in docker cache: %w", ErrNotFound{Reference: ref})
283+
// log as debug because this isn't a terminal error
284+
log.Debugf(err.Error())
285+
return ImageMetadata{}, err
282286
}
283287

284-
summary, err := NewImageSummary(image, result)
288+
summary, err := NewImageSummaryFromInspect(ref, result)
285289
if err != nil {
286-
return ImageSummary{}, log.Error(fmt.Errorf("failed to extract image %s in docker cache: %w", image, err))
290+
return ImageMetadata{}, log.Error(fmt.Errorf("failed to extract image %s in docker cache: %w", image, err))
287291
}
288292

289293
return summary, nil
@@ -331,6 +335,39 @@ func (r *Registry) GetBundleMetadata(ctx context.Context, ref cnab.OCIReference,
331335
}, nil
332336
}
333337

338+
// GetImageMetadata returns information about an image in a registry
339+
// Use ErrNotFound to detect if the error is because the image is not in the registry.
340+
func (r *Registry) GetImageMetadata(ctx context.Context, ref cnab.OCIReference, opts RegistryOptions) (ImageMetadata, error) {
341+
ctx, span := tracing.StartSpan(ctx, attribute.String("reference", ref.String()))
342+
defer span.EndSpan()
343+
344+
// Check if we already have the image in the Docker cache
345+
cachedResult, err := r.GetCachedImage(ctx, ref)
346+
if err != nil {
347+
if !errors.Is(err, ErrNotFound{}) {
348+
return ImageMetadata{}, err
349+
}
350+
}
351+
352+
// Check if we have the repository digest cached for the referenced image
353+
if cachedDigest, err := cachedResult.Digest(); err == nil {
354+
span.SetAttributes(attribute.String("cached-digest", cachedDigest.String()))
355+
return cachedResult, nil
356+
}
357+
358+
// Do a HEAD against the registry to retrieve image metadata without pulling the entire image contents
359+
desc, err := crane.Head(ref.String(), opts.toCraneOptions()...)
360+
if err != nil {
361+
if notFoundErr := asNotFoundError(err, ref); notFoundErr != nil {
362+
return ImageMetadata{}, span.Error(notFoundErr)
363+
}
364+
return ImageMetadata{}, span.Errorf("error fetching image metadata for %s: %w", ref, err)
365+
}
366+
367+
span.SetAttributes(attribute.String("fetched-digest", desc.Digest.String()))
368+
return NewImageSummaryFromDescriptor(ref, desc)
369+
}
370+
334371
// asNotFoundError checks if the error is an HTTP 404 not found error, and if so returns a corresponding ErrNotFound instance.
335372
func asNotFoundError(err error, ref cnab.OCIReference) error {
336373
var httpError *transport.Error
@@ -343,49 +380,56 @@ func asNotFoundError(err error, ref cnab.OCIReference) error {
343380
return nil
344381
}
345382

346-
// ImageSummary contains information about an OCI image.
347-
type ImageSummary struct {
348-
types.ImageInspect
349-
imageRef cnab.OCIReference
383+
// ImageMetadata contains information about an OCI image.
384+
type ImageMetadata struct {
385+
Reference cnab.OCIReference
386+
RepoDigests []string
350387
}
351388

352-
func NewImageSummary(imageRef string, sum types.ImageInspect) (ImageSummary, error) {
353-
ref, err := cnab.ParseOCIReference(imageRef)
354-
if err != nil {
355-
return ImageSummary{}, err
356-
}
357-
358-
img := ImageSummary{
359-
imageRef: ref,
360-
ImageInspect: sum,
389+
func NewImageSummaryFromInspect(ref cnab.OCIReference, sum types.ImageInspect) (ImageMetadata, error) {
390+
img := ImageMetadata{
391+
Reference: ref,
392+
RepoDigests: sum.RepoDigests,
361393
}
362394
if img.IsZero() {
363-
return ImageSummary{}, fmt.Errorf("invalid image summary for image reference %s", imageRef)
395+
return ImageMetadata{}, fmt.Errorf("invalid image summary for image reference %s", ref)
364396
}
365397

366398
return img, nil
367399
}
368400

369-
func (i ImageSummary) GetImageReference() cnab.OCIReference {
370-
return i.imageRef
401+
func NewImageSummaryFromDescriptor(ref cnab.OCIReference, desc *oci.Descriptor) (ImageMetadata, error) {
402+
repoDigest, err := ref.WithDigest(digest.NewDigestFromHex(desc.Digest.Algorithm, desc.Digest.Hex))
403+
if err != nil {
404+
return ImageMetadata{}, fmt.Errorf("error building an OCI reference from image %s and digest %s", ref.Repository(), ref.Digest())
405+
}
406+
407+
return ImageMetadata{
408+
Reference: ref,
409+
RepoDigests: []string{repoDigest.String()},
410+
}, nil
411+
}
412+
413+
func (i ImageMetadata) String() string {
414+
return i.Reference.String()
371415
}
372416

373-
func (i ImageSummary) IsZero() bool {
374-
return i.ID == ""
417+
func (i ImageMetadata) IsZero() bool {
418+
return i.String() == ""
375419
}
376420

377421
// Digest returns the image digest for the image reference.
378-
func (i ImageSummary) Digest() (digest.Digest, error) {
422+
func (i ImageMetadata) Digest() (digest.Digest, error) {
379423
if len(i.RepoDigests) == 0 {
380-
return "", fmt.Errorf("failed to get digest for image: %s", i.imageRef.String())
424+
return "", fmt.Errorf("failed to get digest for image: %s", i)
381425
}
382426
var imgDigest digest.Digest
383427
for _, rd := range i.RepoDigests {
384428
imgRef, err := cnab.ParseOCIReference(rd)
385429
if err != nil {
386430
return "", err
387431
}
388-
if imgRef.Repository() != i.imageRef.Repository() {
432+
if imgRef.Repository() != i.Reference.Repository() {
389433
continue
390434
}
391435

@@ -398,7 +442,7 @@ func (i ImageSummary) Digest() (digest.Digest, error) {
398442
}
399443

400444
if imgDigest == "" {
401-
return "", fmt.Errorf("cannot find image digest for desired repo %s", i.imageRef.String())
445+
return "", fmt.Errorf("cannot find image digest for desired repo %s", i)
402446
}
403447

404448
if err := imgDigest.Validate(); err != nil {

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66

77
"get.porter.sh/porter/pkg/cnab"
88
"github.com/docker/docker/api/types"
9+
oci "github.com/google/go-containerregistry/pkg/v1"
910
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
11+
"github.com/stretchr/testify/assert"
1012
"github.com/stretchr/testify/require"
1113
)
1214

@@ -31,13 +33,6 @@ func TestImageSummary(t *testing.T) {
3133
expected: expectedOutput{imageRef: "test/image:latest", digest: "sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687"},
3234
expectedErr: "",
3335
},
34-
{
35-
name: "invalid image reference",
36-
imgRef: "test-",
37-
imageSummary: types.ImageInspect{ID: "test", RepoDigests: []string{"test/image@sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687"}},
38-
expected: expectedOutput{hasInitErr: true},
39-
expectedErr: "invalid reference format",
40-
},
4136
{
4237
name: "empty repo digests",
4338
imgRef: "test/image:latest",
@@ -61,12 +56,12 @@ func TestImageSummary(t *testing.T) {
6156
for _, tt := range testcases {
6257
tt := tt
6358
t.Run(tt.name, func(t *testing.T) {
64-
sum, err := NewImageSummary(tt.imgRef, tt.imageSummary)
59+
sum, err := NewImageSummaryFromInspect(cnab.MustParseOCIReference(tt.imgRef), tt.imageSummary)
6560
if tt.expected.hasInitErr {
6661
require.ErrorContains(t, err, tt.expectedErr)
6762
return
6863
}
69-
require.Equal(t, sum.GetImageReference().String(), tt.expected.imageRef)
64+
require.Equal(t, sum.Reference.String(), tt.expected.imageRef)
7065
digest, err := sum.Digest()
7166
if tt.expected.digest == "" {
7267
require.ErrorContains(t, err, tt.expectedErr)
@@ -77,6 +72,20 @@ func TestImageSummary(t *testing.T) {
7772
}
7873
}
7974

75+
func TestNewImageSummaryFromDescriptor(t *testing.T) {
76+
// Validate that we can round trip a digest in the different representations without it changing.
77+
// This is a regression test because at one point we used the wrong constructor and it ended up making a new hash, which is difficult to detect and troubleshoot.
78+
hash, err := oci.NewHash("sha256:499f71eec2e3bd78f26c268bbf5b2a65f73b96216fac4a89b86b5ebf115527b6")
79+
require.NoError(t, err, "error creating test hash")
80+
ref := cnab.MustParseOCIReference("localhost:5000/whalesayd@" + hash.String())
81+
desc := &oci.Descriptor{Digest: hash}
82+
s, err := NewImageSummaryFromDescriptor(ref, desc)
83+
require.NoError(t, err, "NewImageSummaryFromDescriptor failed")
84+
repoDigest, err := s.Digest()
85+
require.NoError(t, err, "failed to get repository digest for image summary")
86+
assert.Equal(t, hash.String(), repoDigest.String())
87+
}
88+
8089
func TestAsNotFoundError(t *testing.T) {
8190
ref := cnab.MustParseOCIReference("example.com/mybuns:v1.2.3")
8291
t.Run("404", func(t *testing.T) {

pkg/porter/generateManifest.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,11 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions
8888
return span.Errorf("failed to parse image %s reference: %w", img.Repository, err)
8989
}
9090

91-
digest, err := p.getImageLatestDigest(ctx, ref)
91+
digest, err := p.getImageDigest(ctx, ref)
9292
if err != nil {
9393
return span.Error(err)
9494
}
95+
span.SetAttributes(attribute.String("digest", digest.Encoded()))
9596

9697
var path string
9798
for _, p := range nc.PathStack {
@@ -115,8 +116,8 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions
115116
return e.WriteFile(build.LOCAL_MANIFEST)
116117
}
117118

118-
func (p *Porter) getImageLatestDigest(ctx context.Context, img cnab.OCIReference) (digest.Digest, error) {
119-
ctx, span := tracing.StartSpan(ctx)
119+
func (p *Porter) getImageDigest(ctx context.Context, img cnab.OCIReference) (digest.Digest, error) {
120+
ctx, span := tracing.StartSpan(ctx, attribute.String("image", img.String()))
120121
defer span.EndSpan()
121122

122123
// if no image tag is specified, default to use latest
@@ -128,17 +129,18 @@ func (p *Porter) getImageLatestDigest(ctx context.Context, img cnab.OCIReference
128129
img = refWithTag
129130
}
130131

131-
// Right now there isn't a way to specify --insecure-registry for build
132-
// because the underlying implementation in PullImage doesn't support it.
133-
err := p.Registry.PullImage(ctx, img, cnabtooci.RegistryOptions{})
132+
// TODO: add --insecure to build so that we can pull from an insecure registry
133+
regOpts := cnabtooci.RegistryOptions{}
134+
imgSummary, err := p.Registry.GetImageMetadata(ctx, img, regOpts)
134135
if err != nil {
135136
return "", err
136137
}
137138

138-
imgSummary, err := p.Registry.GetCachedImage(ctx, img)
139+
imgDigest, err := imgSummary.Digest()
139140
if err != nil {
140-
return "", err
141+
return "", span.Error(err)
141142
}
142143

143-
return imgSummary.Digest()
144+
span.SetAttributes(attribute.String("digest", imgDigest.String()))
145+
return imgDigest, nil
144146
}

0 commit comments

Comments
 (0)