Skip to content

Commit

Permalink
Merge pull request #1109 from qweeah/qweeah/optmize-delete
Browse files Browse the repository at this point in the history
perf: optimize request count for manifest and blob deletion
  • Loading branch information
qweeah authored Sep 12, 2023
2 parents 7a822cc + ad77f3c commit 6609ecb
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 30 deletions.
10 changes: 5 additions & 5 deletions cmd/oras/internal/option/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (

// Remote options struct.
type Remote struct {
DistributionSpec
CACertFilePath string
Insecure bool
Configs []string
Expand All @@ -51,7 +52,6 @@ type Remote struct {

resolveFlag []string
applyDistributionSpec bool
distributionSpec distributionSpec
headerFlags []string
headers http.Header
warned map[string]*sync.Map
Expand Down Expand Up @@ -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")
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
}
Expand Down
18 changes: 9 additions & 9 deletions cmd/oras/internal/option/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
9 changes: 3 additions & 6 deletions cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(ctx, dst, auth.ActionPull, auth.ActionPush)
subject, err := dst.Resolve(ctx, opts.Reference)
if err != nil {
return err
Expand Down
4 changes: 4 additions & 0 deletions cmd/oras/root/blob/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -81,6 +83,8 @@ 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 {
Expand Down
9 changes: 9 additions & 0 deletions cmd/oras/root/manifest/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -86,6 +88,13 @@ 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 {
Expand Down
9 changes: 3 additions & 6 deletions cmd/oras/root/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -188,11 +187,9 @@ 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 {
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(ctx, dst, auth.ActionPull, auth.ActionPush)

if tag := opts.Reference; tag == "" {
err = oras.CopyGraph(ctx, union, dst, root, copyOptions.CopyGraphOptions)
Expand Down
12 changes: 8 additions & 4 deletions internal/registryutil/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,16 @@ 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(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
}

0 comments on commit 6609ecb

Please sign in to comment.