Repository: regclient/regclient
Pull request #363 (2023-02-17 01:50:03)

Fallback from head to get when digest is missing
<!--

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

Some registries are missing the docker digest header.
<!-- If this is a bug fix, include "fixes #xxxx", or "closes #xxxx" -->

### Describe the change

This triggers a fallback to a GET request on those registries.
<!-- 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 image digest registry.redhat.io/rhel8/buildah:8.7
```
<!-- Include steps that can be taken to verify the change -->

### Changelog text

Fallback to GET when HEAD request is missing digest header
<!-- 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 41ab730..780a09b 100644
--- a/cmd/regctl/artifact.go
+++ b/cmd/regctl/artifact.go
@@ -390,7 +390,7 @@ func runArtifactGet(cmd *cobra.Command, args []string) error {
 			return err
 		}
 		defer rdr.Close()
-		io.Copy(os.Stdout, rdr)
+		io.Copy(cmd.OutOrStdout(), rdr)
 	}
 
 	return nil
@@ -440,7 +440,7 @@ func runArtifactList(cmd *cobra.Command, args []string) error {
 	case "rawHeaders", "raw-headers", "headers":
 		artifactOpts.formatList = "{{ range $key,$vals := .Manifest.RawHeaders}}{{range $val := $vals}}{{printf \"%s: %s\\n\" $key $val }}{{end}}{{end}}"
 	}
-	return template.Writer(os.Stdout, artifactOpts.formatList, rl)
+	return template.Writer(cmd.OutOrStdout(), artifactOpts.formatList, rl)
 }
 
 func runArtifactPut(cmd *cobra.Command, args []string) error {
@@ -518,7 +518,7 @@ func runArtifactPut(cmd *cobra.Command, args []string) error {
 
 	var subjectDesc *types.Descriptor
 	if !rSubject.IsZero() {
-		smh, err := rc.ManifestHead(ctx, rSubject)
+		smh, err := rc.ManifestHead(ctx, rSubject, regclient.WithManifestRequireDigest())
 		if err != nil {
 			return fmt.Errorf("unable to find subject manifest: %w", err)
 		}
@@ -711,7 +711,7 @@ func runArtifactPut(cmd *cobra.Command, args []string) error {
 	if artifactOpts.byDigest && artifactOpts.formatPut == "" {
 		artifactOpts.formatPut = "{{ printf \"%s\\n\" .Manifest.GetDescriptor.Digest }}"
 	}
-	return template.Writer(os.Stdout, artifactOpts.formatPut, result)
+	return template.Writer(cmd.OutOrStdout(), artifactOpts.formatPut, result)
 }
 
 func runArtifactTree(cmd *cobra.Command, args []string) error {
diff --git a/cmd/regctl/index.go b/cmd/regctl/index.go
index a072c40..b76702f 100644
--- a/cmd/regctl/index.go
+++ b/cmd/regctl/index.go
@@ -350,7 +350,7 @@ func indexBuildDescList(ctx context.Context, rc *regclient.RegClient, r ref.Ref)
 		if err != nil {
 			return nil, err
 		}
-		mCopy, err := rc.ManifestHead(ctx, srcRef)
+		mCopy, err := rc.ManifestHead(ctx, srcRef, regclient.WithManifestRequireDigest())
 		if err != nil {
 			return nil, err
 		}
diff --git a/cmd/regctl/manifest.go b/cmd/regctl/manifest.go
index 9122de9..806bd20 100644
--- a/cmd/regctl/manifest.go
+++ b/cmd/regctl/manifest.go
@@ -88,6 +88,7 @@ var manifestOpts struct {
 	list          bool
 	platform      string
 	referrers     bool
+	requireDigest bool
 	requireList   bool
 }
 
@@ -101,6 +102,7 @@ func init() {
 	manifestHeadCmd.Flags().StringVarP(&manifestOpts.formatHead, "format", "", "", "Format output with go template syntax (use \"raw-body\" for the original manifest)")
 	manifestHeadCmd.Flags().BoolVarP(&manifestOpts.list, "list", "", true, "Do not resolve platform from manifest list (enabled by default)")
 	manifestHeadCmd.Flags().StringVarP(&manifestOpts.platform, "platform", "p", "", "Specify platform (e.g. linux/amd64 or local)")
+	manifestHeadCmd.Flags().BoolVarP(&manifestOpts.requireDigest, "require-digest", "", false, "Fallback to get request if digest is not received")
 	manifestHeadCmd.Flags().BoolVarP(&manifestOpts.requireList, "require-list", "", false, "Fail if manifest list is not received")
 	manifestHeadCmd.RegisterFlagCompletionFunc("platform", completeArgPlatform)
 	manifestHeadCmd.Flags().MarkHidden("list")
@@ -212,7 +214,7 @@ func runManifestDelete(cmd *cobra.Command, args []string) error {
 	defer rc.Close(ctx, r)
 
 	if r.Digest == "" && manifestOpts.forceTagDeref {
-		m, err := rc.ManifestHead(ctx, r)
+		m, err := rc.ManifestHead(ctx, r, regclient.WithManifestRequireDigest())
 		if err != nil {
 			return err
 		}
@@ -285,10 +287,10 @@ func runManifestDiff(cmd *cobra.Command, args []string) error {
 
 	mDiff := diff.Diff(strings.Split(string(m1Json), "\n"), strings.Split(string(m2Json), "\n"), diffOpts...)
 
-	_, err = fmt.Fprintln(os.Stdout, strings.Join(mDiff, "\n"))
+	_, err = fmt.Fprintln(cmd.OutOrStdout(), strings.Join(mDiff, "\n"))
 	return err
 	// TODO: support templating
-	// return template.Writer(os.Stdout, manifestOpts.format, mDiff)
+	// return template.Writer(cmd.OutOrStdout(), manifestOpts.format, mDiff)
 }
 
 func runManifestHead(cmd *cobra.Command, args []string) error {
@@ -311,8 +313,13 @@ func runManifestHead(cmd *cobra.Command, args []string) error {
 		"tag":  r.Tag,
 	}).Debug("Manifest head")
 
+	mOpts := []regclient.ManifestOpts{}
+	if manifestOpts.requireDigest || (!flagChanged(cmd, "require-digest") && !flagChanged(cmd, "format")) {
+		mOpts = append(mOpts, regclient.WithManifestRequireDigest())
+	}
+
 	// attempt to request only the headers, avoids Docker Hub rate limits
-	m, err := rc.ManifestHead(ctx, r)
+	m, err := rc.ManifestHead(ctx, r, mOpts...)
 	if err != nil {
 		return err
 	}
@@ -333,7 +340,7 @@ func runManifestHead(cmd *cobra.Command, args []string) error {
 			return fmt.Errorf("failed retrieving platform specific digest: %w", err)
 		}
 		r.Digest = desc.Digest.String()
-		m, err = rc.ManifestHead(ctx, r)
+		m, err = rc.ManifestHead(ctx, r, mOpts...)
 		if err != nil {
 			return fmt.Errorf("failed retrieving platform specific digest: %w", err)
 		}
@@ -345,7 +352,7 @@ func runManifestHead(cmd *cobra.Command, args []string) error {
 	case "rawHeaders", "raw-headers", "headers":
 		manifestOpts.formatHead = "{{ range $key,$vals := .RawHeaders}}{{range $val := $vals}}{{printf \"%s: %s\\n\" $key $val }}{{end}}{{end}}"
 	}
-	return template.Writer(os.Stdout, manifestOpts.formatHead, m)
+	return template.Writer(cmd.OutOrStdout(), manifestOpts.formatHead, m)
 }
 
 func runManifestGet(cmd *cobra.Command, args []string) error {
@@ -376,7 +383,7 @@ func runManifestGet(cmd *cobra.Command, args []string) error {
 	case "rawHeaders", "raw-headers", "headers":
 		manifestOpts.formatGet = "{{ range $key,$vals := .RawHeaders}}{{range $val := $vals}}{{printf \"%s: %s\\n\" $key $val }}{{end}}{{end}}"
 	}
-	return template.Writer(os.Stdout, manifestOpts.formatGet, m)
+	return template.Writer(cmd.OutOrStdout(), manifestOpts.formatGet, m)
 }
 
 func runManifestPut(cmd *cobra.Command, args []string) error {
@@ -423,5 +430,5 @@ func runManifestPut(cmd *cobra.Command, args []string) error {
 	if manifestOpts.byDigest && manifestOpts.formatPut == "" {
 		manifestOpts.formatPut = "{{ printf \"%s\\n\" .Manifest.GetDescriptor.Digest }}"
 	}
-	return template.Writer(os.Stdout, manifestOpts.formatPut, result)
+	return template.Writer(cmd.OutOrStdout(), manifestOpts.formatPut, result)
 }
diff --git a/cmd/regsync/root.go b/cmd/regsync/root.go
index 902fab2..f86d5f4 100644
--- a/cmd/regsync/root.go
+++ b/cmd/regsync/root.go
@@ -574,7 +574,7 @@ func (s ConfigSync) process(ctx context.Context, action string) error {
 
 // process a sync step
 func (s ConfigSync) processRef(ctx context.Context, src, tgt ref.Ref, action string) error {
-	mSrc, err := rc.ManifestHead(ctx, src)
+	mSrc, err := rc.ManifestHead(ctx, src, regclient.WithManifestRequireDigest())
 	if err != nil && errors.Is(err, types.ErrUnsupportedAPI) {
 		mSrc, err = rc.ManifestGet(ctx, src)
 	}
@@ -585,7 +585,7 @@ func (s ConfigSync) processRef(ctx context.Context, src, tgt ref.Ref, action str
 		}).Error("Failed to lookup source manifest")
 		return err
 	}
-	mTgt, err := rc.ManifestHead(ctx, tgt)
+	mTgt, err := rc.ManifestHead(ctx, tgt, regclient.WithManifestRequireDigest())
 	tgtExists := (err == nil)
 	tgtMatches := false
 	if err == nil && manifest.GetDigest(mSrc).String() == manifest.GetDigest(mTgt).String() {
diff --git a/image.go b/image.go
index 10144e6..b0eb1af 100644
--- a/image.go
+++ b/image.go
@@ -200,7 +200,7 @@ func (rc *RegClient) ImageCheckBase(ctx context.Context, r ref.Ref, opts ...Imag
 
 	// if the digest is available, check if that matches the base name
 	if opt.checkBaseDigest != "" {
-		baseMH, err := rc.ManifestHead(ctx, baseR)
+		baseMH, err := rc.ManifestHead(ctx, baseR, WithManifestRequireDigest())
 		if err != nil {
 			return err
 		}
@@ -398,7 +398,7 @@ func (rc *RegClient) imageCopyOpt(ctx context.Context, refSrc ref.Ref, refTgt re
 		opt.forceRecursive = true
 	}
 	// check if source and destination already match
-	mdh, errD := rc.ManifestHead(ctx, refTgt)
+	mdh, errD := rc.ManifestHead(ctx, refTgt, WithManifestRequireDigest())
 	if opt.forceRecursive {
 		// copy forced, unable to run below skips
 	} else if errD == nil && refTgt.Digest != "" && digest.Digest(refTgt.Digest) == mdh.GetDescriptor().Digest {
@@ -408,7 +408,7 @@ func (rc *RegClient) imageCopyOpt(ctx context.Context, refSrc ref.Ref, refTgt re
 		}).Info("Copy not needed, target already up to date")
 		return nil
 	} else if errD == nil && refTgt.Digest == "" {
-		msh, errS := rc.ManifestHead(ctx, refSrc)
+		msh, errS := rc.ManifestHead(ctx, refSrc, WithManifestRequireDigest())
 		if errS == nil && msh.GetDescriptor().Digest == mdh.GetDescriptor().Digest {
 			rc.log.WithFields(logrus.Fields{
 				"source": refSrc.Reference,
diff --git a/manifest.go b/manifest.go
index 8097e49..65daab4 100644
--- a/manifest.go
+++ b/manifest.go
@@ -10,8 +10,9 @@ import (
 )
 
 type manifestOpt struct {
-	d          types.Descriptor
-	schemeOpts []scheme.ManifestOpts
+	d             types.Descriptor
+	schemeOpts    []scheme.ManifestOpts
+	requireDigest bool
 }
 
 // ManifestOpts define options for the Manifest* commands
@@ -46,6 +47,13 @@ func WithManifestDesc(d types.Descriptor) ManifestOpts {
 	}
 }
 
+// WithManifestRequireDigest falls back from a HEAD to a GET request when digest headers aren't received.
+func WithManifestRequireDigest() ManifestOpts {
+	return func(opts *manifestOpt) {
+		opts.requireDigest = true
+	}
+}
+
 // ManifestDelete removes a manifest, including all tags pointing to that registry
 // The reference must include the digest to delete (see TagDelete for deleting a tag)
 // All tags pointing to the manifest will be deleted
@@ -86,12 +94,23 @@ func (rc *RegClient) ManifestGet(ctx context.Context, r ref.Ref, opts ...Manifes
 }
 
 // ManifestHead queries for the existence of a manifest and returns metadata (digest, media-type, size)
-func (rc *RegClient) ManifestHead(ctx context.Context, r ref.Ref) (manifest.Manifest, error) {
+func (rc *RegClient) ManifestHead(ctx context.Context, r ref.Ref, opts ...ManifestOpts) (manifest.Manifest, error) {
+	opt := manifestOpt{schemeOpts: []scheme.ManifestOpts{}}
+	for _, fn := range opts {
+		fn(&opt)
+	}
 	schemeAPI, err := rc.schemeGet(r.Scheme)
 	if err != nil {
 		return nil, err
 	}
-	return schemeAPI.ManifestHead(ctx, r)
+	m, err := schemeAPI.ManifestHead(ctx, r)
+	if err != nil {
+		return m, err
+	}
+	if opt.requireDigest && m.GetDescriptor().Digest.String() == "" {
+		m, err = schemeAPI.ManifestGet(ctx, r)
+	}
+	return m, err
 }
 
 // ManifestPut pushes a manifest


Test patch:
diff --git a/cmd/regctl/manifest_test.go b/cmd/regctl/manifest_test.go
new file mode 100644
index 0000000..d40adbe
--- /dev/null
+++ b/cmd/regctl/manifest_test.go
@@ -0,0 +1,74 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/regclient/regclient/types"
+)
+
+func TestManifestHead(t *testing.T) {
+	tt := []struct {
+		name        string
+		args        []string
+		expectErr   error
+		expectOut   string
+		outContains bool
+	}{
+		{
+			name:      "Missing arg",
+			args:      []string{"manifest", "head"},
+			expectErr: fmt.Errorf("accepts 1 arg(s), received 0"),
+		},
+		{
+			name:      "Invalid ref",
+			args:      []string{"manifest", "head", "invalid*ref"},
+			expectErr: fmt.Errorf("invalid reference \"%s\"", "invalid*ref"),
+		},
+		{
+			name:      "Missing manifest",
+			args:      []string{"manifest", "head", "ocidir://../../testdata/testrepo:missing"},
+			expectErr: types.ErrNotFound,
+		},
+		{
+			name:        "Digest",
+			args:        []string{"manifest", "head", "ocidir://../../testdata/testrepo:v1"},
+			expectOut:   "sha256:",
+			outContains: true,
+		},
+		{
+			name:        "Platform amd64",
+			args:        []string{"manifest", "head", "ocidir://../../testdata/testrepo:v1", "--platform", "linux/amd64"},
+			expectOut:   "sha256:",
+			outContains: true,
+		},
+		{
+			name:      "Platform unknown",
+			args:      []string{"manifest", "head", "ocidir://../../testdata/testrepo:v1", "--platform", "linux/unknown"},
+			expectErr: ErrNotFound,
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			out, err := cobraTest(t, tc.args...)
+			if tc.expectErr != nil {
+				if err == nil {
+					t.Errorf("did not receive expected error: %v", tc.expectErr)
+				} else if !errors.Is(err, tc.expectErr) && err.Error() != tc.expectErr.Error() {
+					t.Errorf("unexpected error, received %v, expected %v", err, tc.expectErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Errorf("returned unexpected error: %v", err)
+				return
+			}
+			if (!tc.outContains && out != tc.expectOut) || (tc.outContains && !strings.Contains(out, tc.expectOut)) {
+				t.Errorf("unexpected output, expected %s, received %s", tc.expectOut, out)
+			}
+		})
+	}
+
+}
diff --git a/manifest_test.go b/manifest_test.go
index aa8b460..b7ac39e 100644
--- a/manifest_test.go
+++ b/manifest_test.go
@@ -29,6 +29,7 @@ func TestManifest(t *testing.T) {
 	getTag := "get"
 	headTag := "head"
 	noheadTag := "nohead"
+	nodigestTag := "nodigest"
 	missingTag := "missing"
 	digest1 := digest.FromString("example1")
 	digest2 := digest.FromString("example2")
@@ -102,6 +103,36 @@ func TestManifest(t *testing.T) {
 				},
 			},
 		},
+		{
+			ReqEntry: reqresp.ReqEntry{
+				Name:   "Get nodigest",
+				Method: "GET",
+				Path:   "/v2" + repoPath + "/manifests/" + nodigestTag,
+			},
+			RespEntry: reqresp.RespEntry{
+				Status: http.StatusOK,
+				Headers: http.Header{
+					"Content-Length":        {fmt.Sprintf("%d", mLen)},
+					"Content-Type":          []string{types.MediaTypeDocker2Manifest},
+					"Docker-Content-Digest": []string{mDigest.String()},
+				},
+				Body: mBody,
+			},
+		},
+		{
+			ReqEntry: reqresp.ReqEntry{
+				Name:   "Head nodigest",
+				Method: "HEAD",
+				Path:   "/v2" + repoPath + "/manifests/" + nodigestTag,
+			},
+			RespEntry: reqresp.RespEntry{
+				Status: http.StatusOK,
+				Headers: http.Header{
+					"Content-Length": {fmt.Sprintf("%d", mLen)},
+					"Content-Type":   []string{types.MediaTypeDocker2Manifest},
+				},
+			},
+		},
 		{
 			ReqEntry: reqresp.ReqEntry{
 				Name:   "Get nohead",
@@ -213,6 +244,23 @@ func TestManifest(t *testing.T) {
 			t.Errorf("Unexpected digest: %s", mHead.GetDescriptor().Digest.String())
 		}
 	})
+	t.Run("Head no digest", func(t *testing.T) {
+		headRef, err := ref.New(tsURL.Host + repoPath + ":" + nodigestTag)
+		if err != nil {
+			t.Errorf("Failed creating getRef: %v", err)
+		}
+		mHead, err := rc.ManifestHead(ctx, headRef, WithManifestRequireDigest())
+		if err != nil {
+			t.Errorf("Failed running ManifestHead: %v", err)
+			return
+		}
+		if manifest.GetMediaType(mHead) != types.MediaTypeDocker2Manifest {
+			t.Errorf("Unexpected media type: %s", manifest.GetMediaType(mHead))
+		}
+		if mHead.GetDescriptor().Digest != mDigest {
+			t.Errorf("Unexpected digest: %s", mHead.GetDescriptor().Digest.String())
+		}
+	})
 	t.Run("Head No Head", func(t *testing.T) {
 		noheadRef, err := ref.New("nohead." + tsURL.Host + repoPath + ":" + noheadTag)
 		if err != nil {