feat(oci): support specifying the manifest digest when pushing it

See opencontainers/distribution-spec#494 and related work Signed-off-by: Andrei Aaron <[email protected]>
andaaron · Jul 19, 2024 · bb02ef8 · bb02ef8
1 parent 26be383
commit bb02ef8
Show file tree

Hide file tree

Showing 26 changed files with 417 additions and 201 deletions.
diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go
@@ -11198,13 +11198,7 @@ func TestSupportedDigestAlgorithms(t *testing.T) {
 
 		client := resty.New()
 
-		// The server picks canonical digests when tags are pushed
-		// See https://github.com/opencontainers/distribution-spec/issues/494
-		// It would be nice to be able to push tags with other digest algorithms and verify those are returned
-		// but there is no way to specify a client preference
-		// so all we can do is verify the correct algorithm is returned
-
-		expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
+		expectedDigestStr := image.DigestForAlgorithm(godigest.SHA512).String()
 
 		verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
 		verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
@@ -11238,13 +11232,7 @@ func TestSupportedDigestAlgorithms(t *testing.T) {
 
 		client := resty.New()
 
-		// The server picks canonical digests when tags are pushed
-		// See https://github.com/opencontainers/distribution-spec/issues/494
-		// It would be nice to be able to push tags with other digest algorithms and verify those are returned
-		// but there is no way to specify a client preference
-		// so all we can do is verify the correct algorithm is returned
-
-		expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
+		expectedDigestStr := image.DigestForAlgorithm(godigest.SHA384).String()
 
 		verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
 		verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
@@ -11266,18 +11254,10 @@ func TestSupportedDigestAlgorithms(t *testing.T) {
 
 		client := resty.New()
 
-		// The server picks canonical digests when tags are pushed
-		// See https://github.com/opencontainers/distribution-spec/issues/494
-		// It would be nice to be able to push tags with other digest algorithms and verify those are returned
-		// but there is no way to specify a client preference
-		// so all we can do is verify the correct algorithm is returned
-		expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
+		expectedDigestStr := multiarch.DigestForAlgorithm(godigest.SHA512).String()
 
 		verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
 		verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
-
-		// While the expected multiarch manifest digest is always using the canonical algorithm
-		// the sub-imgage manifest digest can use any algorith
 		verifyReturnedManifestDigest(t, client, baseURL, name,
 			subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
 		verifyReturnedManifestDigest(t, client, baseURL, name,
@@ -11303,9 +11283,6 @@ func TestSupportedDigestAlgorithms(t *testing.T) {
 
 		expectedDigestStr := multiarch.DigestForAlgorithm(godigest.SHA512).String()
 		verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
-
-		// While the expected multiarch manifest digest is always using the canonical algorithm
-		// the sub-imgage manifest digest can use any algorith
 		verifyReturnedManifestDigest(t, client, baseURL, name,
 			subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
 		verifyReturnedManifestDigest(t, client, baseURL, name,
@@ -11328,18 +11305,10 @@ func TestSupportedDigestAlgorithms(t *testing.T) {
 
 		client := resty.New()
 
-		// The server picks canonical digests when tags are pushed
-		// See https://github.com/opencontainers/distribution-spec/issues/494
-		// It would be nice to be able to push tags with other digest algorithms and verify those are returned
-		// but there is no way to specify a client preference
-		// so all we can do is verify the correct algorithm is returned
-		expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
+		expectedDigestStr := multiarch.DigestForAlgorithm(godigest.SHA384).String()
 
 		verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
 		verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
-
-		// While the expected multiarch manifest digest is always using the canonical algorithm
-		// the sub-imgage manifest digest can use any algorith
 		verifyReturnedManifestDigest(t, client, baseURL, name,
 			subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
 		verifyReturnedManifestDigest(t, client, baseURL, name,

diff --git a/pkg/api/routes.go b/pkg/api/routes.go
@@ -610,7 +610,9 @@ func (rh *RouteHandler) GetReferrers(response http.ResponseWriter, request *http
 	digest, err := godigest.Parse(digestStr)
 
 	if !ok || digestStr == "" || err != nil {
-		response.WriteHeader(http.StatusBadRequest)
+		details := map[string]string{"digest": digestStr}
+		e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+		zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 		return
 	}
@@ -660,6 +662,7 @@ func (rh *RouteHandler) GetReferrers(response http.ResponseWriter, request *http
 // @Produce json
 // @Param   name         path    string     true        "repository name"
 // @Param   reference    path    string     true        "image reference or digest"
+// @Param   digest       query   string     false       "manifest digest"
 // @Header  201 {object} constants.DistContentDigestKey
 // @Success 201 {string} string "created"
 // @Failure 400 {string} string "bad request"
@@ -686,6 +689,29 @@ func (rh *RouteHandler) UpdateManifest(response http.ResponseWriter, request *ht
 		return
 	}
 
+	var expectedDigest godigest.Digest
+
+	// if a digest is present in the query parameters use it
+	digests, ok := request.URL.Query()["digest"]
+	if ok {
+		if len(digests) != 1 {
+			details := map[string]string{"digest": fmt.Sprintf("%v", digests)}
+			e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+			zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
+
+			return
+		}
+
+		expectedDigest = godigest.Digest(digests[0])
+		if err := expectedDigest.Validate(); err != nil {
+			details := map[string]string{"digest": digests[0]}
+			e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+			zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
+
+			return
+		}
+	}
+
 	mediaType := request.Header.Get("Content-Type")
 	if !storageCommon.IsSupportedMediaType(mediaType) {
 		err := apiErr.NewError(apiErr.MANIFEST_INVALID).AddDetail(map[string]string{"mediaType": mediaType})
@@ -704,7 +730,7 @@ func (rh *RouteHandler) UpdateManifest(response http.ResponseWriter, request *ht
 		return
 	}
 
-	digest, subjectDigest, err := imgStore.PutImageManifest(name, reference, mediaType, body)
+	digest, subjectDigest, err := imgStore.PutImageManifest(name, reference, mediaType, body, expectedDigest)
 	if err != nil {
 		details := zerr.GetDetails(err)
 		if errors.Is(err, zerr.ErrRepoNotFound) { //nolint:gocritic // errorslint conflicts with gocritic:IfElseChain
@@ -927,7 +953,9 @@ func (rh *RouteHandler) CheckBlob(response http.ResponseWriter, request *http.Re
 	digestStr, ok := vars["digest"]
 
 	if !ok || digestStr == "" {
-		response.WriteHeader(http.StatusNotFound)
+		details := map[string]string{"digest": digestStr}
+		e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+		zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 		return
 	}
@@ -1068,7 +1096,9 @@ func (rh *RouteHandler) GetBlob(response http.ResponseWriter, request *http.Requ
 	digestStr, ok := vars["digest"]
 
 	if !ok || digestStr == "" {
-		response.WriteHeader(http.StatusNotFound)
+		details := map[string]string{"digest": digestStr}
+		e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+		zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 		return
 	}
@@ -1176,7 +1206,9 @@ func (rh *RouteHandler) DeleteBlob(response http.ResponseWriter, request *http.R
 	digest, err := godigest.Parse(digestStr)
 
 	if !ok || digestStr == "" || err != nil {
-		response.WriteHeader(http.StatusNotFound)
+		details := map[string]string{"digest": digestStr}
+		e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+		zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 		return
 	}
@@ -1309,7 +1341,9 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request *
 	digests, ok := request.URL.Query()["digest"]
 	if ok {
 		if len(digests) != 1 {
-			response.WriteHeader(http.StatusBadRequest)
+			details := map[string]string{"digest": fmt.Sprintf("%v", digests)}
+			e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+			zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 			return
 		}
@@ -1555,7 +1589,7 @@ func (rh *RouteHandler) PatchBlobUpload(response http.ResponseWriter, request *h
 // @Param   session_id   path    string     true        "upload session_id"
 // @Param   digest       query   string     true        "blob/layer digest"
 // @Success 201 {string} string "created"
-// @Header  202 {string} Location "/v2/{name}/blobs/uploads/{digest}"
+// @Header  202 {string} Location "/v2/{name}/blobs/uploads/{session_id}"
 // @Header  200 {object} constants.DistContentDigestKey
 // @Failure 404 {string} string "not found"
 // @Failure 500 {string} string "internal server error"
@@ -1581,14 +1615,18 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
 
 	digests, ok := request.URL.Query()["digest"]
 	if !ok || len(digests) != 1 {
-		response.WriteHeader(http.StatusBadRequest)
+		details := map[string]string{"digest": fmt.Sprintf("%v", digests)}
+		e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+		zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 		return
 	}
 
 	digest, err := godigest.Parse(digests[0])
 	if err != nil {
-		response.WriteHeader(http.StatusBadRequest)
+		details := map[string]string{"digest": digests[0]}
+		e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
+		zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
 		return
 	}

diff --git a/pkg/api/routes_test.go b/pkg/api/routes_test.go
@@ -233,8 +233,8 @@ func TestRoutes(t *testing.T) {
 					"reference": "reference",
 				},
 				&mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", zerr.ErrRepoNotFound
 					},
@@ -248,8 +248,8 @@ func TestRoutes(t *testing.T) {
 				},
 
 				&mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", zerr.ErrManifestNotFound
 					},
@@ -262,8 +262,8 @@ func TestRoutes(t *testing.T) {
 					"reference": "reference",
 				},
 				&mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", zerr.ErrBadManifest
 					},
@@ -276,8 +276,8 @@ func TestRoutes(t *testing.T) {
 					"reference": "reference",
 				},
 				&mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", zerr.ErrBlobNotFound
 					},
@@ -291,8 +291,8 @@ func TestRoutes(t *testing.T) {
 					"reference": "reference",
 				},
 				&mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", zerr.ErrRepoBadVersion
 					},

diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go
@@ -4552,8 +4552,8 @@ func TestMetaDBWhenSigningImages(t *testing.T) {
 			Convey("imageIsSignature fails", func() {
 				// make image store ignore the wrong format of the input
 				ctlr.StoreController.DefaultStore = mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", nil
 					},
@@ -5632,8 +5632,8 @@ func TestMetaDBWhenDeletingImages(t *testing.T) {
 
 			Convey("imageIsSignature fails", func() {
 				ctlr.StoreController.DefaultStore = mocks.MockedImageStore{
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", nil
 					},
@@ -5658,8 +5658,8 @@ func TestMetaDBWhenDeletingImages(t *testing.T) {
 
 						return configBlob, nil
 					},
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", nil
 					},
@@ -5688,8 +5688,8 @@ func TestMetaDBWhenDeletingImages(t *testing.T) {
 
 						return configBlob, nil
 					},
-					PutImageManifestFn: func(repo, reference, mediaType string, body []byte) (godigest.Digest,
-						godigest.Digest, error,
+					PutImageManifestFn: func(repo, reference, mediaType string, body []byte,
+						expectedDigest godigest.Digest) (godigest.Digest, godigest.Digest, error,
 					) {
 						return "", "", ErrTestError
 					},

diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go
@@ -113,7 +113,7 @@ func (registry *DestinationRegistry) CommitImage(imageReference types.ImageRefer
 	// is image manifest
 	switch mediaType {
 	case ispec.MediaTypeImageManifest:
-		if err := registry.copyManifest(repo, manifestBlob, reference, tempImageStore); err != nil {
+		if err := registry.copyManifest(repo, manifestBlob, reference, manifestDigest, tempImageStore); err != nil {
 			if errors.Is(err, zerr.ErrImageLintAnnotations) {
 				registry.log.Error().Str("errorType", common.TypeOf(err)).
 					Err(err).Msg("couldn't upload manifest because of missing annotations")
@@ -148,7 +148,7 @@ func (registry *DestinationRegistry) CommitImage(imageReference types.ImageRefer
 				return err
 			}
 
-			if err := registry.copyManifest(repo, manifestBuf, manifest.Digest.String(),
+			if err := registry.copyManifest(repo, manifestBuf, manifest.Digest.String(), manifest.Digest,
 				tempImageStore); err != nil {
 				if errors.Is(err, zerr.ErrImageLintAnnotations) {
 					registry.log.Error().Str("errorType", common.TypeOf(err)).
@@ -161,7 +161,7 @@ func (registry *DestinationRegistry) CommitImage(imageReference types.ImageRefer
 			}
 		}
 
-		_, _, err = imageStore.PutImageManifest(repo, reference, mediaType, manifestBlob)
+		_, _, err = imageStore.PutImageManifest(repo, reference, mediaType, manifestBlob, manifestDigest)
 		if err != nil {
 			registry.log.Error().Str("errorType", common.TypeOf(err)).Str("repo", repo).Str("reference", reference).
 				Err(err).Msg("couldn't upload manifest")
@@ -193,7 +193,7 @@ func (registry *DestinationRegistry) CleanupImage(imageReference types.ImageRefe
 }
 
 func (registry *DestinationRegistry) copyManifest(repo string, manifestContent []byte, reference string,
-	tempImageStore storageTypes.ImageStore,
+	manifestDigest digest.Digest, tempImageStore storageTypes.ImageStore,
 ) error {
 	imageStore := registry.storeController.GetImageStore(repo)
 
@@ -226,7 +226,7 @@ func (registry *DestinationRegistry) copyManifest(repo string, manifestContent [
 	}
 
 	digest, _, err := imageStore.PutImageManifest(repo, reference,
-		ispec.MediaTypeImageManifest, manifestContent)
+		ispec.MediaTypeImageManifest, manifestContent, manifestDigest)
 	if err != nil {
 		registry.log.Error().Str("errorType", common.TypeOf(err)).
 			Err(err).Msg("couldn't upload manifest")

diff --git a/pkg/extensions/sync/references/cosign.go b/pkg/extensions/sync/references/cosign.go
@@ -135,7 +135,7 @@ func (ref CosignReference) SyncReferences(ctx context.Context, localRepo, remote
 
 		// push manifest
 		referenceDigest, _, err := imageStore.PutImageManifest(localRepo, cosignTag,
-			ispec.MediaTypeImageManifest, manifestBuf)
+			ispec.MediaTypeImageManifest, manifestBuf, digest)
 		if err != nil {
 			ref.log.Error().Str("errorType", common.TypeOf(err)).
 				Str("repository", localRepo).Str("subject", subjectDigestStr).

diff --git a/pkg/extensions/sync/references/oci.go b/pkg/extensions/sync/references/oci.go
@@ -224,7 +224,7 @@ func syncManifest(ctx context.Context, client *client.Client, imageStore storage
 	}
 
 	refDigest, _, err = imageStore.PutImageManifest(localRepo, desc.Digest.String(),
-		desc.MediaType, OCIRefBuf)
+		desc.MediaType, OCIRefBuf, desc.Digest)
 	if err != nil {
 		log.Error().Str("errorType", common.TypeOf(err)).
 			Str("repository", localRepo).Str("subject", subjectDigestStr).

diff --git a/pkg/extensions/sync/references/referrers_tag.go b/pkg/extensions/sync/references/referrers_tag.go
@@ -85,7 +85,9 @@ func (ref TagReferences) SyncReferences(ctx context.Context, localRepo, remoteRe
 		return refsDigests, err
 	}
 
-	skipTagRefs, err := ref.canSkipReferences(localRepo, subjectDigestStr, string(godigest.FromBytes(indexContent)))
+	indexDigest := godigest.FromBytes(indexContent)
+
+	skipTagRefs, err := ref.canSkipReferences(localRepo, subjectDigestStr, indexDigest.String())
 	if err != nil {
 		ref.log.Error().Err(err).Str("repository", localRepo).Str("subject", subjectDigestStr).
 			Msg("couldn't check if the upstream index with referrers tag for image can be skipped")
@@ -131,7 +133,7 @@ func (ref TagReferences) SyncReferences(ctx context.Context, localRepo, remoteRe
 
 	referrersTag := getReferrersTagFromSubjectDigest(subjectDigestStr)
 
-	_, _, err = imageStore.PutImageManifest(localRepo, referrersTag, index.MediaType, indexContent)
+	_, _, err = imageStore.PutImageManifest(localRepo, referrersTag, index.MediaType, indexContent, indexDigest)
 	if err != nil {
 		ref.log.Error().Str("errorType", common.TypeOf(err)).
 			Str("repository", localRepo).Str("subject", subjectDigestStr).