Skip to content

Commit

Permalink
Refactor fetcher, writer, and progress (#1625)
Browse files Browse the repository at this point in the history
This will allow reuse across a repository.

One major difference is that keychains are no longer resolved within
the option execution, but lazily during fetcher/writer construction,
which enables us to create options without knowing the repo.
  • Loading branch information
jonjohnsonjr committed Apr 6, 2023
1 parent 249d7e1 commit 6ac92e8
Show file tree
Hide file tree
Showing 15 changed files with 179 additions and 185 deletions.
24 changes: 7 additions & 17 deletions pkg/v1/remote/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,32 +31,27 @@ type catalog struct {

// CatalogPage calls /_catalog, returning the list of repositories on the registry.
func CatalogPage(target name.Registry, last string, n int, options ...Option) ([]string, error) {
o, err := makeOptions(target, options...)
o, err := makeOptions(options...)
if err != nil {
return nil, err
}

scopes := []string{target.Scope(transport.PullScope)}
tr, err := transport.NewWithContext(o.context, target, o.auth, o.transport, scopes)
f, err := makeFetcher(o.context, target, o)
if err != nil {
return nil, err
}

query := fmt.Sprintf("last=%s&n=%d", url.QueryEscape(last), n)

uri := url.URL{
Scheme: target.Scheme(),
Host: target.RegistryStr(),
Path: "/v2/_catalog",
RawQuery: query,
RawQuery: fmt.Sprintf("last=%s&n=%d", url.QueryEscape(last), n),
}

client := http.Client{Transport: tr}
req, err := http.NewRequest(http.MethodGet, uri.String(), nil)
if err != nil {
return nil, err
}
resp, err := client.Do(req.WithContext(o.context))
resp, err := f.client.Do(req.WithContext(o.context))
if err != nil {
return nil, err
}
Expand All @@ -76,13 +71,11 @@ func CatalogPage(target name.Registry, last string, n int, options ...Option) ([

// Catalog calls /_catalog, returning the list of repositories on the registry.
func Catalog(ctx context.Context, target name.Registry, options ...Option) ([]string, error) {
o, err := makeOptions(target, options...)
o, err := makeOptions(options...)
if err != nil {
return nil, err
}

scopes := []string{target.Scope(transport.PullScope)}
tr, err := transport.NewWithContext(o.context, target, o.auth, o.transport, scopes)
f, err := makeFetcher(o.context, target, o)
if err != nil {
return nil, err
}
Expand All @@ -92,13 +85,10 @@ func Catalog(ctx context.Context, target name.Registry, options ...Option) ([]st
Host: target.RegistryStr(),
Path: "/v2/_catalog",
}

if o.pageSize > 0 {
uri.RawQuery = fmt.Sprintf("n=%d", o.pageSize)
}

client := http.Client{Transport: tr}

// WithContext overrides the ctx passed directly.
if o.context != context.Background() {
ctx = o.context
Expand All @@ -123,7 +113,7 @@ func Catalog(ctx context.Context, target name.Registry, options ...Option) ([]st
}
req = req.WithContext(ctx)

resp, err := client.Do(req)
resp, err := f.client.Do(req)
if err != nil {
return nil, err
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/v1/remote/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,15 @@ import (

// Delete removes the specified image reference from the remote registry.
func Delete(ref name.Reference, options ...Option) error {
o, err := makeOptions(ref.Context(), options...)
o, err := makeOptions(options...)
if err != nil {
return err
}
scopes := []string{ref.Scope(transport.DeleteScope)}
tr, err := transport.NewWithContext(o.context, ref.Context().Registry, o.auth, o.transport, scopes)
w, err := makeWriter(o.context, ref.Context(), nil, o)
if err != nil {
return err
}
c := &http.Client{Transport: tr}
c := w.client

u := url.URL{
Scheme: ref.Context().Registry.Scheme(),
Expand Down
95 changes: 66 additions & 29 deletions pkg/v1/remote/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/google/go-containerregistry/internal/redact"
"github.com/google/go-containerregistry/internal/verify"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/logs"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
Expand Down Expand Up @@ -59,6 +60,8 @@ func (e *ErrSchema1) Error() string {
type Descriptor struct {
fetcher
v1.Descriptor

ref name.Reference
Manifest []byte

// So we can share this implementation with Image.
Expand Down Expand Up @@ -100,36 +103,37 @@ func Head(ref name.Reference, options ...Option) (*v1.Descriptor, error) {
acceptable = append(acceptable, acceptableImageMediaTypes...)
acceptable = append(acceptable, acceptableIndexMediaTypes...)

o, err := makeOptions(ref.Context(), options...)
o, err := makeOptions(options...)
if err != nil {
return nil, err
}

f, err := makeFetcher(ref, o)
f, err := makeFetcher(o.context, ref.Context(), o)
if err != nil {
return nil, err
}

return f.headManifest(ref, acceptable)
return f.headManifest(o.context, ref, acceptable)
}

// Handle options and fetch the manifest with the acceptable MediaTypes in the
// Accept header.
func get(ref name.Reference, acceptable []types.MediaType, options ...Option) (*Descriptor, error) {
o, err := makeOptions(ref.Context(), options...)
o, err := makeOptions(options...)
if err != nil {
return nil, err
}
f, err := makeFetcher(ref, o)
f, err := makeFetcher(o.context, ref.Context(), o)
if err != nil {
return nil, err
}
b, desc, err := f.fetchManifest(ref, acceptable)
b, desc, err := f.fetchManifest(o.context, ref, acceptable)
if err != nil {
return nil, err
}
return &Descriptor{
fetcher: *f,
ref: ref,
Manifest: b,
Descriptor: *desc,
platform: o.platform,
Expand Down Expand Up @@ -169,7 +173,7 @@ func (d *Descriptor) Image() (v1.Image, error) {
}
return &mountableImage{
Image: imgCore,
Reference: d.Ref,
Reference: d.ref,
}, nil
}

Expand All @@ -196,6 +200,7 @@ func (d *Descriptor) ImageIndex() (v1.ImageIndex, error) {
func (d *Descriptor) remoteImage() *remoteImage {
return &remoteImage{
fetcher: d.fetcher,
ref: d.ref,
manifest: d.Manifest,
mediaType: d.MediaType,
descriptor: &d.Descriptor,
Expand All @@ -205,38 +210,70 @@ func (d *Descriptor) remoteImage() *remoteImage {
func (d *Descriptor) remoteIndex() *remoteIndex {
return &remoteIndex{
fetcher: d.fetcher,
ref: d.ref,
manifest: d.Manifest,
mediaType: d.MediaType,
descriptor: &d.Descriptor,
}
}

type resource interface {
Scheme() string
RegistryStr() string
Scope(string) string

authn.Resource
}

// fetcher implements methods for reading from a registry.
type fetcher struct {
Ref name.Reference
Client *http.Client
target resource
client *http.Client
context context.Context
}

func makeFetcher(ref name.Reference, o *options) (*fetcher, error) {
tr, err := transport.NewWithContext(o.context, ref.Context().Registry, o.auth, o.transport, []string{ref.Scope(transport.PullScope)})
func makeFetcher(ctx context.Context, target resource, o *options) (*fetcher, error) {
auth := o.auth
if o.keychain != nil {
kauth, err := o.keychain.Resolve(target)
if err != nil {
return nil, err
}
auth = kauth
}

reg, ok := target.(name.Registry)
if !ok {
repo, ok := target.(name.Repository)
if !ok {
return nil, fmt.Errorf("unexpected resource: %T", target)
}
reg = repo.Registry
}

tr, err := transport.NewWithContext(ctx, reg, auth, o.transport, []string{target.Scope(transport.PullScope)})
if err != nil {
return nil, err
}
return &fetcher{
Ref: ref,
Client: &http.Client{Transport: tr},
context: o.context,
target: target,
client: &http.Client{Transport: tr},
context: ctx,
}, nil
}

// url returns a url.Url for the specified path in the context of this remote image reference.
func (f *fetcher) url(resource, identifier string) url.URL {
return url.URL{
Scheme: f.Ref.Context().Registry.Scheme(),
Host: f.Ref.Context().RegistryStr(),
Path: fmt.Sprintf("/v2/%s/%s/%s", f.Ref.Context().RepositoryStr(), resource, identifier),
u := url.URL{
Scheme: f.target.Scheme(),
Host: f.target.RegistryStr(),
// Default path if this is not a repository.
Path: "/v2/_catalog",
}
if repo, ok := f.target.(name.Repository); ok {
u.Path = fmt.Sprintf("/v2/%s/%s/%s", repo.RepositoryStr(), resource, identifier)
}
return u
}

// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#referrers-tag-schema
Expand All @@ -253,7 +290,7 @@ func (f *fetcher) fetchReferrers(ctx context.Context, filter map[string]string,
}
req.Header.Set("Accept", string(types.OCIImageIndex))

resp, err := f.Client.Do(req)
resp, err := f.client.Do(req)
if err != nil {
return nil, err
}
Expand All @@ -271,7 +308,7 @@ func (f *fetcher) fetchReferrers(ctx context.Context, filter map[string]string,
}

// The registry doesn't support the Referrers API endpoint, so we'll use the fallback tag scheme.
b, _, err := f.fetchManifest(fallbackTag(d), []types.MediaType{types.OCIImageIndex})
b, _, err := f.fetchManifest(ctx, fallbackTag(d), []types.MediaType{types.OCIImageIndex})
if err != nil {
return nil, err
}
Expand All @@ -289,7 +326,7 @@ func (f *fetcher) fetchReferrers(ctx context.Context, filter map[string]string,
return filterReferrersResponse(filter, &im), nil
}

func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType) ([]byte, *v1.Descriptor, error) {
func (f *fetcher) fetchManifest(ctx context.Context, ref name.Reference, acceptable []types.MediaType) ([]byte, *v1.Descriptor, error) {
u := f.url("manifests", ref.Identifier())
req, err := http.NewRequest(http.MethodGet, u.String(), nil)
if err != nil {
Expand All @@ -301,7 +338,7 @@ func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType
}
req.Header.Set("Accept", strings.Join(accept, ","))

resp, err := f.Client.Do(req.WithContext(f.context))
resp, err := f.client.Do(req.WithContext(ctx))
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -332,7 +369,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(), ref)
}
}

Expand Down Expand Up @@ -363,7 +400,7 @@ func (f *fetcher) fetchManifest(ref name.Reference, acceptable []types.MediaType
return manifest, &desc, nil
}

func (f *fetcher) headManifest(ref name.Reference, acceptable []types.MediaType) (*v1.Descriptor, error) {
func (f *fetcher) headManifest(ctx context.Context, ref name.Reference, acceptable []types.MediaType) (*v1.Descriptor, error) {
u := f.url("manifests", ref.Identifier())
req, err := http.NewRequest(http.MethodHead, u.String(), nil)
if err != nil {
Expand All @@ -375,7 +412,7 @@ func (f *fetcher) headManifest(ref name.Reference, acceptable []types.MediaType)
}
req.Header.Set("Accept", strings.Join(accept, ","))

resp, err := f.Client.Do(req.WithContext(f.context))
resp, err := f.client.Do(req.WithContext(ctx))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -408,7 +445,7 @@ func (f *fetcher) headManifest(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, fmt.Errorf("manifest digest: %q does not match requested digest: %q for %q", digest, dgst.DigestStr(), f.Ref)
return nil, fmt.Errorf("manifest digest: %q does not match requested digest: %q for %q", digest, dgst.DigestStr(), ref)
}
}

Expand All @@ -427,7 +464,7 @@ func (f *fetcher) fetchBlob(ctx context.Context, size int64, h v1.Hash) (io.Read
return nil, err
}

resp, err := f.Client.Do(req.WithContext(ctx))
resp, err := f.client.Do(req.WithContext(ctx))
if err != nil {
return nil, redact.Error(err)
}
Expand Down Expand Up @@ -458,7 +495,7 @@ func (f *fetcher) headBlob(h v1.Hash) (*http.Response, error) {
return nil, err
}

resp, err := f.Client.Do(req.WithContext(f.context))
resp, err := f.client.Do(req.WithContext(f.context))
if err != nil {
return nil, redact.Error(err)
}
Expand All @@ -478,7 +515,7 @@ func (f *fetcher) blobExists(h v1.Hash) (bool, error) {
return false, err
}

resp, err := f.Client.Do(req.WithContext(f.context))
resp, err := f.client.Do(req.WithContext(f.context))
if err != nil {
return false, redact.Error(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/v1/remote/descriptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ func TestHead_MissingHeaders(t *testing.T) {
func TestRedactFetchBlob(t *testing.T) {
ctx := context.Background()
f := fetcher{
Ref: mustNewTag(t, "original.com/repo:latest"),
Client: &http.Client{
target: mustNewTag(t, "original.com/repo:latest").Context(),
client: &http.Client{
Transport: errTransport{},
},
context: ctx,
Expand Down
5 changes: 3 additions & 2 deletions pkg/v1/remote/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ var acceptableImageMediaTypes = []types.MediaType{
// remoteImage accesses an image from a remote registry
type remoteImage struct {
fetcher
ref name.Reference
manifestLock sync.Mutex // Protects manifest
manifest []byte
configLock sync.Mutex // Protects config
Expand Down Expand Up @@ -84,7 +85,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.context, r.ref, acceptableImageMediaTypes)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -186,7 +187,7 @@ func (rl *remoteImageLayer) Compressed() (io.ReadCloser, error) {
return nil, err
}

resp, err := rl.ri.Client.Do(req.WithContext(ctx))
resp, err := rl.ri.client.Do(req.WithContext(ctx))
if err != nil {
lastErr = err
continue
Expand Down
Loading

0 comments on commit 6ac92e8

Please sign in to comment.