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

Refactor fetcher/writer constructors #1625

Merged
merged 1 commit into from
Apr 6, 2023
Merged
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
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