Skip to content

Commit

Permalink
feat(oci): support specifying the manifest digest when pushing it
Browse files Browse the repository at this point in the history 
See opencontainers/distribution-spec#494 and related work

Signed-off-by: Andrei Aaron <[email protected]>
  • Loading branch information
@andaaron
andaaron committed Jul 19, 2024
1 parent 26be383 commit 0da6b09
Show file tree
Hide file tree
Showing 26 changed files with 429 additions and 205 deletions.
39 changes: 4 additions & 35 deletions pkg/api/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
72 changes: 59 additions & 13 deletions pkg/api/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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"
Expand All @@ -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})
Expand All @@ -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
Expand Down Expand Up @@ -917,7 +943,9 @@ func (rh *RouteHandler) CheckBlob(response http.ResponseWriter, request *http.Re
name, ok := vars["name"]

if !ok || name == "" {
response.WriteHeader(http.StatusNotFound)
details := map[string]string{"name": ""}
e := apiErr.NewError(apiErr.NAME_UNKNOWN).AddDetail(details)
zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))

return
}
Expand All @@ -927,7 +955,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": ""}
e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))

return
}
Expand Down Expand Up @@ -1058,7 +1088,9 @@ func (rh *RouteHandler) GetBlob(response http.ResponseWriter, request *http.Requ
name, ok := vars["name"]

if !ok || name == "" {
response.WriteHeader(http.StatusNotFound)
details := map[string]string{"name": ""}
e := apiErr.NewError(apiErr.NAME_UNKNOWN).AddDetail(details)
zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))

return
}
Expand All @@ -1068,7 +1100,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": ""}
e := apiErr.NewError(apiErr.DIGEST_INVALID).AddDetail(details)
zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))

return
}
Expand Down Expand Up @@ -1167,7 +1201,9 @@ func (rh *RouteHandler) DeleteBlob(response http.ResponseWriter, request *http.R
name, ok := vars["name"]

if !ok || name == "" {
response.WriteHeader(http.StatusNotFound)
details := map[string]string{"name": ""}
e := apiErr.NewError(apiErr.NAME_UNKNOWN).AddDetail(details)
zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))

return
}
Expand All @@ -1176,7 +1212,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
}
Expand Down Expand Up @@ -1231,7 +1269,9 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request *
name, ok := vars["name"]

if !ok || name == "" {
response.WriteHeader(http.StatusNotFound)
details := map[string]string{"name": ""}
e := apiErr.NewError(apiErr.NAME_UNKNOWN).AddDetail(details)
zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))

return
}
Expand Down Expand Up @@ -1309,7 +1349,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
}
Expand Down Expand Up @@ -1555,7 +1597,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"
Expand All @@ -1581,14 +1623,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
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/api/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand All @@ -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
},
Expand All @@ -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
},
Expand All @@ -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
},
Expand All @@ -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
},
Expand Down
16 changes: 8 additions & 8 deletions pkg/extensions/search/search_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand All @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down
10 changes: 5 additions & 5 deletions pkg/extensions/sync/destination.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)).
Expand All @@ -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")
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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")
Expand Down
Loading

0 comments on commit 0da6b09

Please sign in to comment.