chore(ux): improve error message for oras manifest push (#1241)

Signed-off-by: Xiaoxuan Wang <[email protected]>

cmd/oras/internal/manifest/manifest.go

@@ -0,0 +1,42 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manifest
+
+import (
+	"encoding/json"
+	"errors"
+)
+
+var (
+	// ErrMediaTypeNotFound is returned when the media type is not specified.
+	ErrMediaTypeNotFound = errors.New(`media type is not specified`)
+	// ErrInvalidJSON is returned when the json file is malformed.
+	ErrInvalidJSON = errors.New("not a valid json file")
+)
+
+// ExtractMediaType parses the media type field of bytes content in json format.
+func ExtractMediaType(content []byte) (string, error) {
+	var manifest struct {
+		MediaType string `json:"mediaType"`
+	}
+	if err := json.Unmarshal(content, &manifest); err != nil {
+		return "", ErrInvalidJSON
+	}
+	if manifest.MediaType == "" {
+		return "", ErrMediaTypeNotFound
+	}
+	return manifest.MediaType, nil
+}

cmd/oras/internal/manifest/manifest_test.go

@@ -0,0 +1,65 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package manifest
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+)
+
+const (
+	manifest          = `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.unknown.config.v1+json","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2},"layers":[{"mediaType":"application/vnd.oci.image.layer.v1.tar","digest":"sha256:5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03","size":6,"annotations":{"org.opencontainers.image.title":"hello.txt"}}]}`
+	manifestMediaType = "application/vnd.oci.image.manifest.v1+json"
+)
+
+func Test_ExtractMediaType(t *testing.T) {
+	// generate test content
+	content := []byte(manifest)
+
+	// test ExtractMediaType
+	want := manifestMediaType
+	got, err := ExtractMediaType(content)
+	if err != nil {
+		t.Fatal("ExtractMediaType() error=", err)
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("ExtractMediaType() = %v, want %v", got, want)
+	}
+}
+
+func Test_ExtractMediaType_invalidContent_notAJson(t *testing.T) {
+	// generate test content
+	content := []byte("manifest")
+
+	// test ExtractMediaType
+	_, err := ExtractMediaType(content)
+	expected := "not a valid json file"
+	if err.Error() != expected {
+		t.Fatalf("ExtractMediaType() error = %v, wantErr %v", err, expected)
+	}
+}
+
+func Test_ExtractMediaType_invalidContent_missingMediaType(t *testing.T) {
+	// generate test content
+	content := []byte(`{"schemaVersion":2}`)
+
+	// test ExtractMediaType
+	_, err := ExtractMediaType(content)
+	if !errors.Is(err, ErrMediaTypeNotFound) {
+		t.Fatalf("ExtractMediaType() error = %v, wantErr %v", err, ErrMediaTypeNotFound)
+	}
+}

cmd/oras/root/manifest/push.go

@@ -31,6 +31,7 @@ import (
 	"oras.land/oras/cmd/oras/internal/argument"
 	"oras.land/oras/cmd/oras/internal/display"
 	oerrors "oras.land/oras/cmd/oras/internal/errors"
+	"oras.land/oras/cmd/oras/internal/manifest"
 	"oras.land/oras/cmd/oras/internal/option"
 	"oras.land/oras/internal/file"
 )
@@ -94,7 +95,7 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit
 			return option.Parse(&opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return pushManifest(cmd.Context(), opts)
+			return pushManifest(cmd, opts)
 		},
 	}
 
@@ -105,8 +106,8 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit
 	return oerrors.Command(cmd, &opts.Target)
 }
 
