From 8149de51770c0ffa08e7ddef9181712b9ba6dd40 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Tue, 14 Jun 2022 14:20:18 -0400 Subject: [PATCH 1/2] remote: Collect ETag in response, optimistically send that back in future updates --- pkg/v1/remote/descriptor.go | 24 +++++++++++++++--------- pkg/v1/remote/image.go | 3 ++- pkg/v1/remote/index.go | 7 +++++-- pkg/v1/remote/write.go | 7 +++++++ 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/pkg/v1/remote/descriptor.go b/pkg/v1/remote/descriptor.go index 755e81929..c339a5d4f 100644 --- a/pkg/v1/remote/descriptor.go +++ b/pkg/v1/remote/descriptor.go @@ -61,6 +61,8 @@ type Descriptor struct { // So we can share this implementation with Image.. platform v1.Platform + + etag string } // RawManifest exists to satisfy the Taggable interface. @@ -122,7 +124,7 @@ func get(ref name.Reference, acceptable []types.MediaType, options ...Option) (* if err != nil { return nil, err } - b, desc, err := f.fetchManifest(ref, acceptable) + b, desc, etag, err := f.fetchManifest(ref, acceptable) if err != nil { return nil, err } @@ -131,6 +133,7 @@ func get(ref name.Reference, acceptable []types.MediaType, options ...Option) (* Manifest: b, Descriptor: *desc, platform: o.platform, + etag: etag, }, nil } @@ -197,6 +200,7 @@ func (d *Descriptor) remoteImage() *remoteImage { manifest: d.Manifest, mediaType: d.MediaType, descriptor: &d.Descriptor, + etag: d.etag, } } @@ -206,6 +210,7 @@ func (d *Descriptor) remoteIndex() *remoteIndex { manifest: d.Manifest, mediaType: d.MediaType, descriptor: &d.Descriptor, + etag: d.etag, } } @@ -237,11 +242,11 @@ func (f *fetcher) url(resource, identifier string) url.URL { } } -func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType) ([]byte, *v1.Descriptor, error) { +func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType) ([]byte, *v1.Descriptor, string, error) { u := f.url("manifests", ref.Identifier()) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, nil, err + return nil, nil, "", err } accept := []string{} for _, mt := range acceptable { @@ -251,22 +256,22 @@ func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType resp, err := f.Client.Do(req.WithContext(f.context)) if err != nil { - return nil, nil, err + return nil, nil, "", err } defer resp.Body.Close() if err := transport.CheckError(resp, http.StatusOK); err != nil { - return nil, nil, err + return nil, nil, "", err } manifest, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, nil, err + return nil, nil, "", err } digest, size, err := v1.SHA256(bytes.NewReader(manifest)) if err != nil { - return nil, nil, err + return nil, nil, "", err } mediaType := types.MediaType(resp.Header.Get("Content-Type")) @@ -280,7 +285,7 @@ func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType // Validate the digest matches what we asked for, if pulling by digest. if dgst, ok := ref.(name.Digest); ok { if digest.String() != dgst.DigestStr() { - return nil, nil, fmt.Errorf("manifest digest: %q does not match requested digest: %q for %q", digest, dgst.DigestStr(), f.Ref) + return nil, nil, "", fmt.Errorf("manifest digest: %q does not match requested digest: %q for %q", digest, dgst.DigestStr(), f.Ref) } } // Do nothing for tags; I give up. @@ -298,7 +303,8 @@ func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType MediaType: mediaType, } - return manifest, &desc, nil + etag := resp.Header.Get("ETag") + return manifest, &desc, etag, nil } func (f *fetcher) headManifest(ref name.Reference, acceptable []types.MediaType) (*v1.Descriptor, error) { diff --git a/pkg/v1/remote/image.go b/pkg/v1/remote/image.go index a36416d8c..c4d40645d 100644 --- a/pkg/v1/remote/image.go +++ b/pkg/v1/remote/image.go @@ -45,6 +45,7 @@ type remoteImage struct { config []byte mediaType types.MediaType descriptor *v1.Descriptor + etag string } var _ partial.CompressedImageCore = (*remoteImage)(nil) @@ -76,7 +77,7 @@ func (r *remoteImage) RawManifest() ([]byte, error) { // NOTE(jonjohnsonjr): We should never get here because the public entrypoints // do type-checking via remote.Descriptor. I've left this here for tests that // directly instantiate a remoteImage. - manifest, desc, err := r.fetchManifest(r.Ref, acceptableImageMediaTypes) + manifest, desc, _, err := r.fetchManifest(r.Ref, acceptableImageMediaTypes) if err != nil { return nil, err } diff --git a/pkg/v1/remote/index.go b/pkg/v1/remote/index.go index 989857918..16a5f6c37 100644 --- a/pkg/v1/remote/index.go +++ b/pkg/v1/remote/index.go @@ -38,6 +38,7 @@ type remoteIndex struct { manifest []byte mediaType types.MediaType descriptor *v1.Descriptor + etag string } // Index provides access to a remote index reference. @@ -75,7 +76,7 @@ func (r *remoteIndex) RawManifest() ([]byte, error) { // NOTE(jonjohnsonjr): We should never get here because the public entrypoints // do type-checking via remote.Descriptor. I've left this here for tests that // directly instantiate a remoteIndex. - manifest, desc, err := r.fetchManifest(r.Ref, acceptableIndexMediaTypes) + manifest, desc, _, err := r.fetchManifest(r.Ref, acceptableIndexMediaTypes) if err != nil { return nil, err } @@ -236,6 +237,7 @@ func (r *remoteIndex) childDescriptor(child v1.Descriptor, platform v1.Platform) var ( manifest []byte err error + etag string ) if child.Data != nil { if err := verify.Descriptor(child); err != nil { @@ -243,7 +245,7 @@ func (r *remoteIndex) childDescriptor(child v1.Descriptor, platform v1.Platform) } manifest = child.Data } else { - manifest, _, err = r.fetchManifest(ref, []types.MediaType{child.MediaType}) + manifest, _, etag, err = r.fetchManifest(ref, []types.MediaType{child.MediaType}) if err != nil { return nil, err } @@ -257,6 +259,7 @@ func (r *remoteIndex) childDescriptor(child v1.Descriptor, platform v1.Platform) Manifest: manifest, Descriptor: child, platform: platform, + etag: etag, }, nil } diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go index d412f953c..0e596fd9c 100644 --- a/pkg/v1/remote/write.go +++ b/pkg/v1/remote/write.go @@ -612,6 +612,13 @@ func (w *writer) commitManifest(ctx context.Context, t Taggable, ref name.Refere } req.Header.Set("Content-Type", string(desc.MediaType)) + if ri, ok := t.(*remoteImage); ok && ri.etag != "" { + req.Header.Set("If-Match", ri.etag) + } + if ri, ok := t.(*remoteIndex); ok && ri.etag != "" { + req.Header.Set("If-Match", ri.etag) + } + resp, err := w.client.Do(req.WithContext(ctx)) if err != nil { return err From 8eed56dd009eed6ca38d3588df1ff819d2411f39 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Tue, 14 Jun 2022 14:57:41 -0400 Subject: [PATCH 2/2] use interface --- pkg/v1/mutate/image.go | 7 +++++++ pkg/v1/partial/with.go | 10 ++++++++++ pkg/v1/remote/image.go | 2 ++ pkg/v1/remote/index.go | 2 ++ pkg/v1/remote/write.go | 7 ++----- 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/pkg/v1/mutate/image.go b/pkg/v1/mutate/image.go index 93c230e3b..150fc43c4 100644 --- a/pkg/v1/mutate/image.go +++ b/pkg/v1/mutate/image.go @@ -39,6 +39,13 @@ type image struct { digestMap map[v1.Hash]v1.Layer } +func (i *image) ETag() string { + if e, ok := i.base.(partial.WithETag); ok { + return e.ETag() + } + return "" +} + var _ v1.Image = (*image)(nil) func (i *image) MediaType() (types.MediaType, error) { diff --git a/pkg/v1/partial/with.go b/pkg/v1/partial/with.go index 3a5c61572..43cbac1f2 100644 --- a/pkg/v1/partial/with.go +++ b/pkg/v1/partial/with.go @@ -25,6 +25,16 @@ import ( "github.com/google/go-containerregistry/pkg/v1/types" ) +// WithETag defines the subset of manifest types that can expose an ETag +// returned by a registry server. +// +// If a manifest provides an ETag using this method, it will be used when +// pushing updates, to prevent race conditions. +type WithETag interface { + // ETag returns the ETag returned by remote registry implementations. + ETag() string +} + // WithRawConfigFile defines the subset of v1.Image used by these helper methods type WithRawConfigFile interface { // RawConfigFile returns the serialized bytes of this image's config file. diff --git a/pkg/v1/remote/image.go b/pkg/v1/remote/image.go index c4d40645d..8794265b1 100644 --- a/pkg/v1/remote/image.go +++ b/pkg/v1/remote/image.go @@ -60,6 +60,8 @@ func Image(ref name.Reference, options ...Option) (v1.Image, error) { return desc.Image() } +func (r *remoteImage) ETag() string { return r.etag } + func (r *remoteImage) MediaType() (types.MediaType, error) { if string(r.mediaType) != "" { return r.mediaType, nil diff --git a/pkg/v1/remote/index.go b/pkg/v1/remote/index.go index 16a5f6c37..3f819e2d7 100644 --- a/pkg/v1/remote/index.go +++ b/pkg/v1/remote/index.go @@ -51,6 +51,8 @@ func Index(ref name.Reference, options ...Option) (v1.ImageIndex, error) { return desc.ImageIndex() } +func (r *remoteIndex) ETag() string { return r.etag } + func (r *remoteIndex) MediaType() (types.MediaType, error) { if string(r.mediaType) != "" { return r.mediaType, nil diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go index 0e596fd9c..91bb14461 100644 --- a/pkg/v1/remote/write.go +++ b/pkg/v1/remote/write.go @@ -612,11 +612,8 @@ func (w *writer) commitManifest(ctx context.Context, t Taggable, ref name.Refere } req.Header.Set("Content-Type", string(desc.MediaType)) - if ri, ok := t.(*remoteImage); ok && ri.etag != "" { - req.Header.Set("If-Match", ri.etag) - } - if ri, ok := t.(*remoteIndex); ok && ri.etag != "" { - req.Header.Set("If-Match", ri.etag) + if e, ok := t.(partial.WithETag); ok && e.ETag() != "" { + req.Header.Set("If-Match", e.ETag()) } resp, err := w.client.Do(req.WithContext(ctx))