chore(ux): improve error message for invalid registry and file refere…

…nce (#1258)

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

cmd/oras/internal/option/target.go

@@ -105,6 +105,14 @@ func (opts *Target) Parse() error {
 		return nil
 	default:
 		opts.Type = TargetTypeRemote
+		if ref, err := registry.ParseReference(opts.RawReference); err != nil {
+			return err
+		} else if ref.Registry == "" || ref.Repository == "" {
+			return &oerrors.Error{
+				Err:            fmt.Errorf("%q is an invalid reference", opts.RawReference),
+				Recommendation: "Please make sure the provided reference is in the form of <registry>/<repo>[:tag|@digest]",
+			}
+		}
 		return opts.Remote.Parse()
 	}
 }

cmd/oras/internal/option/target_test.go

@@ -39,7 +39,10 @@ func TestTarget_Parse_oci(t *testing.T) {
 }
 
 func TestTarget_Parse_remote(t *testing.T) {
-	opts := Target{IsOCILayout: false}
+	opts := Target{
+		RawReference: "mocked/test",
+		IsOCILayout:  false,
+	}
 	if err := opts.Parse(); err != nil {
 		t.Errorf("Target.Parse() error = %v", err)
 	}
@@ -48,6 +51,16 @@ func TestTarget_Parse_remote(t *testing.T) {
 	}
 }
 
+func TestTarget_Parse_remote_err(t *testing.T) {
+	opts := Target{
+		RawReference: "/test",
+		IsOCILayout:  false,
+	}
+	if err := opts.Parse(); err == nil {
+		t.Errorf("expect Target.Parse() to fail but not")
+	}
+}
+
 func Test_parseOCILayoutReference(t *testing.T) {
 	type args struct {
 		raw string

cmd/oras/root/file.go

@@ -17,7 +17,9 @@ package root
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io/fs"
 	"path/filepath"
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -42,7 +44,7 @@ func loadFiles(ctx context.Context, store *file.Store, annotations map[string]ma
 		if verbose {
 			fmt.Println("Preparing", name)
 		}
-		file, err := store.Add(ctx, name, mediaType, filename)
+		file, err := addFile(ctx, store, name, mediaType, filename)
 		if err != nil {
 			return nil, err
 		}
@@ -62,3 +64,15 @@ func loadFiles(ctx context.Context, store *file.Store, annotations map[string]ma
 	}
 	return files, nil
 }
+
+func addFile(ctx context.Context, store *file.Store, name string, mediaType string, filename string) (ocispec.Descriptor, error) {
+	file, err := store.Add(ctx, name, mediaType, filename)
+	if err != nil {
+		var pathErr *fs.PathError
+		if errors.As(err, &pathErr) {
+			err = pathErr
+		}
+		return ocispec.Descriptor{}, err
+	}
+	return file, nil
+}

cmd/oras/root/push.go

@@ -157,7 +157,7 @@ func runPush(ctx context.Context, opts *pushOptions) error {
 		if err != nil {
 			return err
 		}
-		desc, err := store.Add(ctx, option.AnnotationConfig, cfgMediaType, path)
+		desc, err := addFile(ctx, store, option.AnnotationConfig, cfgMediaType, path)
 		if err != nil {
 			return err
 		}

test/e2e/suite/command/attach.go

@@ -56,7 +56,7 @@ var _ = Describe("ORAS beginners:", func() {
 
 		It("should fail if no file reference or manifest annotation provided for OCI layout", func() {
 			root := GinkgoT().TempDir()
-			ORAS("attach", "--artifact-type", "oras/test", LayoutRef(root, foobar.Tag)).
+			ORAS("attach", "--artifact-type", "oras/test", LayoutRef(root, foobar.Tag), Flags.Layout).
 				ExpectFailure().MatchErrKeyWords("Error: neither file nor annotation", "Usage:").Exec()
 		})
 

test/e2e/suite/command/push.go

@@ -49,6 +49,22 @@ var _ = Describe("ORAS beginners:", func() {
 			gomega.Expect(err).Should(gbytes.Say(`Run "oras push -h"`))
 		})
 
+		It("should fail if the provided reference is not valid", func() {
+			err := ORAS("push", "/oras").ExpectFailure().Exec().Err
+			gomega.Expect(err).Should(gbytes.Say(`Error: "/oras" is an invalid reference`))
+			gomega.Expect(err).Should(gbytes.Say(regexp.QuoteMeta("Please make sure the provided reference is in the form of <registry>/<repo>[:tag|@digest]")))
+		})
+
+		It("should fail if the to-be-pushed file is not found", func() {
+			tempDir := GinkgoT().TempDir()
+			notFoundFilePath := "file/not/found"
+			err := ORAS("push", RegistryRef(ZOTHost, pushTestRepo("file-not-found"), ""), notFoundFilePath).
+				WithWorkDir(tempDir).
+				ExpectFailure().Exec().Err
+			gomega.Expect(err).Should(gbytes.Say(filepath.Join(tempDir, notFoundFilePath)))
+			gomega.Expect(err).Should(gbytes.Say("no such file or directory"))
+		})
+
 		It("should fail to use --config and --artifact-type at the same time for OCI spec v1.0 registry", func() {
 			tempDir := PrepareTempFiles()
 			repo := pushTestRepo("no-mediatype")