Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remote: Collect ETag in response, optimistically send that back in future updates #1386

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/v1/mutate/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions pkg/v1/partial/with.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 15 additions & 9 deletions pkg/v1/remote/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -131,6 +133,7 @@ func get(ref name.Reference, acceptable []types.MediaType, options ...Option) (*
Manifest: b,
Descriptor: *desc,
platform: o.platform,
etag: etag,
}, nil
}

Expand Down Expand Up @@ -197,6 +200,7 @@ func (d *Descriptor) remoteImage() *remoteImage {
manifest: d.Manifest,
mediaType: d.MediaType,
descriptor: &d.Descriptor,
etag: d.etag,
}
}

Expand All @@ -206,6 +210,7 @@ func (d *Descriptor) remoteIndex() *remoteIndex {
manifest: d.Manifest,
mediaType: d.MediaType,
descriptor: &d.Descriptor,
etag: d.etag,
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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"))
Expand All @@ -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.
Expand All @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/v1/remote/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type remoteImage struct {
config []byte
mediaType types.MediaType
descriptor *v1.Descriptor
etag string
}

var _ partial.CompressedImageCore = (*remoteImage)(nil)
Expand All @@ -59,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
Expand All @@ -76,7 +79,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
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/v1/remote/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -50,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
Expand All @@ -75,7 +78,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
}
Expand Down Expand Up @@ -236,14 +239,15 @@ 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 {
return nil, err
}
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
}
Expand All @@ -257,6 +261,7 @@ func (r *remoteIndex) childDescriptor(child v1.Descriptor, platform v1.Platform)
Manifest: manifest,
Descriptor: child,
platform: platform,
etag: etag,
}, nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/v1/remote/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,10 @@ func (w *writer) commitManifest(ctx context.Context, t Taggable, ref name.Refere
}
req.Header.Set("Content-Type", string(desc.MediaType))

if e, ok := t.(partial.WithETag); ok && e.ETag() != "" {
req.Header.Set("If-Match", e.ETag())
}

resp, err := w.client.Do(req.WithContext(ctx))
if err != nil {
return err
Expand Down