Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ux): show registry error and hint for dockehub #1222

Merged
merged 46 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
5dd63fd
chore(ux): show registry error and hint for dockehub
qweeah Dec 28, 2023
6994167
code clean
qweeah Dec 28, 2023
3c10c7d
support not found error
qweeah Dec 29, 2023
b0fa65a
Merge remote-tracking branch 'origin_src/main' into docker-error
qweeah Dec 29, 2023
0925a45
support `oras ls` and `oras pull`
qweeah Dec 29, 2023
11cb9cf
add e2e test
qweeah Dec 29, 2023
dbdaa59
code clean
qweeah Dec 29, 2023
c19a844
add error test for pull
qweeah Dec 29, 2023
354df3e
add unit test
qweeah Jan 2, 2024
20e23e3
support error prefix in cp and resolve
qweeah Jan 2, 2024
8594411
support push
qweeah Jan 2, 2024
fdb5d96
Merge remote-tracking branch 'origin_src/main' into docker-error
qweeah Jan 5, 2024
6d0a533
fix e2e
qweeah Jan 5, 2024
317556b
scrub debug info & handle empty response body
qweeah Jan 5, 2024
4076635
fix e2e
qweeah Jan 5, 2024
1d3fefa
fix e2e
qweeah Jan 5, 2024
1c01b0a
fix e2e
qweeah Jan 7, 2024
937d80c
fix e2e
qweeah Jan 7, 2024
d905d0f
Merge remote-tracking branch 'origin_src/main' into docker-error
qweeah Jan 8, 2024
ae3dd7b
support all commands and add e2e
qweeah Jan 8, 2024
70ac3a0
bug fix
qweeah Jan 8, 2024
92588f5
improve readability
qweeah Jan 8, 2024
668152d
resolve empty resp body
qweeah Jan 8, 2024
fa3adf1
resolve e2e
qweeah Jan 8, 2024
cc42b04
code clean
qweeah Jan 8, 2024
666c518
fix e2e
qweeah Jan 8, 2024
028beb0
fix e2e
qweeah Jan 8, 2024
716e62a
resolve comment
qweeah Jan 9, 2024
4503268
code clean
qweeah Jan 9, 2024
9d299a9
bug fix
qweeah Jan 9, 2024
3607c16
resolve comments
qweeah Jan 9, 2024
a9664b3
resolve comments
qweeah Jan 11, 2024
0131f02
bug fix
qweeah Jan 11, 2024
e47e0d3
add coverage
qweeah Jan 11, 2024
0494f77
add unit test
qweeah Jan 11, 2024
19b7ec1
fix e2e
qweeah Jan 11, 2024
e530def
fix full reference match and add UT
qweeah Jan 11, 2024
ff6a9ba
cover more
qweeah Jan 11, 2024
b12d29e
code clean
qweeah Jan 11, 2024
ce2582a
add TODO
qweeah Jan 11, 2024
7c99330
add coverage
qweeah Jan 11, 2024
162a9dc
increase coverage
qweeah Jan 11, 2024
c15c3f5
refactor trimming
qweeah Jan 11, 2024
ddc9794
resolve comments
qweeah Jan 15, 2024
af4601a
Merge remote-tracking branch 'origin_src/main' into docker-error
qweeah Jan 15, 2024
e602b89
fix unit test
qweeah Jan 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions cmd/oras/internal/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,16 @@

import (
"fmt"
"strings"

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

// RegistryErrorPrefix is the commandline prefix for errors from registry.
const RegistryErrorPrefix = "Error response from registry:"

// Error is the error type for CLI error messaging.
type Error struct {
Err error
Expand Down Expand Up @@ -60,6 +65,60 @@
}
}

// Modifier modifies the error during cmd execution.
type Modifier interface {
Modify(cmd *cobra.Command, err error) (modifiedErr error, modified bool)
}

// Command returns an error-handled cobra command.
func Command(cmd *cobra.Command, handler Modifier) *cobra.Command {
runE := cmd.RunE
cmd.RunE = func(cmd *cobra.Command, args []string) error {
err := runE(cmd, args)
if err != nil {
err, _ = handler.Modify(cmd, err)
return err
}
return nil
}
return cmd
}

// Trim tries to trim toTrim from err.
func Trim(err error, toTrim error) error {
var inner error
if errResp, ok := toTrim.(*errcode.ErrorResponse); ok {
if len(errResp.Errors) == 0 {
return fmt.Errorf("recognizable error message not found: %w", toTrim)
}
inner = errResp.Errors
} else {
return err
}

Check warning on line 97 in cmd/oras/internal/errors/errors.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/errors/errors.go#L95-L97

Added lines #L95 - L97 were not covered by tests

if rewrapped := reWrap(err, toTrim, inner); rewrapped != nil {
return rewrapped
}
return inner
}

