From 7b6462766639421cffcc73d24d2aa375fd2b1ceb Mon Sep 17 00:00:00 2001 From: jonjohnsonjr Date: Tue, 20 Apr 2021 09:38:54 -0700 Subject: [PATCH] Rework gcrane options (#986) This allows us to plumb a user-agent everywhere, and makes some things a bit more consistent. This is getting unwieldy, but it works. --- cmd/gcrane/cmd/copy.go | 4 ++-- cmd/gcrane/cmd/gc.go | 4 ++-- cmd/gcrane/cmd/list.go | 14 ++++++++++++-- pkg/gcrane/copy.go | 30 ++++++++++++++++-------------- pkg/gcrane/options.go | 28 +++++++++++++++++++++++++++- pkg/v1/google/list.go | 12 ++++++------ pkg/v1/google/options.go | 10 +++++----- 7 files changed, 70 insertions(+), 32 deletions(-) diff --git a/cmd/gcrane/cmd/copy.go b/cmd/gcrane/cmd/copy.go index 0dc134116..e15d17212 100644 --- a/cmd/gcrane/cmd/copy.go +++ b/cmd/gcrane/cmd/copy.go @@ -38,11 +38,11 @@ func NewCmdCopy() *cobra.Command { // We should wire this up to signal handlers and make sure we // respect the cancellation downstream. ctx := context.TODO() - if err := gcrane.CopyRepository(ctx, src, dst, gcrane.WithJobs(jobs)); err != nil { + if err := gcrane.CopyRepository(ctx, src, dst, gcrane.WithJobs(jobs), gcrane.WithUserAgent(userAgent())); err != nil { log.Fatal(err) } } else { - if err := gcrane.Copy(src, dst); err != nil { + if err := gcrane.Copy(src, dst, gcrane.WithUserAgent(userAgent())); err != nil { log.Fatal(err) } } diff --git a/cmd/gcrane/cmd/gc.go b/cmd/gcrane/cmd/gc.go index 986f68f6b..07784dce0 100644 --- a/cmd/gcrane/cmd/gc.go +++ b/cmd/gcrane/cmd/gc.go @@ -50,13 +50,13 @@ func gc(root string, recursive bool) { auth := google.WithAuthFromKeychain(gcrane.Keychain) if recursive { - if err := google.Walk(repo, printUntaggedImages, auth); err != nil { + if err := google.Walk(repo, printUntaggedImages, auth, google.WithUserAgent(userAgent())); err != nil { log.Fatalln(err) } return } - tags, err := google.List(repo, auth) + tags, err := google.List(repo, auth, google.WithUserAgent(userAgent())) if err := printUntaggedImages(repo, tags, err); err != nil { log.Fatalln(err) } diff --git a/cmd/gcrane/cmd/list.go b/cmd/gcrane/cmd/list.go index 757ea6a7a..94ca56585 100644 --- a/cmd/gcrane/cmd/list.go +++ b/cmd/gcrane/cmd/list.go @@ -18,13 +18,23 @@ import ( "encoding/json" "fmt" "log" + "path" + "github.com/google/go-containerregistry/cmd/crane/cmd" "github.com/google/go-containerregistry/pkg/gcrane" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/google" "github.com/spf13/cobra" ) +func userAgent() string { + if cmd.Version != "" { + return path.Join("gcrane", cmd.Version) + } + + return "gcrane" +} + // NewCmdList creates a new cobra.Command for the ls subcommand. func NewCmdList() *cobra.Command { recursive := false @@ -51,13 +61,13 @@ func ls(root string, recursive, j bool) { } if recursive { - if err := google.Walk(repo, printImages(j), google.WithAuthFromKeychain(gcrane.Keychain)); err != nil { + if err := google.Walk(repo, printImages(j), google.WithAuthFromKeychain(gcrane.Keychain), google.WithUserAgent(userAgent())); err != nil { log.Fatalln(err) } return } - tags, err := google.List(repo, google.WithAuthFromKeychain(gcrane.Keychain)) + tags, err := google.List(repo, google.WithAuthFromKeychain(gcrane.Keychain), google.WithUserAgent(userAgent())) if err != nil { log.Fatalln(err) } diff --git a/pkg/gcrane/copy.go b/pkg/gcrane/copy.go index 014e3077d..e16dd50d9 100644 --- a/pkg/gcrane/copy.go +++ b/pkg/gcrane/copy.go @@ -60,16 +60,17 @@ func GCRBackoff() retry.Backoff { } // Copy copies a remote image or index from src to dst. -func Copy(src, dst string) error { +func Copy(src, dst string, opts ...Option) error { + o := makeOptions(opts...) // Just reuse crane's copy logic with gcrane's credential logic. - return crane.Copy(src, dst, crane.WithAuthFromKeychain(Keychain)) + return crane.Copy(src, dst, o.crane...) } // CopyRepository copies everything from the src GCR repository to the // dst GCR repository. func CopyRepository(ctx context.Context, src, dst string, opts ...Option) error { o := makeOptions(opts...) - return recursiveCopy(ctx, src, dst, o.jobs) + return recursiveCopy(ctx, src, dst, o) } type task struct { @@ -84,9 +85,10 @@ type copier struct { dstRepo name.Repository tasks chan task + opt *options } -func newCopier(src, dst string, jobs int) (*copier, error) { +func newCopier(src, dst string, o *options) (*copier, error) { srcRepo, err := name.NewRepository(src) if err != nil { return nil, fmt.Errorf("parsing repo %q: %v", src, err) @@ -98,14 +100,14 @@ func newCopier(src, dst string, jobs int) (*copier, error) { } // A queue of size 2*jobs should keep each goroutine busy. - tasks := make(chan task, jobs*2) + tasks := make(chan task, o.jobs*2) - return &copier{srcRepo, dstRepo, tasks}, nil + return &copier{srcRepo, dstRepo, tasks, o}, nil } // recursiveCopy copies images from repo src to repo dst. -func recursiveCopy(ctx context.Context, src, dst string, jobs int) error { - c, err := newCopier(src, dst, jobs) +func recursiveCopy(ctx context.Context, src, dst string, o *options) error { + c, err := newCopier(src, dst, o) if err != nil { return err } @@ -116,7 +118,7 @@ func recursiveCopy(ctx context.Context, src, dst string, jobs int) error { logs.Warn.Printf("failed walkFn for repo %s: %v", repo, err) // If we hit an error when listing the repo, try re-listing with backoff. if err := backoffErrors(GCRBackoff(), func() error { - tags, err = google.List(repo, google.WithAuthFromKeychain(Keychain)) + tags, err = google.List(repo, o.google...) return err }); err != nil { return fmt.Errorf("failed List for repo %s: %v", repo, err) @@ -136,14 +138,14 @@ func recursiveCopy(ctx context.Context, src, dst string, jobs int) error { // Start walking the repo, enqueuing items in c.tasks. g.Go(func() error { defer close(c.tasks) - if err := google.Walk(c.srcRepo, walkFn, google.WithAuthFromKeychain(Keychain)); err != nil { + if err := google.Walk(c.srcRepo, walkFn, o.google...); err != nil { return fmt.Errorf("failed to Walk: %v", err) } return nil }) // Pull items off of c.tasks and copy the images. - for i := 0; i < jobs; i++ { + for i := 0; i < o.jobs; i++ { g.Go(func() error { for task := range c.tasks { // If we hit an error when trying to copy the images, @@ -173,7 +175,7 @@ func (c *copier) copyRepo(ctx context.Context, oldRepo name.Repository, tags *go // Figure out what we actually need to copy. want := tags.Manifests have := make(map[string]google.ManifestInfo) - haveTags, err := google.List(newRepo, google.WithAuthFromKeychain(Keychain)) + haveTags, err := google.List(newRepo, c.opt.google...) if err != nil { if !hasStatusCode(err, http.StatusNotFound) { return err @@ -233,7 +235,7 @@ func (c *copier) copyImages(ctx context.Context, t task) error { if err != nil { return err } - desc, err := remote.Get(srcRef, remote.WithAuthFromKeychain(Keychain)) + desc, err := remote.Get(srcRef, c.opt.remote...) if err != nil { return err } @@ -241,7 +243,7 @@ func (c *copier) copyImages(ctx context.Context, t task) error { for _, tag := range t.manifest.Tags[1:] { dstImg := t.newRepo.Tag(tag) - if err := remote.Tag(dstImg, desc, remote.WithAuthFromKeychain(Keychain)); err != nil { + if err := remote.Tag(dstImg, desc, c.opt.remote...); err != nil { return err } } diff --git a/pkg/gcrane/options.go b/pkg/gcrane/options.go index 34ee5a878..a868daa0a 100644 --- a/pkg/gcrane/options.go +++ b/pkg/gcrane/options.go @@ -16,18 +16,34 @@ package gcrane import ( "runtime" + + "github.com/google/go-containerregistry/pkg/crane" + "github.com/google/go-containerregistry/pkg/v1/google" + "github.com/google/go-containerregistry/pkg/v1/remote" ) // Option is a functional option for gcrane operations. type Option func(*options) type options struct { - jobs int + jobs int + remote []remote.Option + google []google.Option + crane []crane.Option } func makeOptions(opts ...Option) *options { o := &options{ jobs: runtime.GOMAXPROCS(0), + remote: []remote.Option{ + remote.WithAuthFromKeychain(Keychain), + }, + google: []google.Option{ + google.WithAuthFromKeychain(Keychain), + }, + crane: []crane.Option{ + crane.WithAuthFromKeychain(Keychain), + }, } for _, option := range opts { @@ -45,3 +61,13 @@ func WithJobs(jobs int) Option { o.jobs = jobs } } + +// WithUserAgent adds the given string to the User-Agent header for any HTTP +// requests. +func WithUserAgent(ua string) Option { + return func(o *options) { + o.remote = append(o.remote, remote.WithUserAgent(ua)) + o.google = append(o.google, google.WithUserAgent(ua)) + o.crane = append(o.crane, crane.WithUserAgent(ua)) + } +} diff --git a/pkg/v1/google/list.go b/pkg/v1/google/list.go index 4ca99b169..93ad052f7 100644 --- a/pkg/v1/google/list.go +++ b/pkg/v1/google/list.go @@ -29,9 +29,9 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) -// ListerOption is a functional option for List and Walk. +// Option is a functional option for List and Walk. // TODO: Can we somehow reuse the remote options here? -type ListerOption func(*lister) error +type Option func(*lister) error type lister struct { auth authn.Authenticator @@ -42,7 +42,7 @@ type lister struct { userAgent string } -func newLister(repo name.Repository, options ...ListerOption) (*lister, error) { +func newLister(repo name.Repository, options ...Option) (*lister, error) { l := &lister{ auth: authn.Anonymous, transport: http.DefaultTransport, @@ -195,7 +195,7 @@ type Tags struct { } // List calls /tags/list for the given repository. -func List(repo name.Repository, options ...ListerOption) (*Tags, error) { +func List(repo name.Repository, options ...Option) (*Tags, error) { l, err := newLister(repo, options...) if err != nil { return nil, err @@ -215,7 +215,7 @@ func List(repo name.Repository, options ...ListerOption) (*Tags, error) { // TODO: Do we want a SkipDir error, as in filepath.WalkFunc? type WalkFunc func(repo name.Repository, tags *Tags, err error) error -func walk(repo name.Repository, tags *Tags, walkFn WalkFunc, options ...ListerOption) error { +func walk(repo name.Repository, tags *Tags, walkFn WalkFunc, options ...Option) error { if tags == nil { // This shouldn't happen. return fmt.Errorf("tags nil for %q", repo) @@ -249,7 +249,7 @@ func walk(repo name.Repository, tags *Tags, walkFn WalkFunc, options ...ListerOp } // Walk recursively descends repositories, calling walkFn. -func Walk(root name.Repository, walkFn WalkFunc, options ...ListerOption) error { +func Walk(root name.Repository, walkFn WalkFunc, options ...Option) error { tags, err := List(root, options...) if err != nil { return walkFn(root, nil, err) diff --git a/pkg/v1/google/options.go b/pkg/v1/google/options.go index 6b6d590bc..f55d126c8 100644 --- a/pkg/v1/google/options.go +++ b/pkg/v1/google/options.go @@ -24,7 +24,7 @@ import ( // WithTransport is a functional option for overriding the default transport // on a remote image -func WithTransport(t http.RoundTripper) ListerOption { +func WithTransport(t http.RoundTripper) Option { return func(l *lister) error { l.transport = t return nil @@ -33,7 +33,7 @@ func WithTransport(t http.RoundTripper) ListerOption { // WithAuth is a functional option for overriding the default authenticator // on a remote image -func WithAuth(auth authn.Authenticator) ListerOption { +func WithAuth(auth authn.Authenticator) Option { return func(l *lister) error { l.auth = auth return nil @@ -42,7 +42,7 @@ func WithAuth(auth authn.Authenticator) ListerOption { // WithAuthFromKeychain is a functional option for overriding the default // authenticator on a remote image using an authn.Keychain -func WithAuthFromKeychain(keys authn.Keychain) ListerOption { +func WithAuthFromKeychain(keys authn.Keychain) Option { return func(l *lister) error { auth, err := keys.Resolve(l.repo.Registry) if err != nil { @@ -58,7 +58,7 @@ func WithAuthFromKeychain(keys authn.Keychain) ListerOption { // WithContext is a functional option for overriding the default // context.Context for HTTP request to list remote images -func WithContext(ctx context.Context) ListerOption { +func WithContext(ctx context.Context) Option { return func(l *lister) error { l.ctx = ctx return nil @@ -69,7 +69,7 @@ func WithContext(ctx context.Context) ListerOption { // requests. This header will also include "go-containerregistry/${version}". // // If you want to completely overwrite the User-Agent header, use WithTransport. -func WithUserAgent(ua string) ListerOption { +func WithUserAgent(ua string) Option { return func(l *lister) error { l.userAgent = ua return nil