feat!: disable auto type conversion for manifests (oras-project#797)

Signed-off-by: Billy Zha <[email protected]>
Parthmandale · Mar 6, 2023 · 16e40b5 · 16e40b5
1 parent 12431d5
commit 16e40b5
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 102 deletions.
diff --git a/cmd/oras/attach.go b/cmd/oras/attach.go
@@ -27,6 +27,7 @@ import (
 	"oras.land/oras-go/v2/content"
 	"oras.land/oras-go/v2/content/file"
 	"oras.land/oras/cmd/oras/internal/option"
+	"oras.land/oras/internal/graph"
 )
 
 type attachOptions struct {
@@ -144,19 +145,24 @@ func runAttach(opts attachOptions) error {
 	graphCopyOptions.Concurrency = opts.concurrency
 	updateDisplayOption(&graphCopyOptions, store, opts.Verbose)
 	copy := func(root ocispec.Descriptor) error {
-		if root.MediaType == ocispec.MediaTypeArtifactManifest {
-			graphCopyOptions.FindSuccessors = func(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) {
-				if content.Equal(node, root) {
-					// skip subject
-					return descs, nil
+		graphCopyOptions.FindSuccessors = func(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) {
+			if content.Equal(node, root) {
+				// skip duplicated Resolve on subject
+				successors, _, config, err := graph.Successors(ctx, fetcher, node)
+				if err != nil {
+					return nil, err
 				}
-				return content.Successors(ctx, fetcher, node)
+				if config != nil {
+					successors = append(successors, *config)
+				}
+				return successors, nil
 			}
+			return content.Successors(ctx, fetcher, node)
 		}
 		return oras.CopyGraph(ctx, store, dst, root, graphCopyOptions)
 	}
 
-	root, err := pushArtifact(dst, pack, &packOpts, copy, &graphCopyOptions, opts.ManifestMediaType == "", opts.Verbose)
+	root, err := pushArtifact(dst, pack, copy)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/oras/internal/option/spec.go b/cmd/oras/internal/option/spec.go
@@ -34,8 +34,6 @@ type ImageSpec struct {
 // Parse parses flags into the option.
 func (opts *ImageSpec) Parse() error {
 	switch opts.specFlag {
-	case "":
-		opts.ManifestMediaType = ""
 	case "v1.1-image":
 		opts.ManifestMediaType = ocispec.MediaTypeImageManifest
 	case "v1.1-artifact":
@@ -48,7 +46,7 @@ func (opts *ImageSpec) Parse() error {
 
 // ApplyFlags applies flags to a command flag set.
 func (opts *ImageSpec) ApplyFlags(fs *pflag.FlagSet) {
-	fs.StringVar(&opts.specFlag, "image-spec", "", "specify manifest type for building artifact. options: v1.1-image, v1.1-artifact")
+	fs.StringVar(&opts.specFlag, "image-spec", "v1.1-image", "specify manifest type for building artifact. options: v1.1-image, v1.1-artifact")
 }
 
 // distributionSpec option struct.

diff --git a/cmd/oras/push.go b/cmd/oras/push.go
@@ -17,10 +17,8 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
-	"net/http"
 	"strings"
 	"sync"
 
@@ -29,8 +27,6 @@ import (
 	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content"
 	"oras.land/oras-go/v2/content/file"
-	"oras.land/oras-go/v2/registry/remote"
-	"oras.land/oras-go/v2/registry/remote/errcode"
 	"oras.land/oras/cmd/oras/internal/display"
 	"oras.land/oras/cmd/oras/internal/fileref"
 	"oras.land/oras/cmd/oras/internal/option"
@@ -193,7 +189,7 @@ func runPush(opts pushOptions) error {
 	}
 
 	// Push
-	root, err := pushArtifact(dst, pack, &packOpts, copy, &copyOptions.CopyGraphOptions, opts.ManifestMediaType == "", opts.Verbose)
+	root, err := pushArtifact(dst, pack, copy)
 	if err != nil {
 		return err
 	}
@@ -236,89 +232,15 @@ func updateDisplayOption(opts *oras.CopyGraphOptions, store content.Fetcher, ver
 type packFunc func() (ocispec.Descriptor, error)
 type copyFunc func(desc ocispec.Descriptor) error
 
-func pushArtifact(dst oras.Target, pack packFunc, packOpts *oras.PackOptions, copy copyFunc, copyOpts *oras.CopyGraphOptions, allowFallback bool, verbose bool) (ocispec.Descriptor, error) {
+func pushArtifact(dst oras.Target, pack packFunc, copy copyFunc) (ocispec.Descriptor, error) {
 	root, err := pack()
 	if err != nil {
 		return ocispec.Descriptor{}, err
 	}
 
-	copyRootAttempted := false
-	preCopy := copyOpts.PreCopy
-	copyOpts.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
-		if content.Equal(root, desc) {
-			// copyRootAttempted helps track whether the returned error is
-			// generated from copying root.
-			copyRootAttempted = true
-		}
-		if preCopy != nil {
-			return preCopy(ctx, desc)
-		}
-		return nil
-	}
-
 	// push
-	if err = copy(root); err == nil {
-		return root, nil
-	}
-
-	if !allowFallback || !copyRootAttempted || root.MediaType != ocispec.MediaTypeArtifactManifest ||
-		!isManifestUnsupported(err) {
-		return ocispec.Descriptor{}, err
-	}
-
-	if err := display.PrintStatus(root, "Fallback ", verbose); err != nil {
-		return ocispec.Descriptor{}, err
-	}
-	if repo, ok := dst.(*remote.Repository); ok {
-		// assumes referrers API is not supported since OCI artifact
-		// media type is not supported
-		repo.SetReferrersCapability(false)
-	}
-	packOpts.PackImageManifest = true
-	root, err = pack()
-	if err != nil {
-		return ocispec.Descriptor{}, err
-	}
-
-	copyOpts.FindSuccessors = func(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) {
-		if content.Equal(node, root) {
-			// skip non-config
-			content, err := content.FetchAll(ctx, fetcher, root)
-			if err != nil {
-				return nil, err
-			}
-			var manifest ocispec.Manifest
-			if err := json.Unmarshal(content, &manifest); err != nil {
-				return nil, err
-			}
-			return []ocispec.Descriptor{manifest.Config}, nil
-		}
-
-		// config has no successors
-		return nil, nil
-	}
 	if err = copy(root); err != nil {
 		return ocispec.Descriptor{}, err
 	}
 	return root, nil
 }
-
-func isManifestUnsupported(err error) bool {
-	var errResp *errcode.ErrorResponse
-	if !errors.As(err, &errResp) || errResp.StatusCode != http.StatusBadRequest {
-		return false
-	}
-
-	var errCode errcode.Error
-	if !errors.As(errResp, &errCode) {
-		return false
-	}
-
-	// As of November 2022, ECR is known to return UNSUPPORTED error when
-	// putting an OCI artifact manifest.
-	switch errCode.Code {
-	case errcode.ErrorCodeManifestInvalid, errcode.ErrorCodeUnsupported:
-		return true
-	}
-	return false
-}
diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go
@@ -50,11 +50,11 @@ var _ = Describe("Remote registry users:", func() {
 			}
 
 			ORAS("push", Reference(Host, repo, tag), files[1], "-v").
-				MatchStatus(statusKeys, true, 1).
+				MatchStatus(statusKeys, true, len(statusKeys)).
 				WithWorkDir(tempDir).Exec()
 			fetched := ORAS("manifest", "fetch", Reference(Host, repo, tag)).Exec().Out
-			Binary("jq", ".blobs[]", "--compact-output").
-				MatchTrimmedContent(fmt.Sprintf(layerDescriptorTemplate, ocispec.MediaTypeImageLayer)).
+			Binary("jq", ".layers[]", "--compact-output").
+				MatchTrimmedContent(fmt.Sprintf(layerDescriptorTemplate, "application/vnd.oci.image.layer.v1.tar")).
 				WithInput(fetched).Exec()
 		})
 
@@ -64,15 +64,15 @@ var _ = Describe("Remote registry users:", func() {
 			extraTag := "2e2"
 
 			ORAS("push", fmt.Sprintf("%s,%s", Reference(Host, repo, tag), extraTag), files[1], "-v").
-				MatchStatus(statusKeys, true, 1).
+				MatchStatus(statusKeys, true, len(statusKeys)).
 				WithWorkDir(tempDir).Exec()
 			fetched := ORAS("manifest", "fetch", Reference(Host, repo, tag)).Exec().Out
-			Binary("jq", ".blobs[]", "--compact-output").
+			Binary("jq", ".layers[]", "--compact-output").
 				MatchTrimmedContent(fmt.Sprintf(layerDescriptorTemplate, ocispec.MediaTypeImageLayer)).
 				WithInput(fetched).Exec()
 
 			fetched = ORAS("manifest", "fetch", Reference(Host, repo, extraTag)).Exec().Out
-			Binary("jq", ".blobs[]", "--compact-output").
+			Binary("jq", ".layers[]", "--compact-output").
 				MatchTrimmedContent(fmt.Sprintf(layerDescriptorTemplate, ocispec.MediaTypeImageLayer)).
 				WithInput(fetched).Exec()
 		})
@@ -84,12 +84,11 @@ var _ = Describe("Remote registry users:", func() {
 			if err := CopyTestData(tempDir); err != nil {
 				panic(err)
 			}
-
 			ORAS("push", Reference(Host, repo, tag), files[1]+":"+layerType, "-v").
-				MatchStatus(statusKeys, true, 1).
+				MatchStatus(statusKeys, true, len(statusKeys)).
 				WithWorkDir(tempDir).Exec()
 			fetched := ORAS("manifest", "fetch", Reference(Host, repo, tag)).Exec().Out
-			Binary("jq", ".blobs[]", "--compact-output").
+			Binary("jq", ".layers[]", "--compact-output").
 				MatchTrimmedContent(fmt.Sprintf(layerDescriptorTemplate, layerType)).
 				WithInput(fetched).Exec()
 		})
@@ -104,7 +103,7 @@ var _ = Describe("Remote registry users:", func() {
 
 			exportPath := "packed.json"
 			ORAS("push", Reference(Host, repo, tag), files[1]+":"+layerType, "-v", "--export-manifest", exportPath).
-				MatchStatus(statusKeys, true, 1).
+				MatchStatus(statusKeys, true, len(statusKeys)).
 				WithWorkDir(tempDir).Exec()
 			fetched := ORAS("manifest", "fetch", Reference(Host, repo, tag)).Exec().Out.Contents()
 			MatchFile(filepath.Join(tempDir, exportPath), string(fetched), DefaultTimeout)
@@ -159,11 +158,11 @@ var _ = Describe("Remote registry users:", func() {
 			}
 
 			ORAS("push", Reference(Host, repo, tag), files[1], "-v", "--annotation", fmt.Sprintf("%s=%s", key, value)).
-				MatchStatus(statusKeys, true, 1).
+				MatchStatus(statusKeys, true, len(statusKeys)).
 				WithWorkDir(tempDir).Exec()
 			fetched := ORAS("manifest", "fetch", Reference(Host, repo, tag)).Exec().Out
 
-			Binary("jq", `.annotations|del(.["org.opencontainers.artifact.created"])`, "--compact-output").
+			Binary("jq", `.annotations|del(.["org.opencontainers.image.created"])`, "--compact-output").
 				MatchTrimmedContent(fmt.Sprintf(`{"%s":"%s"}`, key, value)).
 				WithInput(fetched).Exec()
 		})