-func pushManifest(ctx context.Context, opts pushOptions) error {
-	ctx, logger := opts.WithContext(ctx)
+func pushManifest(cmd *cobra.Command, opts pushOptions) error {
+	ctx, logger := opts.WithContext(cmd.Context())
 	var target oras.Target
 	var err error
 	target, err = opts.NewTarget(opts.Common, logger)
@@ -126,8 +127,15 @@ func pushManifest(ctx context.Context, opts pushOptions) error {
 	// get manifest media type
 	mediaType := opts.mediaType
 	if opts.mediaType == "" {
-		mediaType, err = file.ParseMediaType(contentBytes)
+		mediaType, err = manifest.ExtractMediaType(contentBytes)
 		if err != nil {
+			if errors.Is(err, manifest.ErrMediaTypeNotFound) {
+				return &oerrors.Error{
+					Err:            fmt.Errorf(`%w via the flag "--media-type" nor in %q`, err, opts.fileRef),
+					Usage:          fmt.Sprintf("%s %s", cmd.Parent().CommandPath(), cmd.Use),
+					Recommendation: `Please specify a valid media type in the manifest JSON or via the "--media-type" flag`,
+				}
+			}
 			return err
 		}
 	}

internal/file/file.go

@@ -16,7 +16,6 @@ limitations under the License.
 package file
 
 import (
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -117,17 +116,3 @@ func PrepareBlobContent(path string, mediaType string, dgstStr string, size int6
 		Size:      actualSize,
 	}, file, nil
 }
-
-// ParseMediaType parses the media type field of bytes content in json format.
-func ParseMediaType(content []byte) (string, error) {
-	var manifest struct {
-		MediaType string `json:"mediaType"`
-	}
-	if err := json.Unmarshal(content, &manifest); err != nil {
-		return "", errors.New("not a valid json file")
-	}
-	if manifest.MediaType == "" {
-		return "", errors.New("media type is not recognized")
-	}
-	return manifest.MediaType, nil
-}

internal/file/file_test.go

@@ -30,7 +30,6 @@ import (
 	"oras.land/oras/internal/file"
 )
 
-const manifestMediaType = "application/vnd.oci.image.manifest.v1+json"
 const blobMediaType = "application/mock-octet-stream"
 const manifest = `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.unknown.config.v1+json","digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a","size":2},"layers":[{"mediaType":"application/vnd.oci.image.layer.v1.tar","digest":"sha256:5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03","size":6,"annotations":{"org.opencontainers.image.title":"hello.txt"}}]}`
 
@@ -267,42 +266,3 @@ func TestFile_PrepareBlobContent_errOpenFile(t *testing.T) {
 		t.Fatalf("PrepareBlobContent() error = %v, wantErr %v", err, expected)
 	}
 }
-
-func TestFile_ParseMediaType(t *testing.T) {
-	// generate test content
-	content := []byte(manifest)
-
-	// test ParseMediaType
-	want := manifestMediaType
-	got, err := file.ParseMediaType(content)
-	if err != nil {
-		t.Fatal("ParseMediaType() error=", err)
-	}
-	if !reflect.DeepEqual(got, want) {
-		t.Errorf("ParseMediaType() = %v, want %v", got, want)
-	}
-}
-
-func TestFile_ParseMediaType_invalidContent_notAJson(t *testing.T) {
-	// generate test content
-	content := []byte("manifest")
-
-	// test ParseMediaType
-	_, err := file.ParseMediaType(content)
-	expected := "not a valid json file"
-	if err.Error() != expected {
-		t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected)
-	}
-}
-
-func TestFile_ParseMediaType_invalidContent_missingMediaType(t *testing.T) {
-	// generate test content
-	content := []byte(`{"schemaVersion":2}`)
-
-	// test ParseMediaType
-	_, err := file.ParseMediaType(content)
-	expected := "media type is not recognized"
-	if err.Error() != expected {
-		t.Fatalf("ParseMediaType() error = %v, wantErr %v", err, expected)
-	}
-}

test/e2e/suite/command/manifest.go

@@ -295,6 +295,7 @@ var _ = Describe("1.1 registry users:", func() {
 
 	When("running `manifest push`", func() {
 		manifest := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:fe9dbc99451d0517d65e048c309f0b5afb2cc513b7a3d456b6cc29fe641386c5","size":53},"layers":[]}`
+		manifestWithoutMediaType := `{"schemaVersion":2,"mediaType":"","config":{"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:fe9dbc99451d0517d65e048c309f0b5afb2cc513b7a3d456b6cc29fe641386c5","size":53},"layers":[]}`
 		digest := "sha256:bc1a59d49fc7c7b0a31f22ca0c743ecdabdb736777e3d9672fa9d97b4fe323f4"
 		descriptor := "{\"mediaType\":\"application/vnd.oci.image.manifest.v1+json\",\"digest\":\"sha256:bc1a59d49fc7c7b0a31f22ca0c743ecdabdb736777e3d9672fa9d97b4fe323f4\",\"size\":247}"
 
@@ -319,6 +320,13 @@ var _ = Describe("1.1 registry users:", func() {
 				MatchKeyWords("Pushed", RegistryRef(ZOTHost, ImageRepo, tag), "Digest:", digest).
 				WithInput(strings.NewReader(manifest)).Exec()
 		})
+
+		It("should fail to push manifest without media type with suggestion", func() {
+			manifestPath := WriteTempFile("manifest.json", manifestWithoutMediaType)
+			tag := "from-file"
+			ORAS("manifest", "push", RegistryRef(ZOTHost, ImageRepo, tag), manifestPath).
+				WithInput(strings.NewReader(manifest)).ExpectFailure().MatchErrKeyWords("Error:", " media type is not specified", "oras manifest push").Exec()
+		})
 	})
 
 	When("running `manifest fetch-config`", func() {