Repository: regclient/regclient
Pull request #470 (2023-06-17 00:49:59)

Improve error handling of artifact tree
<!--

Commits must be signed indicating your agreement to the [DCO](https://developercertificate.org/).
See [DCO missing](https://github.com/src-d/guide/blob/master/developer-community/fix-DCO.md) for steps to fix a missing signoff.

-->

### Fixes issue

`regctl artifact tree` doesn't give context for an error, or show partial output before the error was encountered.
<!-- If this is a bug fix, include "fixes #xxxx", or "closes #xxxx" -->

### Describe the change

This now returns the partial tree, and postpones error handling to include partial results.
<!-- Include the type of change: bug fix, new feature, breaking change, documentation update -->
<!-- Describe what was changed, why the change was made, and how it was implemented -->

### How to verify it

```
regctl artifact tree ghcr.io/sudo-bmitch/oci-sandbox:loop
```
<!-- Include steps that can be taken to verify the change -->

### Changelog text

Improve error handling of `regctl artifact tree`.
<!-- If the release changelog should have an entry for this, include it here -->

### Please verify and check that the pull request fulfills the following requirements

<!-- Mark the following with an [X] to verify they are included -->

- [X] Tests have been added or not applicable
- [X] Documentation has been added, updated, or not applicable
- [X] Changes have been rebased to main
- [X] Multiple commits to the same code have been squashed



Patch:
diff --git a/cmd/regctl/artifact.go b/cmd/regctl/artifact.go
index 42be06b..e85f866 100644
--- a/cmd/regctl/artifact.go
+++ b/cmd/regctl/artifact.go
@@ -793,11 +793,14 @@ func runArtifactTree(cmd *cobra.Command, args []string) error {
 
 	seen := []string{}
 	tr, err := treeAddResult(ctx, rc, r, seen, referrerOpts)
+	var twErr error
+	if tr != nil {
+		twErr = template.Writer(cmd.OutOrStdout(), artifactOpts.formatTree, tr)
+	}
 	if err != nil {
 		return err
 	}
-
-	return template.Writer(cmd.OutOrStdout(), artifactOpts.formatTree, tr)
+	return twErr
 }
 
 func treeAddResult(ctx context.Context, rc *regclient.RegClient, r ref.Ref, seen []string, rOpts []scheme.ReferrerOpts) (*treeResult, error) {
@@ -818,8 +821,7 @@ func treeAddResult(ctx context.Context, rc *regclient.RegClient, r ref.Ref, seen
 	// track already seen manifests
 	dig := m.GetDescriptor().Digest.String()
 	if sliceHasStr(seen, dig) {
-		// loop detected, consider making this an error
-		return &tr, fmt.Errorf("loop detected")
+		return &tr, fmt.Errorf("%w, already processed %s", ErrLoopEncountered, dig)
 	}
 	seen = append(seen, dig)
 
@@ -828,33 +830,35 @@ func treeAddResult(ctx context.Context, rc *regclient.RegClient, r ref.Ref, seen
 		tr.Child = []*treeResult{}
 		mi, ok := m.(manifest.Indexer)
 		if !ok {
-			return nil, fmt.Errorf("failed to convert a manifest list to indexer for %s", r.CommonName())
+			return &tr, fmt.Errorf("failed to convert a manifest list to indexer for %s", r.CommonName())
 		}
 		dl, err := mi.GetManifestList()
 		if err != nil {
-			return nil, fmt.Errorf("failed to get platforms for %s: %w", r.CommonName(), err)
+			return &tr, fmt.Errorf("failed to get platforms for %s: %w", r.CommonName(), err)
 		}
 		for _, d := range dl {
 			rChild := r
 			rChild.Tag = ""
 			rChild.Digest = d.Digest.String()
 			tChild, err := treeAddResult(ctx, rc, rChild, seen, rOpts)
-			if err != nil {
-				return nil, err
+			if tChild != nil {
+				tChild.ArtifactType = d.ArtifactType
+				if d.Platform != nil {
+					pCopy := *d.Platform
+					tChild.Platform = &pCopy
+				}
+				tr.Child = append(tr.Child, tChild)
 			}
-			tChild.ArtifactType = d.ArtifactType
-			if d.Platform != nil {
-				pCopy := *d.Platform
-				tChild.Platform = &pCopy
+			if err != nil {
+				return &tr, err
 			}
-			tr.Child = append(tr.Child, tChild)
 		}
 	}
 
 	// get referrers
 	rl, err := rc.ReferrerList(ctx, r, rOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("failed to check referrers for %s: %w", r.CommonName(), err)
+		return &tr, fmt.Errorf("failed to check referrers for %s: %w", r.CommonName(), err)
 	}
 	if len(rl.Descriptors) > 0 {
 		tr.Referrer = []*treeResult{}
@@ -863,15 +867,17 @@ func treeAddResult(ctx context.Context, rc *regclient.RegClient, r ref.Ref, seen
 			rReferrer.Tag = ""
 			rReferrer.Digest = d.Digest.String()
 			tReferrer, err := treeAddResult(ctx, rc, rReferrer, seen, rOpts)
-			if err != nil {
-				return nil, err
+			if tReferrer != nil {
+				tReferrer.ArtifactType = d.ArtifactType
+				if d.Platform != nil {
+					pCopy := *d.Platform
+					tReferrer.Platform = &pCopy
+				}
+				tr.Referrer = append(tr.Referrer, tReferrer)
 			}
-			tReferrer.ArtifactType = d.ArtifactType
-			if d.Platform != nil {
-				pCopy := *d.Platform
-				tReferrer.Platform = &pCopy
+			if err != nil {
+				return &tr, err
 			}
-			tr.Referrer = append(tr.Referrer, tReferrer)
 		}
 	}
 
diff --git a/cmd/regctl/error.go b/cmd/regctl/error.go
index 8ab4e3c..715dcf7 100644
--- a/cmd/regctl/error.go
+++ b/cmd/regctl/error.go
@@ -7,6 +7,8 @@ var (
 	ErrCredsNotFound = errors.New("auth creds not found")
 	// ErrInvalidInput indicates a required field is invalid
 	ErrInvalidInput = errors.New("invalid input")
+	// ErrLoopEncountered indicates a loop was encountered when walking the artifact tree
+	ErrLoopEncountered = errors.New("loop encountered")
 	// ErrMissingInput indicates a required field is missing
 	ErrMissingInput = errors.New("required input missing")
 	// ErrNotFound isn't there, search for your value elsewhere


Test patch:
diff --git a/cmd/regctl/artifact_test.go b/cmd/regctl/artifact_test.go
index 0ce2b5e..e1f9f57 100644
--- a/cmd/regctl/artifact_test.go
+++ b/cmd/regctl/artifact_test.go
@@ -125,6 +125,11 @@ func TestArtifactTree(t *testing.T) {
 			expectOut:   "Children",
 			outContains: true,
 		},
+		{
+			name:      "Loop",
+			args:      []string{"artifact", "tree", "ocidir://../../testdata/testrepo:loop"},
+			expectErr: ErrLoopEncountered,
+		},
 		{
 			name:        "Referrers",
 			args:        []string{"artifact", "tree", "ocidir://../../testdata/testrepo:v2"},
diff --git a/testdata/build-testrepo.sh b/testdata/build-testrepo.sh
index 562517e..73ec760 100755
--- a/testdata/build-testrepo.sh
+++ b/testdata/build-testrepo.sh
@@ -114,24 +114,27 @@ regctl image copy ocidir://testrepo:v3 "ocidir://testrepo:sha256-${v1_dig}.${v3_
 
 echo "child" | regctl artifact put --artifact-type application/example.child ocidir://testrepo:child
 dig_tag="$(regctl manifest head --format 'sha256-{{.GetDescriptor.Digest.Hex}}' ocidir://testrepo:child)"
-mt="$(regctl manifest head --format '{{.GetDescriptor.MediaType}}' ocidir://testrepo:child)"
-size="$(regctl manifest head --format '{{.GetDescriptor.Size}}' ocidir://testrepo:child)"
-digest="$(regctl manifest head --format '{{.GetDescriptor.Digest}}' ocidir://testrepo:child)"
+mt_child="$(regctl manifest head --format '{{.GetDescriptor.MediaType}}' ocidir://testrepo:child)"
+size_child="$(regctl manifest head --format '{{.GetDescriptor.Size}}' ocidir://testrepo:child)"
+digest_child="$(regctl manifest head --format '{{.GetDescriptor.Digest}}' ocidir://testrepo:child)"
+mt_loop="$(regctl manifest head --format '{{.GetDescriptor.MediaType}}' ocidir://testrepo:loop)"
+size_loop="$(regctl manifest head --format '{{.GetDescriptor.Size}}' ocidir://testrepo:loop)"
+digest_loop="$(regctl manifest head --format '{{.GetDescriptor.Digest}}' ocidir://testrepo:loop)"
 regctl manifest put ocidir://testrepo:loop <<EOF
 {
   "schemaVersion": 2,
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
     {
-      "mediaType": "${mt}",
-      "size": ${size},
-      "digest": "${digest}"
+      "mediaType": "${mt_child}",
+      "size": ${size_child},
+      "digest": "${digest_child}"
     }
   ],
   "subject": {
-    "mediaType": "${mt}",
-    "size": ${size},
-    "digest": "${digest}"
+    "mediaType": "${mt_child}",
+    "size": ${size_child},
+    "digest": "${digest_child}"
   }
 }
 EOF
@@ -141,9 +144,9 @@ regctl manifest put ocidir://testrepo:${dig_tag} <<EOF
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
     {
-      "mediaType": "${mt}",
-      "size": ${size},
-      "digest": "${digest}",
+      "mediaType": "${mt_loop}",
+      "size": ${size_loop},
+      "digest": "${digest_loop}",
       "artifactType": "application/example.child"
     }
   ]
diff --git a/testdata/testrepo/blobs/sha256/32f0c6dae73ed9712577cb7f859102434e0689773c38236bd33bf2a7ff39bab8 b/testdata/testrepo/blobs/sha256/32f0c6dae73ed9712577cb7f859102434e0689773c38236bd33bf2a7ff39bab8
new file mode 100644
index 0000000..7378cee
--- /dev/null
+++ b/testdata/testrepo/blobs/sha256/32f0c6dae73ed9712577cb7f859102434e0689773c38236bd33bf2a7ff39bab8
@@ -0,0 +1,12 @@
+{
+  "schemaVersion": 2,
+  "mediaType": "application/vnd.oci.image.index.v1+json",
+  "manifests": [
+    {
+      "mediaType": "application/vnd.oci.image.index.v1+json",
+      "size": 479,
+      "digest": "sha256:36b87a47db0b8826ac569e53563fe3d600c1304a82c69a1043ffe324d713b592",
+      "artifactType": "application/example.child"
+    }
+  ]
+}
diff --git a/testdata/testrepo/blobs/sha256/a2dec663d98b31020708afe24358d17434430805ed04857874879546ac2caa2e b/testdata/testrepo/blobs/sha256/a2dec663d98b31020708afe24358d17434430805ed04857874879546ac2caa2e
deleted file mode 100644
index 1bae029..0000000
--- a/testdata/testrepo/blobs/sha256/a2dec663d98b31020708afe24358d17434430805ed04857874879546ac2caa2e
+++ /dev/null
@@ -1,12 +0,0 @@
-{
-  "schemaVersion": 2,
-  "mediaType": "application/vnd.oci.image.index.v1+json",
-  "manifests": [
-    {
-      "mediaType": "application/vnd.oci.image.manifest.v1+json",
-      "size": 414,
-      "digest": "sha256:507bd066139b92463a30828d26fb9b9e10b2bb6b0e22a875b8062798c8bf9e5b",
-      "artifactType": "application/example.child"
-    }
-  ]
-}
diff --git a/testdata/testrepo/index.json b/testdata/testrepo/index.json
index fb4c45b..9bdfd35 100644
--- a/testdata/testrepo/index.json
+++ b/testdata/testrepo/index.json
@@ -1,1 +1,1 @@
-{"schemaVersion":2,"mediaType":"application/vnd.oci.image.index.v1+json","manifests":[{"mediaType":"application/vnd.oci.image.index.v1+json","size":964,"digest":"sha256:7ba20869dedc0090a7074b8574eb1f096f705f6f30082d08a3160b86848089d0","annotations":{"org.opencontainers.image.ref.name":"b1"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":964,"digest":"sha256:15a3231581118fd8ce33fd581cbae6958e990cbc99a6a7a1f50c63cce95d4ec5","annotations":{"org.opencontainers.image.ref.name":"b2"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":964,"digest":"sha256:98c0bb8411acb5dafef5223e13e8f553190ab3d1994062b5785788733cdbfca3","annotations":{"org.opencontainers.image.ref.name":"b3"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":1356,"digest":"sha256:4a88e72dd0e4245e6ecfbc6faae751eeeff82861f9ef39634bea07d77dbb1f40","annotations":{"org.opencontainers.image.ref.name":"v1"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":922,"digest":"sha256:4eef5c8459d36adab1f354e332af99915cb75dfe4b91e19cdd53ccfb33aa0418","annotations":{"org.opencontainers.image.ref.name":"v2"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":1141,"digest":"sha256:d0b099bf44f68230c262a2a2397d4caf9c74da80374b4674b8e41bc4c8579275","annotations":{"org.opencontainers.image.ref.name":"v3"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":394,"digest":"sha256:fcb04a33e5e9ecd98d30b05a784de6fe65a26e84316a995c55a12dfbb02132e8","annotations":{"org.opencontainers.image.ref.name":"a-docker"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":583,"digest":"sha256:54dd4e105916f7a95f7911c0328ce753d7bd08034538933c042cfae3f63bc573","annotations":{"org.opencontainers.image.ref.name":"a1"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":482,"digest":"sha256:7df92b086d7ec46a64ae0ff5652d371a7cf524d7076a925130807f464355c0d4","annotations":{"org.opencontainers.image.ref.name":"sha256-4eef5c8459d36adab1f354e332af99915cb75dfe4b91e19cdd53ccfb33aa0418"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":588,"digest":"sha256:559c971b91100f44900af38bb1391a7b2637afe674087ec68e5e6a92abcce57b","annotations":{"org.opencontainers.image.ref.name":"a2"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":576,"digest":"sha256:05d037f7d6b8fa8438c2d5f0cabd089358a56c057143527d8f60139e6799e171"},{"mediaType":"application/vnd.oci.image.index.v1+json","size":282,"digest":"sha256:876d3d65d4df98cdee450fb417721d1b568a4c56af1851126c18a5e22f2a1830","annotations":{"org.opencontainers.image.ref.name":"sha256-ef05efc8cfd478ac3140fce1297bd6b72dc5f5f1df31bfce690aa903a2c20310"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":576,"digest":"sha256:045226b7a97d5cc8a7c585f6816815ed662b0833734c6cbfa901137079e0997c"},{"mediaType":"application/vnd.oci.image.index.v1+json","size":282,"digest":"sha256:acf88dd4fa9307038a4099ff4ccd13ae77fe4d52a5f3d5edcb5a77142627ab52","annotations":{"org.opencontainers.image.ref.name":"sha256-4638b261096d1b87665ca154599c4ac1664a16ee1bc12b302f8aae24e336e071"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":576,"digest":"sha256:49fa909609a7f6c7589fb7cfa966e2cb04b682c5b2ca1da1de943a21e26bfe70"},{"mediaType":"application/vnd.oci.image.index.v1+json","size":282,"digest":"sha256:cf7f350eaff86a56089a93aff69bfa26aeac72d086bb5f0490bd46179260a77f","annotations":{"org.opencontainers.image.ref.name":"sha256-56ed84e8d4f983411218751bcb5d544e14e28d210101a12c63a1be03108de01b"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":1141,"digest":"sha256:d0b099bf44f68230c262a2a2397d4caf9c74da80374b4674b8e41bc4c8579275","annotations":{"org.opencontainers.image.ref.name":"sha256-4a88e72dd0e4245e6ecfbc6faae751eeeff82861f9ef39634bea07d77dbb1f40.d0b099bf44f68230.meta"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":414,"digest":"sha256:507bd066139b92463a30828d26fb9b9e10b2bb6b0e22a875b8062798c8bf9e5b","annotations":{"org.opencontainers.image.ref.name":"child"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":479,"digest":"sha256:36b87a47db0b8826ac569e53563fe3d600c1304a82c69a1043ffe324d713b592","annotations":{"org.opencontainers.image.ref.name":"loop"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":342,"digest":"sha256:a2dec663d98b31020708afe24358d17434430805ed04857874879546ac2caa2e","annotations":{"org.opencontainers.image.ref.name":"sha256-507bd066139b92463a30828d26fb9b9e10b2bb6b0e22a875b8062798c8bf9e5b"}}]}
\ No newline at end of file
+{"schemaVersion":2,"mediaType":"application/vnd.oci.image.index.v1+json","manifests":[{"mediaType":"application/vnd.oci.image.index.v1+json","size":964,"digest":"sha256:7ba20869dedc0090a7074b8574eb1f096f705f6f30082d08a3160b86848089d0","annotations":{"org.opencontainers.image.ref.name":"b1"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":964,"digest":"sha256:15a3231581118fd8ce33fd581cbae6958e990cbc99a6a7a1f50c63cce95d4ec5","annotations":{"org.opencontainers.image.ref.name":"b2"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":964,"digest":"sha256:98c0bb8411acb5dafef5223e13e8f553190ab3d1994062b5785788733cdbfca3","annotations":{"org.opencontainers.image.ref.name":"b3"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":1356,"digest":"sha256:4a88e72dd0e4245e6ecfbc6faae751eeeff82861f9ef39634bea07d77dbb1f40","annotations":{"org.opencontainers.image.ref.name":"v1"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":922,"digest":"sha256:4eef5c8459d36adab1f354e332af99915cb75dfe4b91e19cdd53ccfb33aa0418","annotations":{"org.opencontainers.image.ref.name":"v2"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":1141,"digest":"sha256:d0b099bf44f68230c262a2a2397d4caf9c74da80374b4674b8e41bc4c8579275","annotations":{"org.opencontainers.image.ref.name":"v3"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":394,"digest":"sha256:fcb04a33e5e9ecd98d30b05a784de6fe65a26e84316a995c55a12dfbb02132e8","annotations":{"org.opencontainers.image.ref.name":"a-docker"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":583,"digest":"sha256:54dd4e105916f7a95f7911c0328ce753d7bd08034538933c042cfae3f63bc573","annotations":{"org.opencontainers.image.ref.name":"a1"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":482,"digest":"sha256:7df92b086d7ec46a64ae0ff5652d371a7cf524d7076a925130807f464355c0d4","annotations":{"org.opencontainers.image.ref.name":"sha256-4eef5c8459d36adab1f354e332af99915cb75dfe4b91e19cdd53ccfb33aa0418"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":588,"digest":"sha256:559c971b91100f44900af38bb1391a7b2637afe674087ec68e5e6a92abcce57b","annotations":{"org.opencontainers.image.ref.name":"a2"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":576,"digest":"sha256:05d037f7d6b8fa8438c2d5f0cabd089358a56c057143527d8f60139e6799e171"},{"mediaType":"application/vnd.oci.image.index.v1+json","size":282,"digest":"sha256:876d3d65d4df98cdee450fb417721d1b568a4c56af1851126c18a5e22f2a1830","annotations":{"org.opencontainers.image.ref.name":"sha256-ef05efc8cfd478ac3140fce1297bd6b72dc5f5f1df31bfce690aa903a2c20310"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":576,"digest":"sha256:045226b7a97d5cc8a7c585f6816815ed662b0833734c6cbfa901137079e0997c"},{"mediaType":"application/vnd.oci.image.index.v1+json","size":282,"digest":"sha256:acf88dd4fa9307038a4099ff4ccd13ae77fe4d52a5f3d5edcb5a77142627ab52","annotations":{"org.opencontainers.image.ref.name":"sha256-4638b261096d1b87665ca154599c4ac1664a16ee1bc12b302f8aae24e336e071"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":576,"digest":"sha256:49fa909609a7f6c7589fb7cfa966e2cb04b682c5b2ca1da1de943a21e26bfe70"},{"mediaType":"application/vnd.oci.image.index.v1+json","size":282,"digest":"sha256:cf7f350eaff86a56089a93aff69bfa26aeac72d086bb5f0490bd46179260a77f","annotations":{"org.opencontainers.image.ref.name":"sha256-56ed84e8d4f983411218751bcb5d544e14e28d210101a12c63a1be03108de01b"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":1141,"digest":"sha256:d0b099bf44f68230c262a2a2397d4caf9c74da80374b4674b8e41bc4c8579275","annotations":{"org.opencontainers.image.ref.name":"sha256-4a88e72dd0e4245e6ecfbc6faae751eeeff82861f9ef39634bea07d77dbb1f40.d0b099bf44f68230.meta"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":414,"digest":"sha256:507bd066139b92463a30828d26fb9b9e10b2bb6b0e22a875b8062798c8bf9e5b","annotations":{"org.opencontainers.image.ref.name":"child"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":479,"digest":"sha256:36b87a47db0b8826ac569e53563fe3d600c1304a82c69a1043ffe324d713b592","annotations":{"org.opencontainers.image.ref.name":"loop"}},{"mediaType":"application/vnd.oci.image.index.v1+json","size":339,"digest":"sha256:32f0c6dae73ed9712577cb7f859102434e0689773c38236bd33bf2a7ff39bab8","annotations":{"org.opencontainers.image.ref.name":"sha256-507bd066139b92463a30828d26fb9b9e10b2bb6b0e22a875b8062798c8bf9e5b"}}]}
\ No newline at end of file