chore(ux): improve error message when attaching without subject artif…

…act (#1430)

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

cmd/oras/internal/errors/errors.go

@@ -26,6 +26,15 @@ import (
 	"oras.land/oras-go/v2/registry/remote/errcode"
 )
 
+// OperationType stands for certain type of operations.
+type OperationType int
+
+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:"
 
@@ -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
@@ -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()),

cmd/oras/internal/option/target.go

@@ -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)
 	}
@@ -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
 }

cmd/oras/root/attach.go

@@ -49,7 +49,7 @@ type attachOptions struct {
 func attachCmd() *cobra.Command {
 	var opts attachOptions
 	cmd := &cobra.Command{
-		Use:   "attach [flags] --artifact-type=<type> <name>{:<tag>|@<digest>} <file>[:<layer_media_type>] [...]",
+		Use:   "attach [flags] --artifact-type=<type> <name>{:<tag>|@<digest>} {<file>[:<layer_media_type>]|--annotation <key>=<value>} [...]",
 		Short: "[Preview] Attach files to an existing artifact",
 		Long: `[Preview] Attach files to an existing artifact
 
@@ -87,10 +87,20 @@ 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 err = opts.EnsureReferenceNotEmpty(cmd, true); err == nil {
+					return nil
+				}
+			}
+			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)
@@ -137,9 +147,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)

test/e2e/suite/command/attach.go

@@ -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() {