revert: "perf: optimize request count for manifest and blob deletion" (…

…#1115)

Signed-off-by: Billy Zha <[email protected]>

cmd/oras/internal/option/remote.go

@@ -42,7 +42,6 @@ import (
 
 // Remote options struct.
 type Remote struct {
-	DistributionSpec
 	CACertFilePath    string
 	Insecure          bool
 	Configs           []string
@@ -52,6 +51,7 @@ type Remote struct {
 
 	resolveFlag           []string
 	applyDistributionSpec bool
+	distributionSpec      distributionSpec
 	headerFlags           []string
 	headers               http.Header
 	warned                map[string]*sync.Map
@@ -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.ReferrersAPI != nil {
-		if err := repo.SetReferrersCapability(*opts.ReferrersAPI); err != nil {
+	if opts.distributionSpec.referrersAPI != nil {
+		if err := repo.SetReferrersCapability(*opts.distributionSpec.referrersAPI); err != nil {
 			return nil, err
 		}
 	}

cmd/oras/internal/option/spec.go

@@ -51,35 +51,35 @@ 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`<version>-<api>-<option>`
 	specFlag string
 }
 
 // Parse parses flags into the option.
-func (opts *DistributionSpec) Parse() error {
+func (opts *distributionSpec) Parse() error {
 	switch opts.specFlag {
 	case "":
-		opts.ReferrersAPI = nil
+		opts.referrersAPI = nil
 	case "v1.1-referrers-tag":
 		isApi := false
-		opts.ReferrersAPI = &isApi
+		opts.referrersAPI = &isApi
 	case "v1.1-referrers-api":
 		isApi := true
-		opts.ReferrersAPI = &isApi
+		opts.referrersAPI = &isApi
 	default:
 		return fmt.Errorf("unknown distribution specification flag: %q", opts.specFlag)
 	}
 	return nil
 }
 
 // ApplyFlagsWithPrefix applies flags to a command flag set with a prefix string.
-func (opts *DistributionSpec) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description string) {
+func (opts *distributionSpec) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description string) {
 	flagPrefix, notePrefix := applyPrefix(prefix, description)
 	fs.StringVar(&opts.specFlag, flagPrefix+"distribution-spec", "", "[Preview] set OCI distribution spec version and API option for "+notePrefix+"target. options: v1.1-referrers-api, v1.1-referrers-tag")
 }

cmd/oras/root/attach.go

@@ -26,6 +26,7 @@ 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"
@@ -118,9 +119,11 @@ func runAttach(ctx context.Context, opts attachOptions) error {
 	if err := opts.EnsureReferenceNotEmpty(); err != nil {
 		return err
 	}
-	// add both pull and push scope hints for dst repository
-	// to save potential push-scope token requests during copy
-	ctx = registryutil.WithScopeHint(ctx, dst, auth.ActionPull, auth.ActionPush)
+	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)
+	}
 	subject, err := dst.Resolve(ctx, opts.Reference)
 	if err != nil {
 		return err

cmd/oras/root/blob/delete.go

@@ -23,9 +23,7 @@ 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 {
@@ -83,8 +81,6 @@ func deleteBlob(ctx context.Context, opts deleteBlobOptions) (err error) {
 		return fmt.Errorf("%s: blob reference must be of the form <name@digest>", opts.targetRef)
 	}
 
-	// add both pull and delete scope hints for dst repository to save potential delete-scope token requests during deleting
-	ctx = registryutil.WithScopeHint(ctx, repo, auth.ActionPull, auth.ActionDelete)
 	blobs := repo.Blobs()
 	desc, err := blobs.Resolve(ctx, opts.targetRef)
 	if err != nil {

cmd/oras/root/manifest/delete.go

@@ -23,10 +23,8 @@ 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 {
@@ -88,13 +86,6 @@ func deleteManifest(ctx context.Context, opts deleteOptions) error {
 		return oerrors.NewErrInvalidReference(repo.Reference)
 	}
 
-	// add both pull and delete scope hints for dst repository to save potential delete-scope token requests during deleting
-	hints := []string{auth.ActionPull, auth.ActionDelete}
-	if opts.ReferrersAPI == nil || !*opts.ReferrersAPI {
-		// possibly needed when adding a new referrers index
-		hints = append(hints, auth.ActionPush)
-	}
-	ctx = registryutil.WithScopeHint(ctx, repo, hints...)
 	manifests := repo.Manifests()
 	desc, err := manifests.Resolve(ctx, opts.targetRef)
 	if err != nil {

cmd/oras/root/push.go

@@ -28,6 +28,7 @@ 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"
@@ -187,9 +188,11 @@ func runPush(ctx context.Context, opts pushOptions) error {
 	union := contentutil.MultiReadOnlyTarget(memoryStore, store)
 	updateDisplayOption(&copyOptions.CopyGraphOptions, union, opts.Verbose)
 	copy := func(root ocispec.Descriptor) error {
-		// add both pull and push scope hints for dst repository
-		// to save potential push-scope token requests during copy
-		ctx = registryutil.WithScopeHint(ctx, dst, auth.ActionPull, auth.ActionPush)
+		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)
+		}
 
 		if tag := opts.Reference; tag == "" {
 			err = oras.CopyGraph(ctx, union, dst, root, copyOptions.CopyGraphOptions)

internal/registryutil/auth.go

@@ -18,16 +18,12 @@ package registryutil
 import (
 	"context"
 
-	"oras.land/oras-go/v2"
-	"oras.land/oras-go/v2/registry/remote"
+	"oras.land/oras-go/v2/registry"
 	"oras.land/oras-go/v2/registry/remote/auth"
 )
 
 // WithScopeHint adds a hinted scope to the context.
-func WithScopeHint(ctx context.Context, target oras.Target, actions ...string) context.Context {
-	if repo, ok := target.(*remote.Repository); ok {
-		scope := auth.ScopeRepository(repo.Reference.Repository, actions...)
-		return auth.AppendScopes(ctx, scope)
-	}
-	return ctx
+func WithScopeHint(ctx context.Context, ref registry.Reference, actions ...string) context.Context {
+	scope := auth.ScopeRepository(ref.Repository, actions...)
+	return auth.AppendScopes(ctx, scope)
 }