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): improve error message when attaching without subject artifact #1430

Merged
merged 23 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
11 changes: 11 additions & 0 deletions cmd/oras/internal/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ import (
"oras.land/oras-go/v2/registry/remote/errcode"
)

// OperationType stands for certain type of operations.
type OperationType int
qweeah marked this conversation as resolved.
Show resolved Hide resolved

const (
// OperationTypeParseArtifactReference represents parsing artifact
// reference operation.
OperationTypeParseArtifactReference OperationType = iota + 1
)

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

Expand All @@ -39,6 +48,7 @@ func (e UnsupportedFormatTypeError) Error() string {

// Error is the error type for CLI error messaging.
type Error struct {
OperationType OperationType
Err error
Usage string
Recommendation string
Expand Down Expand Up @@ -160,6 +170,7 @@ func NewErrEmptyTagOrDigest(ref string, cmd *cobra.Command, needsTag bool) error
errMsg = "no tag or digest specified"
}
return &Error{
OperationType: OperationTypeParseArtifactReference,
Err: fmt.Errorf(`"%s": %s`, ref, errMsg),
Usage: fmt.Sprintf("%s %s", cmd.Parent().CommandPath(), cmd.Use),
Recommendation: fmt.Sprintf(`Please specify a reference in the form of %s. Run "%s -h" for more options and examples`, form, cmd.CommandPath()),
Expand Down
9 changes: 6 additions & 3 deletions cmd/oras/internal/option/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,14 @@ func (opts *Target) Parse(cmd *cobra.Command) error {
return opts.parseOCILayoutReference()
default:
opts.Type = TargetTypeRemote
if _, err := registry.ParseReference(opts.RawReference); err != nil {
if ref, err := registry.ParseReference(opts.RawReference); err != nil {
return &oerrors.Error{
OperationType: oerrors.OperationTypeParseArtifactReference,
Err: fmt.Errorf("%q: %w", opts.RawReference, err),
Recommendation: "Please make sure the provided reference is in the form of <registry>/<repo>[:tag|@digest]",
}
} else {
opts.Reference = ref.Reference
}
return opts.Remote.Parse(cmd)
}
Expand Down Expand Up @@ -243,9 +246,9 @@ func (opts *Target) NewReadonlyTarget(ctx context.Context, common Common, logger
}

// EnsureReferenceNotEmpty returns formalized error when the reference is empty.
func (opts *Target) EnsureReferenceNotEmpty(cmd *cobra.Command, needsTag bool) error {
func (opts *Target) EnsureReferenceNotEmpty(cmd *cobra.Command, allowTag bool) error {
if opts.Reference == "" {
return oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, needsTag)
return oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, allowTag)
}
return nil
}
Expand Down
21 changes: 15 additions & 6 deletions cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,22 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
PreRunE: func(cmd *cobra.Command, args []string) error {
opts.RawReference = args[0]
opts.FileRefs = args[1:]
if err := option.Parse(cmd, &opts); err != nil {
return err
err := option.Parse(cmd, &opts)
if err == nil {
if opts.Reference != "" {
return nil
}
// ensure reference is not empty
err = oerrors.NewErrEmptyTagOrDigest(opts.RawReference, cmd, true)
qweeah marked this conversation as resolved.
Show resolved Hide resolved
}
if len(opts.FileRefs) == 0 {
// no file argument provided
if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.OperationTypeParseArtifactReference {
// invalid reference
err.Recommendation = fmt.Sprintf("Are you missing an artifact reference to attach to? %s", err.Recommendation)
}
}
return nil
return err
},
RunE: func(cmd *cobra.Command, args []string) error {
return runAttach(cmd, &opts)
Expand Down Expand Up @@ -137,9 +149,6 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error {
if err != nil {
return err
}
if err := opts.EnsureReferenceNotEmpty(cmd, true); 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)
Expand Down
21 changes: 17 additions & 4 deletions test/e2e/suite/command/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,25 @@ var _ = Describe("ORAS beginners:", func() {
ExpectFailure().MatchErrKeyWords("unknown distribution specification flag").Exec()
})

It("should fail with error suggesting subject missed", func() {
err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, "")).ExpectFailure().Exec().Err
Expect(err).Should(gbytes.Say("Error"))
Expect(err).Should(gbytes.Say("\nAre you missing an artifact reference to attach to?"))
})

It("should fail with error suggesting right form", func() {
err := ORAS("attach", "--artifact-type", "oras/test", RegistryRef(ZOTHost, ImageRepo, ""), "./test.json").ExpectFailure().Exec().Err
Expect(err).Should(gbytes.Say("Error"))
Expect(err).Should(gbytes.Say("no tag or digest specified"))
Expect(err).ShouldNot(gbytes.Say("\nAre you missing an artifact reference to attach to?"))
})

It("should fail and show detailed error description if no argument provided", func() {
err := ORAS("attach").ExpectFailure().Exec().Err
gomega.Expect(err).Should(gbytes.Say("Error"))
gomega.Expect(err).Should(gbytes.Say("\nUsage: oras attach"))
gomega.Expect(err).Should(gbytes.Say("\n"))
gomega.Expect(err).Should(gbytes.Say(`Run "oras attach -h"`))
Expect(err).Should(gbytes.Say("Error"))
Expect(err).Should(gbytes.Say("\nUsage: oras attach"))
Expect(err).Should(gbytes.Say("\n"))
Expect(err).Should(gbytes.Say(`Run "oras attach -h"`))
})

It("should fail if distribution spec is not valid", func() {
Expand Down
Loading