Skip to content

Commit

Permalink
chore(ux): give recommendation when empty creds found during basic au…
Browse files Browse the repository at this point in the history
…th (#1235)

Signed-off-by: Billy Zha <[email protected]>
  • Loading branch information
qweeah authored Jan 19, 2024
1 parent 0514a57 commit 812bb59
Show file tree
Hide file tree
Showing 27 changed files with 136 additions and 54 deletions.
38 changes: 31 additions & 7 deletions cmd/oras/internal/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ limitations under the License.
package errors

import (
"errors"
"fmt"
"strings"

"github.com/spf13/cobra"
"oras.land/oras-go/v2/registry/remote/auth"
"oras.land/oras-go/v2/registry/remote/errcode"
)

Expand Down Expand Up @@ -83,8 +85,8 @@ func Command(cmd *cobra.Command, handler Modifier) *cobra.Command {
return cmd
}

// Trim tries to trim toTrim from err.
func Trim(err error, toTrim error) error {
// TrimErrResp tries to trim toTrim from err.
func TrimErrResp(err error, toTrim error) error {
var inner error
if errResp, ok := toTrim.(*errcode.ErrorResponse); ok {
if len(errResp.Errors) == 0 {
Expand All @@ -94,14 +96,36 @@ func Trim(err error, toTrim error) error {
} else {
return err
}
return reWrap(err, toTrim, inner)
}

if rewrapped := reWrap(err, toTrim, inner); rewrapped != nil {
return rewrapped
// TrimErrBasicCredentialNotFound trims the credentials from err.
// Caller should make sure the err is auth.ErrBasicCredentialNotFound.
func TrimErrBasicCredentialNotFound(err error) error {
toTrim := err
inner := err
for {
switch x := inner.(type) {
case interface{ Unwrap() error }:
toTrim = inner
inner = x.Unwrap()
continue
case interface{ Unwrap() []error }:
for _, errItem := range x.Unwrap() {
if errors.Is(errItem, auth.ErrBasicCredentialNotFound) {
toTrim = errItem
inner = errItem
break
}
}
continue
}
break
}
return inner
return reWrap(err, toTrim, auth.ErrBasicCredentialNotFound)
}

// reWrap re-wraps errA to errC and trims out errB, returns nil if scrub fails.
// reWrap re-wraps errA to errC and trims out errB, returns errC if scrub fails.
// +---------- errA ----------+
// | +---- errB ----+ | +---- errA ----+
// | | errC | | => | errC |
Expand All @@ -115,7 +139,7 @@ func reWrap(errA, errB, errC error) error {
if idx := strings.Index(contentA, contentB); idx > 0 {
return fmt.Errorf("%s%w", contentA[:idx], errC)
}
return nil
return errC
}

// NewErrEmptyTagOrDigest creates a new error based on the reference string.
Expand Down
38 changes: 34 additions & 4 deletions cmd/oras/internal/option/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ import (
"strings"
"sync"

credentials "github.com/oras-project/oras-credentials-go"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"oras.land/oras-go/v2/errdef"
"oras.land/oras-go/v2/registry/remote"
"oras.land/oras-go/v2/registry/remote/auth"
"oras.land/oras-go/v2/registry/remote/credentials"
"oras.land/oras-go/v2/registry/remote/errcode"
"oras.land/oras-go/v2/registry/remote/retry"
oerrors "oras.land/oras/cmd/oras/internal/errors"
Expand Down Expand Up @@ -62,6 +62,7 @@ type Remote struct {
headers http.Header
warned map[string]*sync.Map
plainHTTP func() (plainHTTP bool, enforced bool)
store credentials.Store
}

// EnableDistributionSpecFlag set distribution specification flag as applicable.
Expand Down Expand Up @@ -226,15 +227,27 @@ func (opts *Remote) authClient(registry string, debug bool) (client *auth.Client
return cred, nil
}
} else {
store, err := credential.NewStore(opts.Configs...)
var err error
opts.store, err = credential.NewStore(opts.Configs...)
if err != nil {
return nil, err
}
client.Credential = credentials.Credential(store)
client.Credential = credentials.Credential(opts.store)
}
return
}

// ConfigPath returns the config path of the credential store.
func (opts *Remote) ConfigPath() (string, error) {
if opts.store == nil {
return "", errors.New("no credential store initialized")
}
if ds, ok := opts.store.(*credentials.DynamicStore); ok {
return ds.ConfigPath(), nil
}
return "", errors.New("store doesn't support getting config path")
}

func (opts *Remote) parseCustomHeaders() error {
if len(opts.headerFlags) != 0 {
headers := map[string][]string{}
Expand Down Expand Up @@ -330,11 +343,28 @@ func (opts *Remote) isPlainHttp(registry string) bool {
// Modify modifies error during cmd execution.
func (opts *Remote) Modify(cmd *cobra.Command, err error) (error, bool) {
var errResp *errcode.ErrorResponse

if errors.Is(err, auth.ErrBasicCredentialNotFound) {
return opts.DecorateCredentialError(err), true
}

if errors.As(err, &errResp) {
cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
return &oerrors.Error{
Err: oerrors.Trim(err, errResp),
Err: oerrors.TrimErrResp(err, errResp),
}, true
}
return err, false
}

// DecorateCredentialError decorate error with recommendation.
func (opts *Remote) DecorateCredentialError(err error) *oerrors.Error {
configPath := " "
if path, pathErr := opts.ConfigPath(); pathErr == nil {
configPath += fmt.Sprintf("at %q ", path)
}
return &oerrors.Error{
Err: oerrors.TrimErrBasicCredentialNotFound(err),
Recommendation: fmt.Sprintf(`Please check whether the registry credential stored in the authentication file%sis correct`, configPath),
}
}
7 changes: 6 additions & 1 deletion cmd/oras/internal/option/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"oras.land/oras-go/v2/errdef"
"oras.land/oras-go/v2/registry"
"oras.land/oras-go/v2/registry/remote"
"oras.land/oras-go/v2/registry/remote/auth"
"oras.land/oras-go/v2/registry/remote/errcode"
oerrors "oras.land/oras/cmd/oras/internal/errors"
"oras.land/oras/cmd/oras/internal/fileref"
Expand Down Expand Up @@ -250,6 +251,10 @@ func (opts *Target) Modify(cmd *cobra.Command, err error) (error, bool) {
return err, false
}

if errors.Is(err, auth.ErrBasicCredentialNotFound) {
return opts.DecorateCredentialError(err), true
}

if errors.Is(err, errdef.ErrNotFound) {
cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
return err, true
Expand All @@ -274,7 +279,7 @@ func (opts *Target) Modify(cmd *cobra.Command, err error) (error, bool) {

cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
ret := &oerrors.Error{
Err: oerrors.Trim(err, errResp),
Err: oerrors.TrimErrResp(err, errResp),
}

if ref.Registry == "docker.io" && errResp.StatusCode == http.StatusUnauthorized {
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
return runAttach(cmd, opts)
return runAttach(cmd, &opts)
},
}

Expand All @@ -99,7 +99,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
return oerrors.Command(cmd, &opts.Target)
}

func runAttach(cmd *cobra.Command, opts attachOptions) error {
func runAttach(cmd *cobra.Command, opts *attachOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
annotations, err := opts.LoadManifestAnnotations()
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/blob/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ Example - Delete a blob and print its descriptor:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return deleteBlob(cmd, opts)
return deleteBlob(cmd, &opts)
},
}

option.ApplyFlags(&opts, cmd.Flags())
return oerrors.Command(cmd, &opts.Target)
}

func deleteBlob(cmd *cobra.Command, opts deleteBlobOptions) (err error) {
func deleteBlob(cmd *cobra.Command, opts *deleteBlobOptions) (err error) {
ctx, logger := opts.WithContext(cmd.Context())
blobs, err := opts.NewBlobDeleter(opts.Common, logger)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/blob/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Example - Fetch and print a blob from OCI image layout archive file 'layout.tar'
},
Aliases: []string{"get"},
RunE: func(cmd *cobra.Command, args []string) error {
return fetchBlob(cmd, opts)
return fetchBlob(cmd, &opts)
},
}

Expand All @@ -90,7 +90,7 @@ Example - Fetch and print a blob from OCI image layout archive file 'layout.tar'
return oerrors.Command(cmd, &opts.Target)
}

func fetchBlob(cmd *cobra.Command, opts fetchBlobOptions) (fetchErr error) {
func fetchBlob(cmd *cobra.Command, opts *fetchBlobOptions) (fetchErr error) {
ctx, logger := opts.WithContext(cmd.Context())
var target oras.ReadOnlyTarget
target, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/blob/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir':
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return pushBlob(cmd.Context(), opts)
return pushBlob(cmd.Context(), &opts)
},
}

Expand All @@ -100,7 +100,7 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir':
return oerrors.Command(cmd, &opts.Target)
}

func pushBlob(ctx context.Context, opts pushBlobOptions) (err error) {
func pushBlob(ctx context.Context, opts *pushBlobOptions) (err error) {
ctx, logger := opts.WithContext(ctx)

target, err := opts.NewTarget(opts.Common, logger)
Expand Down
6 changes: 3 additions & 3 deletions cmd/oras/root/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Example - Copy an artifact with multiple tags with concurrency tuned:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return runCopy(cmd, opts)
return runCopy(cmd, &opts)
},
}
cmd.Flags().BoolVarP(&opts.recursive, "recursive", "r", false, "[Preview] recursively copy the artifact and its referrer artifacts")
Expand All @@ -105,7 +105,7 @@ Example - Copy an artifact with multiple tags with concurrency tuned:
return oerrors.Command(cmd, &opts.BinaryTarget)
}

func runCopy(cmd *cobra.Command, opts copyOptions) error {
func runCopy(cmd *cobra.Command, opts *copyOptions) error {
ctx, logger := opts.WithContext(cmd.Context())

// Prepare source
Expand Down Expand Up @@ -148,7 +148,7 @@ func runCopy(cmd *cobra.Command, opts copyOptions) error {
return nil
}

func doCopy(ctx context.Context, src oras.ReadOnlyGraphTarget, dst oras.GraphTarget, opts copyOptions) (ocispec.Descriptor, error) {
func doCopy(ctx context.Context, src oras.ReadOnlyGraphTarget, dst oras.GraphTarget, opts *copyOptions) (ocispec.Descriptor, error) {
// Prepare copy options
committed := &sync.Map{}
extendedCopyOptions := oras.DefaultExtendedCopyOptions
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/cp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func Test_doCopy(t *testing.T) {
opts.From.Reference = desc.Digest.String()
dst := memory.New()
// test
_, err = doCopy(context.Background(), src, dst, opts)
_, err = doCopy(context.Background(), src, dst, &opts)
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/oras/root/discover.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return runDiscover(cmd, opts)
return runDiscover(cmd, &opts)
},
}

Expand All @@ -92,7 +92,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout
return oerrors.Command(cmd, &opts.Target)
}

func runDiscover(cmd *cobra.Command, opts discoverOptions) error {
func runDiscover(cmd *cobra.Command, opts *discoverOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
repo, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
if err != nil {
Expand All @@ -112,7 +112,7 @@ func runDiscover(cmd *cobra.Command, opts discoverOptions) error {

if opts.outputType == "tree" {
root := tree.New(fmt.Sprintf("%s@%s", opts.Path, desc.Digest))
err = fetchAllReferrers(ctx, repo, desc, opts.artifactType, root, &opts)
err = fetchAllReferrers(ctx, repo, desc, opts.artifactType, root, opts)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import (
"os"
"strings"

credentials "github.com/oras-project/oras-credentials-go"
"github.com/spf13/cobra"
"golang.org/x/term"
"oras.land/oras-go/v2/registry/remote/credentials"
"oras.land/oras/cmd/oras/internal/argument"
oerrors "oras.land/oras/cmd/oras/internal/errors"
"oras.land/oras/cmd/oras/internal/option"
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ package root
import (
"context"

credentials "github.com/oras-project/oras-credentials-go"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"oras.land/oras-go/v2/registry/remote/credentials"
"oras.land/oras/cmd/oras/internal/argument"
oerrors "oras.land/oras/cmd/oras/internal/errors"
"oras.land/oras/internal/credential"
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/manifest/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Example - Delete a manifest by digest 'sha256:99e4703fbf30916f549cd6bfa9cdbab614
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return deleteManifest(cmd, opts)
return deleteManifest(cmd, &opts)
},
}

Expand All @@ -75,7 +75,7 @@ Example - Delete a manifest by digest 'sha256:99e4703fbf30916f549cd6bfa9cdbab614
return oerrors.Command(cmd, &opts.Target)
}

func deleteManifest(cmd *cobra.Command, opts deleteOptions) error {
func deleteManifest(cmd *cobra.Command, opts *deleteOptions) error {
ctx, logger := opts.WithContext(cmd.Context())
manifests, err := opts.NewManifestDeleter(opts.Common, logger)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/manifest/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar':
},
Aliases: []string{"get"},
RunE: func(cmd *cobra.Command, args []string) error {
return fetchManifest(cmd, opts)
return fetchManifest(cmd, &opts)
},
}

Expand All @@ -90,7 +90,7 @@ Example - Fetch raw manifest from an OCI layout archive file 'layout.tar':
return oerrors.Command(cmd, &opts.Target)
}

func fetchManifest(cmd *cobra.Command, opts fetchOptions) (fetchErr error) {
func fetchManifest(cmd *cobra.Command, opts *fetchOptions) (fetchErr error) {
ctx, logger := opts.WithContext(cmd.Context())

target, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/manifest/fetch_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Example - Fetch and print the prettified descriptor of the config:
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return fetchConfig(cmd, opts)
return fetchConfig(cmd, &opts)
},
}

Expand All @@ -87,7 +87,7 @@ Example - Fetch and print the prettified descriptor of the config:
return oerrors.Command(cmd, &opts.Target)
}

func fetchConfig(cmd *cobra.Command, opts fetchConfigOptions) (fetchErr error) {
func fetchConfig(cmd *cobra.Command, opts *fetchConfigOptions) (fetchErr error) {
ctx, logger := opts.WithContext(cmd.Context())

repo, err := opts.NewReadonlyTarget(ctx, opts.Common, logger)
Expand Down
Loading

0 comments on commit 812bb59

Please sign in to comment.