// reWrap re-wraps errA to errC and trims out errB, returns nil if scrub fails.
// +---------- errA ----------+
// | +---- errB ----+ | +---- errA ----+
// | | errC | | => | errC |
// | +--------------+ | +--------------+
// +--------------------------+
func reWrap(errA, errB, errC error) error {
// TODO: trim dedicated error type when
// https://github.com/oras-project/oras-go/issues/677 is done
contentA := errA.Error()
contentB := errB.Error()
if idx := strings.Index(contentA, contentB); idx > 0 {
return fmt.Errorf("%s%w", contentA[:idx], errC)
}
return nil
}

// NewErrEmptyTagOrDigest creates a new error based on the reference string.
func NewErrEmptyTagOrDigest(ref registry.Reference) error {
return NewErrEmptyTagOrDigestStr(ref.String())
Expand Down
18 changes: 17 additions & 1 deletion cmd/oras/internal/option/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,23 @@ import (

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/errcode"
"oras.land/oras-go/v2/registry/remote/retry"
oerrors "oras.land/oras/cmd/oras/internal/errors"
"oras.land/oras/internal/credential"
"oras.land/oras/internal/crypto"
onet "oras.land/oras/internal/net"
"oras.land/oras/internal/trace"
"oras.land/oras/internal/version"
)

// Remote options struct.
// Remote options struct contains flags and arguments specifying one registry.
// Remote implements oerrors.Handler and interface.
type Remote struct {
DistributionSpec
CACertFilePath string
Expand Down Expand Up @@ -322,3 +326,15 @@ func (opts *Remote) isPlainHttp(registry string) bool {
}
return plainHTTP
}

// Modify modifies error during cmd execution.
func (opts *Remote) Modify(cmd *cobra.Command, err error) (error, bool) {
var errResp *errcode.ErrorResponse
if errors.As(err, &errResp) {
cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
return &oerrors.Error{
Err: oerrors.Trim(err, errResp),
}, true
}
return err, false
}
59 changes: 59 additions & 0 deletions cmd/oras/internal/option/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@ import (
"fmt"
"io"
"io/fs"
"net/http"
"os"
"strings"
"sync"

"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"oras.land/oras-go/v2"
"oras.land/oras-go/v2/content"
"oras.land/oras-go/v2/content/oci"
"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/errcode"
oerrors "oras.land/oras/cmd/oras/internal/errors"
"oras.land/oras/cmd/oras/internal/fileref"
)
Expand All @@ -43,6 +47,7 @@ const (

// Target struct contains flags and arguments specifying one registry or image
// layout.
// Target implements oerrors.Handler interface.
type Target struct {
Remote
RawReference string
Expand Down Expand Up @@ -239,8 +244,54 @@ func (opts *Target) EnsureReferenceNotEmpty() error {
return nil
}

// Modify handles error during cmd execution.
func (opts *Target) Modify(cmd *cobra.Command, err error) (error, bool) {
if opts.IsOCILayout {
return err, false
}

if errors.Is(err, errdef.ErrNotFound) {
cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
return err, true
}

var errResp *errcode.ErrorResponse
if errors.As(err, &errResp) {
ref := registry.Reference{Registry: opts.RawReference}
if errResp.URL.Host != ref.Host() {
// raw reference is not registry host
var parseErr error
ref, parseErr = registry.ParseReference(opts.RawReference)
if parseErr != nil {
// this should not happen
return err, false
}
if errResp.URL.Host != ref.Host() {
// not handle if the error is not from the target
return err, false
}
}

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

if ref.Registry == "docker.io" && errResp.StatusCode == http.StatusUnauthorized {
if ref.Repository != "" && !strings.Contains(ref.Repository, "/") {
// docker.io/xxx -> docker.io/library/xxx
ref.Repository = "library/" + ref.Repository
ret.Recommendation = fmt.Sprintf("Namespace seems missing. Do you mean `%s %s`?", cmd.CommandPath(), ref)
}
}
return ret, true
}
return err, false
}

// BinaryTarget struct contains flags and arguments specifying two registries or
// image layouts.
// BinaryTarget implements errors.Handler interface.
type BinaryTarget struct {
From Target
To Target
Expand Down Expand Up @@ -269,3 +320,11 @@ func (opts *BinaryTarget) Parse() error {
opts.To.resolveFlag = append(opts.resolveFlag, opts.To.resolveFlag...)
return Parse(opts)
}

// Modify handles error during cmd execution.
func (opts *BinaryTarget) Modify(cmd *cobra.Command, err error) (error, bool) {
if modifiedErr, modified := opts.From.Modify(cmd, err); modified {
return modifiedErr, modified
}
return opts.To.Modify(cmd, err)
}
164 changes: 164 additions & 0 deletions cmd/oras/internal/option/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,15 @@ limitations under the License.
package option

import (
"errors"
"net/http"
"net/url"
"reflect"
"testing"

"github.com/spf13/cobra"
"oras.land/oras-go/v2/registry/remote/errcode"
oerrors "oras.land/oras/cmd/oras/internal/errors"
)

func TestTarget_Parse_oci(t *testing.T) {
Expand Down Expand Up @@ -75,3 +83,159 @@ func Test_parseOCILayoutReference(t *testing.T) {
})
}
}

func TestTarget_Modify_ociLayout(t *testing.T) {
errClient := errors.New("client error")
opts := &Target{}
got, modified := opts.Modify(&cobra.Command{}, errClient)

if modified {
t.Errorf("expect error not to be modified but received true")
}
if got != errClient {
t.Errorf("unexpected output from Target.Process() = %v", got)
}
}

func TestTarget_Modify_errInvalidReference(t *testing.T) {
errResp := &errcode.ErrorResponse{
URL: &url.URL{Host: "registry-1.docker.io"},
StatusCode: http.StatusUnauthorized,
Errors: errcode.Errors{
errcode.Error{
Code: "000",
Message: "mocked message",
Detail: map[string]string{"mocked key": "mocked value"},
},
},
}
opts := &Target{
RawReference: "invalid-reference",
}
got, modified := opts.Modify(&cobra.Command{}, errResp)

if modified {
t.Errorf("expect error not to be modified but received true")
}
if got != errResp {
t.Errorf("unexpected output from Target.Process() = %v", got)
}
}

func TestTarget_Modify_errHostNotMatching(t *testing.T) {
errResp := &errcode.ErrorResponse{
URL: &url.URL{Host: "registry-1.docker.io"},
StatusCode: http.StatusUnauthorized,
Errors: errcode.Errors{
errcode.Error{
Code: "000",
Message: "mocked message",
Detail: map[string]string{"mocked key": "mocked value"},
},
},
}

opts := &Target{
RawReference: "registry-2.docker.io/test:tag",
}
_, modified := opts.Modify(&cobra.Command{}, errResp)
if modified {
t.Errorf("expect error not to be modified but received true")
}
}

func TestTarget_Modify_dockerHint(t *testing.T) {
type fields struct {
Remote Remote
RawReference string
Type string
Reference string
Path string
IsOCILayout bool
}
errs := errcode.Errors{
errcode.Error{
Code: "000",
Message: "mocked message",
Detail: map[string]string{"mocked key": "mocked value"},
},
}
tests := []struct {
name string
fields fields
err error
modifiedErr *oerrors.Error
}{
{
"namespace already exists",
fields{RawReference: "docker.io/library/alpine:latest"},
&errcode.ErrorResponse{
URL: &url.URL{Host: "registry-1.docker.io"},
StatusCode: http.StatusUnauthorized,
Errors: errs,
},
&oerrors.Error{Err: errs},
},
{
"no namespace",
fields{RawReference: "docker.io"},
&errcode.ErrorResponse{
URL: &url.URL{Host: "registry-1.docker.io"},
StatusCode: http.StatusUnauthorized,
Errors: errs,
},
&oerrors.Error{Err: errs},
},
{
"not 401",
fields{RawReference: "docker.io"},
&errcode.ErrorResponse{
URL: &url.URL{Host: "registry-1.docker.io"},
StatusCode: http.StatusConflict,
Errors: errs,
},
&oerrors.Error{Err: errs},
},
{
"should hint",
fields{
RawReference: "docker.io/alpine",
Path: "oras test",
},
&errcode.ErrorResponse{
URL: &url.URL{Host: "registry-1.docker.io"},
StatusCode: http.StatusUnauthorized,
Errors: errs,
},
&oerrors.Error{
Err: errs,
Recommendation: "Namespace seems missing. Do you mean ` docker.io/library/alpine`?",
},
},
}

cmd := &cobra.Command{}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
opts := &Target{
Remote: tt.fields.Remote,
RawReference: tt.fields.RawReference,
Type: tt.fields.Type,
Reference: tt.fields.Reference,
Path: tt.fields.Path,
IsOCILayout: tt.fields.IsOCILayout,
}
got, modified := opts.Modify(cmd, tt.err)
gotErr, ok := got.(*oerrors.Error)
if !ok {
t.Errorf("expecting error to be *oerrors.Error but received %T", got)
}
if !reflect.DeepEqual(gotErr.Err, tt.modifiedErr.Err) || gotErr.Usage != tt.modifiedErr.Usage || gotErr.Recommendation != tt.modifiedErr.Recommendation {
t.Errorf("Target.Modify() error = %v, wantErr %v", gotErr, tt.modifiedErr)
}
if !modified {
t.Errorf("Failed to modify %v", tt.err)
}
})
}
}
Loading
Loading