From caa3dd20d369f6337a642c60eb1670ee2278a7a7 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 7 Sep 2023 08:25:42 +0000 Subject: [PATCH 1/5] perf: optmize request count for manifest and blob deletion Signed-off-by: Billy Zha --- cmd/oras/root/attach.go | 9 +++------ cmd/oras/root/blob/delete.go | 4 ++++ cmd/oras/root/manifest/delete.go | 4 ++++ cmd/oras/root/push.go | 9 +++------ internal/registryutil/auth.go | 13 +++++++++---- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index f60c9f390..e7d3d2d0f 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -26,7 +26,6 @@ import ( "oras.land/oras-go/v2" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/content/file" - "oras.land/oras-go/v2/registry/remote" "oras.land/oras-go/v2/registry/remote/auth" "oras.land/oras/cmd/oras/internal/option" "oras.land/oras/internal/graph" @@ -119,11 +118,9 @@ func runAttach(ctx context.Context, opts attachOptions) error { if err := opts.EnsureReferenceNotEmpty(); err != nil { return err } - if repo, ok := dst.(*remote.Repository); ok { - // add both pull and push scope hints for dst repository - // to save potential push-scope token requests during copy - ctx = registryutil.WithScopeHint(ctx, repo.Reference, auth.ActionPull, auth.ActionPush) - } + // add both pull and push scope hints for dst repository + // to save potential push-scope token requests during copy + ctx = registryutil.WithScopeHint(dst, ctx, auth.ActionPull, auth.ActionPush) subject, err := dst.Resolve(ctx, opts.Reference) if err != nil { return err diff --git a/cmd/oras/root/blob/delete.go b/cmd/oras/root/blob/delete.go index e1fbb09ac..1363842d3 100644 --- a/cmd/oras/root/blob/delete.go +++ b/cmd/oras/root/blob/delete.go @@ -23,7 +23,9 @@ import ( "github.com/spf13/cobra" "oras.land/oras-go/v2/errdef" + "oras.land/oras-go/v2/registry/remote/auth" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/internal/registryutil" ) type deleteBlobOptions struct { @@ -81,6 +83,8 @@ func deleteBlob(ctx context.Context, opts deleteBlobOptions) (err error) { return fmt.Errorf("%s: blob reference must be of the form ", opts.targetRef) } + // add both pull and delete scope hints for dst repository to save potential delete-scope token requests during deleting + ctx = registryutil.WithScopeHint(repo, ctx, auth.ActionPull, auth.ActionDelete) blobs := repo.Blobs() desc, err := blobs.Resolve(ctx, opts.targetRef) if err != nil { diff --git a/cmd/oras/root/manifest/delete.go b/cmd/oras/root/manifest/delete.go index 4458fae77..4a780df21 100644 --- a/cmd/oras/root/manifest/delete.go +++ b/cmd/oras/root/manifest/delete.go @@ -23,8 +23,10 @@ import ( "github.com/spf13/cobra" "oras.land/oras-go/v2/errdef" + "oras.land/oras-go/v2/registry/remote/auth" oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/internal/registryutil" ) type deleteOptions struct { @@ -86,6 +88,8 @@ func deleteManifest(ctx context.Context, opts deleteOptions) error { return oerrors.NewErrInvalidReference(repo.Reference) } + // add both pull, push and delete scope hints for dst repository to save potential delete-scope token requests during deleting + ctx = registryutil.WithScopeHint(repo, ctx, auth.ActionPull, auth.ActionPush, auth.ActionDelete) manifests := repo.Manifests() desc, err := manifests.Resolve(ctx, opts.targetRef) if err != nil { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 81ecf853a..0ae8792a5 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -28,7 +28,6 @@ import ( "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/content/file" "oras.land/oras-go/v2/content/memory" - "oras.land/oras-go/v2/registry/remote" "oras.land/oras-go/v2/registry/remote/auth" "oras.land/oras/cmd/oras/internal/display" "oras.land/oras/cmd/oras/internal/fileref" @@ -188,11 +187,9 @@ func runPush(ctx context.Context, opts pushOptions) error { union := contentutil.MultiReadOnlyTarget(memoryStore, store) updateDisplayOption(©Options.CopyGraphOptions, union, opts.Verbose) copy := func(root ocispec.Descriptor) error { - if repo, ok := dst.(*remote.Repository); ok { - // add both pull and push scope hints for dst repository - // to save potential push-scope token requests during copy - ctx = registryutil.WithScopeHint(ctx, repo.Reference, auth.ActionPull, auth.ActionPush) - } + // add both pull and push scope hints for dst repository + // to save potential push-scope token requests during copy + ctx = registryutil.WithScopeHint(dst, ctx, auth.ActionPull, auth.ActionPush) if tag := opts.Reference; tag == "" { err = oras.CopyGraph(ctx, union, dst, root, copyOptions.CopyGraphOptions) diff --git a/internal/registryutil/auth.go b/internal/registryutil/auth.go index 4a601f0c6..5c47b613b 100644 --- a/internal/registryutil/auth.go +++ b/internal/registryutil/auth.go @@ -18,12 +18,17 @@ package registryutil import ( "context" - "oras.land/oras-go/v2/registry" + "oras.land/oras-go/v2" + "oras.land/oras-go/v2/registry/remote" "oras.land/oras-go/v2/registry/remote/auth" ) // WithScopeHint adds a hinted scope to the context. -func WithScopeHint(ctx context.Context, ref registry.Reference, actions ...string) context.Context { - scope := auth.ScopeRepository(ref.Repository, actions...) - return auth.AppendScopes(ctx, scope) +func WithScopeHint(target oras.Target, ctx context.Context, actions ...string) context.Context { + if repo, ok := target.(*remote.Repository); ok { + scope := auth.ScopeRepository(repo.Reference.Repository, actions...) + return auth.AppendScopes(ctx, scope) + } else { + return ctx + } } From 80b654fcc4dc72b093086916865f1668bc2e8395 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 07:54:47 +0000 Subject: [PATCH 2/5] obtain smaller scope for `manifest delete` Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 18 +++++++++--------- cmd/oras/internal/option/spec.go | 18 +++++++++--------- cmd/oras/root/manifest/delete.go | 7 ++++++- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index c56282cc2..fd263b38b 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -51,11 +51,11 @@ type Remote struct { resolveFlag []string applyDistributionSpec bool - distributionSpec distributionSpec - headerFlags []string - headers http.Header - warned map[string]*sync.Map - plainHTTP func() (plainHTTP bool, enforced bool) + DistributionSpec + headerFlags []string + headers http.Header + warned map[string]*sync.Map + plainHTTP func() (plainHTTP bool, enforced bool) } // EnableDistributionSpecFlag set distribution specification flag as applicable. @@ -93,7 +93,7 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description flagPrefix, notePrefix = applyPrefix(prefix, description) if opts.applyDistributionSpec { - opts.distributionSpec.ApplyFlagsWithPrefix(fs, prefix, description) + opts.DistributionSpec.ApplyFlagsWithPrefix(fs, prefix, description) } fs.StringVarP(&opts.Username, flagPrefix+"username", shortUser, "", notePrefix+"registry username") fs.StringVarP(&opts.Password, flagPrefix+"password", shortPassword, "", notePrefix+"registry password or identity token") @@ -117,7 +117,7 @@ func (opts *Remote) Parse() error { if err := opts.readPassword(); err != nil { return err } - return opts.distributionSpec.Parse() + return opts.DistributionSpec.Parse() } // readPassword tries to read password with optional cmd prompt. @@ -299,8 +299,8 @@ func (opts *Remote) NewRepository(reference string, common Common, logger logrus return nil, err } repo.SkipReferrersGC = true - if opts.distributionSpec.referrersAPI != nil { - if err := repo.SetReferrersCapability(*opts.distributionSpec.referrersAPI); err != nil { + if opts.ReferrersAPI != nil { + if err := repo.SetReferrersCapability(*opts.ReferrersAPI); err != nil { return nil, err } } diff --git a/cmd/oras/internal/option/spec.go b/cmd/oras/internal/option/spec.go index 837959761..965eb929b 100644 --- a/cmd/oras/internal/option/spec.go +++ b/cmd/oras/internal/option/spec.go @@ -51,27 +51,27 @@ func (opts *ImageSpec) ApplyFlags(fs *pflag.FlagSet) { fs.StringVar(&opts.flag, "image-spec", ImageSpecV1_1, fmt.Sprintf("[Experimental] specify manifest type for building artifact. options: %s, %s", ImageSpecV1_1, ImageSpecV1_0)) } -// distributionSpec option struct. -type distributionSpec struct { - // referrersAPI indicates the preference of the implementation of the Referrers API. +// DistributionSpec option struct. +type DistributionSpec struct { + // ReferrersAPI indicates the preference of the implementation of the Referrers API. // Set to true for referrers API, false for referrers tag scheme, and nil for auto fallback. - referrersAPI *bool + ReferrersAPI *bool // specFlag should be provided in form of`--