fix: only pull configuration in root (#684)

Resolves #682

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

cmd/oras/pull.go

@@ -29,7 +29,7 @@ import (
 	"oras.land/oras/cmd/oras/internal/display"
 	"oras.land/oras/cmd/oras/internal/errors"
 	"oras.land/oras/cmd/oras/internal/option"
-	"oras.land/oras/internal/descriptor"
+	"oras.land/oras/internal/graph"
 )
 
 type pullOptions struct {
@@ -116,6 +116,7 @@ func runPull(opts pullOptions) error {
 	if targetPlatform != nil {
 		copyOptions.WithTargetPlatform(targetPlatform)
 	}
+	var getConfigOnce sync.Once
 	copyOptions.FindSuccessors = func(ctx context.Context, fetcher content.Fetcher, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
 		statusFetcher := content.FetcherFunc(func(ctx context.Context, target ocispec.Descriptor) (fetched io.ReadCloser, fetchErr error) {
 			if _, ok := printed.LoadOrStore(generateContentKey(target), true); ok {
@@ -140,44 +141,44 @@ func runPull(opts pullOptions) error {
 			}
 			return rc, nil
 		})
-		successors, err := content.Successors(ctx, statusFetcher, desc)
+
+		nodes, subject, config, err := graph.Successors(ctx, statusFetcher, desc)
 		if err != nil {
 			return nil, err
 		}
-		var ret []ocispec.Descriptor
-		// Iterate all the successors to
-		// 1) Add name annotation to config if configPath is not empty
-		// 2) Skip fetching unnamed leaf nodes
-		for i, s := range successors {
-			// Save the config when:
-			// 1) MediaType matches, or
-			// 2) MediaType not specified and current node is config.
-			// Note: For a manifest, the 0th indexed element is always a
-			// manifest config.
-			if (s.MediaType == configMediaType || (configMediaType == "" && i == 0 && descriptor.IsImageManifest(desc))) && configPath != "" {
-				// Add annotation for manifest config
-				if s.Annotations == nil {
-					s.Annotations = make(map[string]string)
+		if subject != nil {
+			nodes = append(nodes, *subject)
+		}
+		if config != nil {
+			getConfigOnce.Do(func() {
+				if configPath != "" && (configMediaType == "" || config.MediaType == configMediaType) {
+					if config.Annotations == nil {
+						config.Annotations = make(map[string]string)
+					}
+					config.Annotations[ocispec.AnnotationTitle] = configPath
 				}
-				s.Annotations[ocispec.AnnotationTitle] = configPath
-			}
+			})
+			nodes = append(nodes, *config)
+		}
+
+		var ret []ocispec.Descriptor
+		for _, s := range nodes {
 			if s.Annotations[ocispec.AnnotationTitle] == "" {
 				ss, err := content.Successors(ctx, fetcher, s)
 				if err != nil {
 					return nil, err
 				}
-				// Skip s if s is unnamed and has no successors.
 				if len(ss) == 0 {
-					if _, loaded := printed.LoadOrStore(generateContentKey(s), true); !loaded {
-						if err = display.PrintStatus(s, "Skipped    ", opts.Verbose); err != nil {
-							return nil, err
-						}
+					// skip s if it is unnamed AND has no successors.
+					if err := printOnce(&printed, s, "Skipped    ", opts.Verbose); err != nil {
+						return nil, err
 					}
 					continue
 				}
 			}
 			ret = append(ret, s)
 		}
+
 		return ret, nil
 	}
 
@@ -201,10 +202,8 @@ func runPull(opts pullOptions) error {
 		}
 		for _, s := range successors {
 			if _, ok := s.Annotations[ocispec.AnnotationTitle]; ok {
-				if _, ok := printed.LoadOrStore(generateContentKey(s), true); !ok {
-					if err = display.PrintStatus(s, "Restored   ", opts.Verbose); err != nil {
-						return err
-					}
+				if err := printOnce(&printed, s, "Restored   ", opts.Verbose); err != nil {
+					return err
 				}
 			}
 		}
@@ -240,3 +239,10 @@ func runPull(opts pullOptions) error {
 func generateContentKey(desc ocispec.Descriptor) string {
 	return desc.Digest.String() + desc.Annotations[ocispec.AnnotationTitle]
 }
+
+func printOnce(printed *sync.Map, s ocispec.Descriptor, msg string, verbose bool) error {
+	if _, loaded := printed.LoadOrStore(generateContentKey(s), true); loaded {
+		return nil
+	}
+	return display.PrintStatus(s, msg, verbose)
+}

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/sirupsen/logrus v1.9.0
 	github.com/spf13/cobra v1.6.1
 	github.com/spf13/pflag v1.0.5
-	oras.land/oras-go/v2 v2.0.0-rc.4
+	oras.land/oras-go/v2 v2.0.0-rc.4.0.20221111045309-74eeb52579de
 )
 
 require (

go.sum

@@ -63,5 +63,5 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools/v3 v3.0.2 h1:kG1BFyqVHuQoVQiR1bWGnfz/fmHvvuiSPIV7rvl360E=
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
-oras.land/oras-go/v2 v2.0.0-rc.4 h1:hg/R2znUQ1+qd43gRmL16VeX1GIZ8hQlLalBjYhhKSk=
-oras.land/oras-go/v2 v2.0.0-rc.4/go.mod h1:YGHvWBGuqRlZgUyXUIoKsR3lcuCOb3DAtG0SEsEw1iY=
+oras.land/oras-go/v2 v2.0.0-rc.4.0.20221111045309-74eeb52579de h1:wd4FNQUAyeKAsL28thn32k+vvAMlYN/a+rdKqsyB9Cs=
+oras.land/oras-go/v2 v2.0.0-rc.4.0.20221111045309-74eeb52579de/go.mod h1:YGHvWBGuqRlZgUyXUIoKsR3lcuCOb3DAtG0SEsEw1iY=

internal/graph/graph.go

@@ -0,0 +1,61 @@
+/*
+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 graph
+
+import (
+	"context"
+	"encoding/json"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"oras.land/oras-go/v2/content"
+	"oras.land/oras/internal/docker"
+)
+
+// Successors returns the nodes directly pointed by the current node, picking
+// out subject and config descriptor if applicable.
+// Returning nil when no subject and config found.
+func Successors(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) (nodes []ocispec.Descriptor, subject, config *ocispec.Descriptor, err error) {
+	switch node.MediaType {
+	case docker.MediaTypeManifest, ocispec.MediaTypeImageManifest:
+		var fetched []byte
+		fetched, err = content.FetchAll(ctx, fetcher, node)
+		if err != nil {
+			return
+		}
+		var manifest ocispec.Manifest
+		if err = json.Unmarshal(fetched, &manifest); err != nil {
+			return
+		}
+		nodes = manifest.Layers
+		subject = manifest.Subject
+		config = &manifest.Config
+	case ocispec.MediaTypeArtifactManifest:
+		var fetched []byte
+		fetched, err = content.FetchAll(ctx, fetcher, node)
+		if err != nil {
+			return
+		}
+		var manifest ocispec.Artifact
+		if err = json.Unmarshal(fetched, &manifest); err != nil {
+			return
+		}
+		nodes = manifest.Blobs
+		subject = manifest.Subject
+	default:
+		nodes, err = content.Successors(ctx, fetcher, node)
+	}
+	return
+}

test/e2e/suite/scenario/oci_image.go

@@ -64,7 +64,7 @@ var _ = Describe("OCI image user:", Ordered, func() {
 			ORAS("pull", Reference(Host, repo, tag), "-v", "--config", files[0], "-o", pullRoot).
 				MatchStatus(statusKeys, true, 3).
 				WithWorkDir(tempDir).
-				WithDescription("should pull files with config").Exec()
+				WithDescription("pull files with config").Exec()
 
 			for _, f := range files {
 				Binary("diff", filepath.Join(f), filepath.Join(pullRoot, f)).