diff --git a/errors/errors.go b/errors/errors.go index 9c974f456b..572e5e42fc 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -68,7 +68,6 @@ var ( ErrTagMetaNotFound = errors.New("repodb: tag metadata not found for given repo and tag names") ErrTypeAssertionFailed = errors.New("storage: failed DatabaseDriver type assertion") ErrInvalidRequestParams = errors.New("resolver: parameter sent has invalid value") - ErrOrphanSignature = errors.New("repodb: signature detected but signed image doesn't exit") ErrBadCtxFormat = errors.New("type assertion failed") ErrEmptyRepoName = errors.New("repodb: repo name can't be empty string") ErrEmptyTag = errors.New("repodb: tag can't be empty string") diff --git a/pkg/api/routes.go b/pkg/api/routes.go index d97ee2c500..c063e9d658 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -434,10 +434,7 @@ func (rh *RouteHandler) GetManifest(response http.ResponseWriter, request *http. if rh.c.RepoDB != nil { err := meta.OnGetManifest(name, reference, content, rh.c.StoreController, rh.c.RepoDB, rh.c.Log) - - if errors.Is(err, zerr.ErrOrphanSignature) { - rh.c.Log.Error().Err(err).Msg("image is an orphan signature") - } else if err != nil { + if err != nil { response.WriteHeader(http.StatusInternalServerError) return @@ -647,9 +644,7 @@ func (rh *RouteHandler) UpdateManifest(response http.ResponseWriter, request *ht if rh.c.RepoDB != nil { err := meta.OnUpdateManifest(name, reference, mediaType, digest, body, rh.c.StoreController, rh.c.RepoDB, rh.c.Log) - if errors.Is(err, zerr.ErrOrphanSignature) { - rh.c.Log.Error().Err(err).Msg("pushed image is an orphan signature") - } else if err != nil { + if err != nil { response.WriteHeader(http.StatusInternalServerError) return @@ -746,9 +741,7 @@ func (rh *RouteHandler) DeleteManifest(response http.ResponseWriter, request *ht if rh.c.RepoDB != nil { err := meta.OnDeleteManifest(name, reference, mediaType, manifestDigest, manifestBlob, rh.c.StoreController, rh.c.RepoDB, rh.c.Log) - if errors.Is(err, zerr.ErrOrphanSignature) { - rh.c.Log.Error().Err(err).Msg("pushed image is an orphan signature") - } else if err != nil { + if err != nil { response.WriteHeader(http.StatusInternalServerError) return diff --git a/pkg/extensions/sync/signatures.go b/pkg/extensions/sync/signatures.go index 1b93840eb2..6f2e73c8a4 100644 --- a/pkg/extensions/sync/signatures.go +++ b/pkg/extensions/sync/signatures.go @@ -180,7 +180,7 @@ func (sig *signaturesCopier) syncCosignSignature(localRepo, remoteRepo, digestSt } // push manifest - _, err = imageStore.PutImageManifest(localRepo, cosignTag, + signatureDigest, err := imageStore.PutImageManifest(localRepo, cosignTag, ispec.MediaTypeImageManifest, cosignManifestBuf) if err != nil { sig.log.Error().Str("errorType", common.TypeOf(err)). @@ -193,9 +193,10 @@ func (sig *signaturesCopier) syncCosignSignature(localRepo, remoteRepo, digestSt sig.log.Debug().Str("repository", localRepo).Str("digest", digestStr). Msg("trying to sync cosign signature for repo digest") - err = repodb.SetMetadataFromInput(localRepo, cosignTag, ispec.MediaTypeImageManifest, - godigest.FromBytes(cosignManifestBuf), cosignManifestBuf, sig.storeController.GetImageStore(localRepo), - sig.repoDB, sig.log) + err := sig.repoDB.AddManifestSignature(localRepo, godigest.Digest(digestStr), repodb.SignatureMetadata{ + SignatureType: repodb.CosignType, + SignatureDigest: signatureDigest.String(), + }) if err != nil { return fmt.Errorf("failed to set metadata for cosign signature '%s@%s': %w", localRepo, digestStr, err) } @@ -258,7 +259,7 @@ func (sig *signaturesCopier) syncORASRefs(localRepo, remoteRepo, digestStr strin } } - _, err = imageStore.PutImageManifest(localRepo, ref.Digest.String(), + signatureDigest, err := imageStore.PutImageManifest(localRepo, ref.Digest.String(), oras.MediaTypeArtifactManifest, body) if err != nil { sig.log.Error().Str("errorType", common.TypeOf(err)). @@ -272,8 +273,10 @@ func (sig *signaturesCopier) syncORASRefs(localRepo, remoteRepo, digestStr strin sig.log.Debug().Str("repository", localRepo).Str("digest", digestStr). Msg("trying to sync oras artifact for digest") - err = repodb.SetMetadataFromInput(localRepo, ref.Digest.String(), ref.MediaType, - ref.Digest, body, sig.storeController.GetImageStore(localRepo), sig.repoDB, sig.log) + err := sig.repoDB.AddManifestSignature(localRepo, godigest.Digest(digestStr), repodb.SignatureMetadata{ + SignatureType: repodb.NotationType, + SignatureDigest: signatureDigest.String(), + }) if err != nil { return fmt.Errorf("failed to set metadata for oras artifact '%s@%s': %w", localRepo, digestStr, err) } @@ -371,9 +374,22 @@ func (sig *signaturesCopier) syncOCIRefs(localRepo, remoteRepo, digestStr string if sig.repoDB != nil { sig.log.Debug().Str("repository", localRepo).Str("digest", digestStr).Msg("trying to add OCI refs for repo digest") - err = repodb.SetMetadataFromInput(localRepo, digestStr, ref.MediaType, - digest, OCIRefBody, sig.storeController.GetImageStore(localRepo), - sig.repoDB, sig.log) + isSig, _, signedManifestDig, err := storage.CheckIsImageSignature(localRepo, OCIRefBody, ref.Digest.String()) + if err != nil { + return fmt.Errorf("failed to set metadata for OCI ref in '%s@%s': %w", localRepo, digestStr, err) + } + + if isSig { + err = sig.repoDB.AddManifestSignature(localRepo, signedManifestDig, repodb.SignatureMetadata{ + SignatureType: repodb.NotationType, + SignatureDigest: digestStr, + }) + } else { + err = repodb.SetImageMetaFromInput(localRepo, digestStr, ref.MediaType, + digest, OCIRefBody, sig.storeController.GetImageStore(localRepo), + sig.repoDB, sig.log) + } + if err != nil { return fmt.Errorf("failed to set metadata for OCI ref in '%s@%s': %w", localRepo, digestStr, err) } diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index cdec5210a1..0669e0785b 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -41,6 +41,7 @@ import ( syncconf "zotregistry.io/zot/pkg/extensions/config/sync" "zotregistry.io/zot/pkg/extensions/sync" logger "zotregistry.io/zot/pkg/log" + "zotregistry.io/zot/pkg/meta/repodb" "zotregistry.io/zot/pkg/storage" "zotregistry.io/zot/pkg/storage/local" "zotregistry.io/zot/pkg/test" @@ -3637,6 +3638,97 @@ func TestSignatures(t *testing.T) { }) } +func getPortFromBaseURL(baseURL string) string { + slice := strings.Split(baseURL, ":") + + return slice[len(slice)-1] +} + +func TestSyncedSignaturesRepoDB(t *testing.T) { + Convey("Verify that repodb update correctly when syncing a signature", t, func() { + repoName := "signed-repo" + tag := "random-signed-image" + updateDuration := 30 * time.Minute + + // Create source registry + + sctlr, srcBaseURL, srcDir, _, _ := makeUpstreamServer(t, false, false) + t.Log(srcDir) + srcPort := getPortFromBaseURL(srcBaseURL) + + scm := test.NewControllerManager(sctlr) + scm.StartAndWait(sctlr.Config.HTTP.Port) + defer scm.StopServer() + + // Push an image + destImage, err := test.GetRandomImage(tag) + So(err, ShouldBeNil) + + signedImageDigest, err := destImage.Digest() + So(err, ShouldBeNil) + + err = test.UploadImage(destImage, srcBaseURL, repoName) + So(err, ShouldBeNil) + + err = test.SignImageUsingNotary(repoName+":"+tag, srcPort) + So(err, ShouldBeNil) + + err = test.SignImageUsingCosign(repoName+":"+tag, srcPort) + So(err, ShouldBeNil) + + // Create destination registry + var ( + regex = ".*" + semver = false + tlsVerify = false + defaultVal = true + ) + + syncConfig := &syncconf.Config{ + Enable: &defaultVal, + Registries: []syncconf.RegistryConfig{ + { + Content: []syncconf.Content{ + { + Prefix: repoName, + Tags: &syncconf.Tags{Regex: ®ex, Semver: &semver}, + }, + }, + URLs: []string{srcBaseURL}, + PollInterval: updateDuration, + TLSVerify: &tlsVerify, + CertDir: "", + OnDemand: true, + }, + }, + } + + dctlr, destBaseURL, dstDir, _ := makeDownstreamServer(t, false, syncConfig) + t.Log(dstDir) + + dcm := test.NewControllerManager(dctlr) + dcm.StartAndWait(dctlr.Config.HTTP.Port) + defer dcm.StopServer() + + // Trigger SyncOnDemand + resp, err := resty.R().Get(destBaseURL + "/v2/" + repoName + "/manifests/" + tag) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + repoMeta, err := dctlr.RepoDB.GetRepoMeta(repoName) + So(err, ShouldBeNil) + So(repoMeta.Tags, ShouldContainKey, tag) + So(len(repoMeta.Tags), ShouldEqual, 1) + So(repoMeta.Signatures, ShouldContainKey, signedImageDigest.String()) + + imageSignatures := repoMeta.Signatures[signedImageDigest.String()] + So(imageSignatures, ShouldContainKey, repodb.CosignType) + So(len(imageSignatures[repodb.CosignType]), ShouldEqual, 1) + So(imageSignatures, ShouldContainKey, repodb.NotationType) + So(len(imageSignatures[repodb.NotationType]), ShouldEqual, 1) + }) +} + func TestOnDemandRetryGoroutine(t *testing.T) { Convey("Verify ondemand sync retries in background on error", t, func() { srcPort := test.GetFreePort() diff --git a/pkg/extensions/sync/utils.go b/pkg/extensions/sync/utils.go index 00bd99c775..fb80bdfe13 100644 --- a/pkg/extensions/sync/utils.go +++ b/pkg/extensions/sync/utils.go @@ -346,7 +346,7 @@ func pushSyncedLocalImage(localRepo, reference, localCachePath string, } if repoDB != nil { - err = repodb.SetMetadataFromInput(localRepo, reference, mediaType, + err = repodb.SetImageMetaFromInput(localRepo, reference, mediaType, manifestDigest, manifestBlob, imageStore, repoDB, log) if err != nil { return fmt.Errorf("failed to set metadata for image '%s %s': %w", localRepo, reference, err) @@ -403,7 +403,7 @@ func copyManifest(localRepo string, manifestContent []byte, reference string, re } if repoDB != nil { - err = repodb.SetMetadataFromInput(localRepo, reference, ispec.MediaTypeImageManifest, + err = repodb.SetImageMetaFromInput(localRepo, reference, ispec.MediaTypeImageManifest, digest, manifestContent, imageStore, repoDB, log) if err != nil { log.Error().Str("errorType", common.TypeOf(err)). diff --git a/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go b/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go index ef4f4b2f72..eb64629961 100644 --- a/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go +++ b/pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go @@ -265,14 +265,10 @@ func (bdw DBWrapper) SetReferrer(repo string, referredDigest godigest.Digest, re // create a new object repoMeta := repodb.RepoMetadata{ - Name: repo, - Tags: map[string]repodb.Descriptor{}, - Statistics: map[string]repodb.DescriptorStatistics{ - referredDigest.String(): {}, - }, - Signatures: map[string]repodb.ManifestSignatures{ - referredDigest.String(): {}, - }, + Name: repo, + Tags: map[string]repodb.Descriptor{}, + Statistics: map[string]repodb.DescriptorStatistics{}, + Signatures: map[string]repodb.ManifestSignatures{}, Referrers: map[string][]repodb.ReferrerInfo{ referredDigest.String(): { referrer, @@ -404,32 +400,22 @@ func (bdw *DBWrapper) SetRepoReference(repo string, reference string, manifestDi repoMetaBlob := buck.Get([]byte(repo)) - // object not found - if len(repoMetaBlob) == 0 { - var err error - // create a new object - repoMeta := repodb.RepoMetadata{ - Name: repo, - Tags: map[string]repodb.Descriptor{}, - Statistics: map[string]repodb.DescriptorStatistics{}, - Signatures: map[string]repodb.ManifestSignatures{}, - Referrers: map[string][]repodb.ReferrerInfo{}, - } + repoMeta := repodb.RepoMetadata{ + Name: repo, + Tags: map[string]repodb.Descriptor{}, + Statistics: map[string]repodb.DescriptorStatistics{}, + Signatures: map[string]repodb.ManifestSignatures{}, + Referrers: map[string][]repodb.ReferrerInfo{}, + } - repoMetaBlob, err = json.Marshal(repoMeta) + // object not found + if len(repoMetaBlob) > 0 { + err := json.Unmarshal(repoMetaBlob, &repoMeta) if err != nil { return err } } - // object found - var repoMeta repodb.RepoMetadata - - err := json.Unmarshal(repoMetaBlob, &repoMeta) - if err != nil { - return err - } - if !common.ReferenceIsDigest(reference) { repoMeta.Tags[reference] = repodb.Descriptor{ Digest: manifestDigest.String(), @@ -449,7 +435,7 @@ func (bdw *DBWrapper) SetRepoReference(repo string, reference string, manifestDi repoMeta.Referrers[manifestDigest.String()] = []repodb.ReferrerInfo{} } - repoMetaBlob, err = json.Marshal(repoMeta) + repoMetaBlob, err := json.Marshal(repoMeta) if err != nil { return err } @@ -747,8 +733,33 @@ func (bdw *DBWrapper) AddManifestSignature(repo string, signedManifestDigest god buck := tx.Bucket([]byte(bolt.RepoMetadataBucket)) repoMetaBlob := buck.Get([]byte(repo)) - if repoMetaBlob == nil { - return zerr.ErrManifestMetaNotFound + + if len(repoMetaBlob) == 0 { + var err error + // create a new object + repoMeta := repodb.RepoMetadata{ + Name: repo, + Tags: map[string]repodb.Descriptor{}, + Signatures: map[string]repodb.ManifestSignatures{ + signedManifestDigest.String(): { + sygMeta.SignatureType: []repodb.SignatureInfo{ + { + SignatureManifestDigest: sygMeta.SignatureDigest, + LayersInfo: sygMeta.LayersInfo, + }, + }, + }, + }, + Statistics: map[string]repodb.DescriptorStatistics{}, + Referrers: map[string][]repodb.ReferrerInfo{}, + } + + repoMetaBlob, err = json.Marshal(repoMeta) + if err != nil { + return err + } + + return buck.Put([]byte(repo), repoMetaBlob) } var repoMeta repodb.RepoMetadata @@ -769,17 +780,10 @@ func (bdw *DBWrapper) AddManifestSignature(repo string, signedManifestDigest god signatureSlice := manifestSignatures[sygMeta.SignatureType] if !common.SignatureAlreadyExists(signatureSlice, sygMeta) { - if sygMeta.SignatureType == repodb.NotationType { - signatureSlice = append(signatureSlice, repodb.SignatureInfo{ - SignatureManifestDigest: sygMeta.SignatureDigest, - LayersInfo: sygMeta.LayersInfo, - }) - } else if sygMeta.SignatureType == repodb.CosignType { - signatureSlice = []repodb.SignatureInfo{{ - SignatureManifestDigest: sygMeta.SignatureDigest, - LayersInfo: sygMeta.LayersInfo, - }} - } + signatureSlice = append(signatureSlice, repodb.SignatureInfo{ + SignatureManifestDigest: sygMeta.SignatureDigest, + LayersInfo: sygMeta.LayersInfo, + }) } manifestSignatures[sygMeta.SignatureType] = signatureSlice diff --git a/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go b/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go index fb674b76d3..7cce82e765 100644 --- a/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go +++ b/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go @@ -621,6 +621,27 @@ func (dwr *DBWrapper) AddManifestSignature(repo string, signedManifestDigest god ) error { repoMeta, err := dwr.GetRepoMeta(repo) if err != nil { + if errors.Is(err, zerr.ErrRepoMetaNotFound) { + repoMeta = repodb.RepoMetadata{ + Name: repo, + Tags: map[string]repodb.Descriptor{}, + Statistics: map[string]repodb.DescriptorStatistics{}, + Signatures: map[string]repodb.ManifestSignatures{ + signedManifestDigest.String(): { + sygMeta.SignatureType: []repodb.SignatureInfo{ + { + SignatureManifestDigest: sygMeta.SignatureDigest, + LayersInfo: sygMeta.LayersInfo, + }, + }, + }, + }, + Referrers: map[string][]repodb.ReferrerInfo{}, + } + + return dwr.SetRepoMeta(repo, repoMeta) + } + return err } @@ -635,26 +656,17 @@ func (dwr *DBWrapper) AddManifestSignature(repo string, signedManifestDigest god signatureSlice := manifestSignatures[sygMeta.SignatureType] if !common.SignatureAlreadyExists(signatureSlice, sygMeta) { - if sygMeta.SignatureType == repodb.NotationType { - signatureSlice = append(signatureSlice, repodb.SignatureInfo{ - SignatureManifestDigest: sygMeta.SignatureDigest, - LayersInfo: sygMeta.LayersInfo, - }) - } else if sygMeta.SignatureType == repodb.CosignType { - signatureSlice = []repodb.SignatureInfo{{ - SignatureManifestDigest: sygMeta.SignatureDigest, - LayersInfo: sygMeta.LayersInfo, - }} - } + signatureSlice = append(signatureSlice, repodb.SignatureInfo{ + SignatureManifestDigest: sygMeta.SignatureDigest, + LayersInfo: sygMeta.LayersInfo, + }) } manifestSignatures[sygMeta.SignatureType] = signatureSlice repoMeta.Signatures[signedManifestDigest.String()] = manifestSignatures - err = dwr.SetRepoMeta(repoMeta.Name, repoMeta) - - return err + return dwr.SetRepoMeta(repoMeta.Name, repoMeta) } func (dwr *DBWrapper) DeleteSignature(repo string, signedManifestDigest godigest.Digest, diff --git a/pkg/meta/repodb/repodb_test.go b/pkg/meta/repodb/repodb_test.go index f81530622e..5a272addbf 100644 --- a/pkg/meta/repodb/repodb_test.go +++ b/pkg/meta/repodb/repodb_test.go @@ -994,6 +994,34 @@ func RunRepoDBTests(repoDB repodb.RepoDB, preparationFuncs ...func() error) { So(err, ShouldNotBeNil) }) + Convey("Test AddImageSignature with inverted order", func() { + var ( + repo1 = "repo1" + tag1 = "0.0.1" + manifestDigest1 = godigest.FromString("fake-manifest1") + ) + + err := repoDB.AddManifestSignature(repo1, manifestDigest1, repodb.SignatureMetadata{ + SignatureType: "cosign", + SignatureDigest: "digest", + }) + So(err, ShouldBeNil) + + err = repoDB.SetRepoReference(repo1, tag1, manifestDigest1, ispec.MediaTypeImageManifest) + So(err, ShouldBeNil) + + err = repoDB.SetManifestData(manifestDigest1, repodb.ManifestData{}) + So(err, ShouldBeNil) + + repoMeta, err := repoDB.GetRepoMeta(repo1) + So(err, ShouldBeNil) + So(repoMeta.Signatures[manifestDigest1.String()]["cosign"][0].SignatureManifestDigest, + ShouldResemble, "digest") + + _, err = repoDB.GetManifestMeta(repo1, "badDigest") + So(err, ShouldNotBeNil) + }) + Convey("Test DeleteSignature", func() { var ( repo1 = "repo1" @@ -1004,7 +1032,7 @@ func RunRepoDBTests(repoDB repodb.RepoDB, preparationFuncs ...func() error) { err := repoDB.SetRepoReference(repo1, tag1, manifestDigest1, ispec.MediaTypeImageManifest) So(err, ShouldBeNil) - err = repoDB.SetManifestMeta(repo1, manifestDigest1, repodb.ManifestMetadata{}) + err = repoDB.SetManifestData(manifestDigest1, repodb.ManifestData{}) So(err, ShouldBeNil) err = repoDB.AddManifestSignature(repo1, manifestDigest1, repodb.SignatureMetadata{ diff --git a/pkg/meta/repodb/storage_parsing.go b/pkg/meta/repodb/storage_parsing.go index 9a5d97084f..dd9b706ce0 100644 --- a/pkg/meta/repodb/storage_parsing.go +++ b/pkg/meta/repodb/storage_parsing.go @@ -65,16 +65,6 @@ func ParseRepo(repo string, repoDB RepoDB, storeController storage.StoreControll return err } - type foundSignatureData struct { - repo string - tag string - signatureType string - signedManifestDigest string - signatureDigest string - } - - var signaturesFound []foundSignatureData - for _, manifest := range indexContent.Manifests { tag, hasTag := manifest.Annotations[ispec.AnnotationRefName] @@ -85,6 +75,7 @@ func ParseRepo(repo string, repoDB RepoDB, storeController storage.StoreControll return err } + // this check helps reduce unecesary reads from storage if manifestMetaIsPresent && hasTag { err = repoDB.SetRepoReference(repo, tag, manifest.Digest, manifest.MediaType) if err != nil { @@ -105,28 +96,27 @@ func ParseRepo(repo string, repoDB RepoDB, storeController storage.StoreControll } isSignature, signatureType, signedManifestDigest, err := storage.CheckIsImageSignature(repo, - manifestBlob, tag, storeController) + manifestBlob, tag) if err != nil { - if errors.Is(err, zerr.ErrOrphanSignature) { - continue - } else { - log.Error().Err(err).Str("repository", repo).Str("tag", tag). - Msg("load-repo: failed checking if image is signature for specified image") + log.Error().Err(err).Str("repository", repo).Str("tag", tag). + Msg("load-repo: failed checking if image is signature for specified image") - return err - } + return err } if isSignature { - // We'll ignore signatures now because the order in which the signed image and signature are added into - // the DB matters. First we add the normal images then the signatures - signaturesFound = append(signaturesFound, foundSignatureData{ - repo: repo, - tag: tag, - signatureType: signatureType, - signedManifestDigest: signedManifestDigest.String(), - signatureDigest: digest.String(), - }) + err := repoDB.AddManifestSignature(repo, signedManifestDigest, + SignatureMetadata{ + SignatureType: signatureType, + SignatureDigest: digest.String(), + }) + if err != nil { + log.Error().Err(err).Str("repository", repo).Str("tag", tag). + Str("manifestDigest", signedManifestDigest.String()). + Msg("load-repo: failed set signature meta for signed image manifest digest") + + return err + } continue } @@ -137,7 +127,7 @@ func ParseRepo(repo string, repoDB RepoDB, storeController storage.StoreControll reference = manifest.Digest.String() } - err = SetMetadataFromInput(repo, reference, manifest.MediaType, manifest.Digest, manifestBlob, + err = SetImageMetaFromInput(repo, reference, manifest.MediaType, manifest.Digest, manifestBlob, imageStore, repoDB, log) if err != nil { log.Error().Err(err).Str("repository", repo).Str("tag", tag). @@ -147,22 +137,6 @@ func ParseRepo(repo string, repoDB RepoDB, storeController storage.StoreControll } } - // manage the signatures found - for _, sigData := range signaturesFound { - err := repoDB.AddManifestSignature(repo, godigest.Digest(sigData.signedManifestDigest), - SignatureMetadata{ - SignatureType: sigData.signatureType, - SignatureDigest: sigData.signatureDigest, - }) - if err != nil { - log.Error().Err(err).Str("repository", sigData.repo).Str("tag", sigData.tag). - Str("manifestDigest", sigData.signedManifestDigest). - Msg("load-repo: failed set signature meta for signed image manifest digest") - - return err - } - } - return nil } @@ -266,7 +240,7 @@ func NewIndexData(repoName string, indexBlob []byte, imageStore storage.ImageSto // SetMetadataFromInput tries to set manifest metadata and update repo metadata by adding the current tag // (in case the reference is a tag). The function expects image manifests and indexes (multi arch images). -func SetMetadataFromInput(repo, reference, mediaType string, digest godigest.Digest, descriptorBlob []byte, +func SetImageMetaFromInput(repo, reference, mediaType string, digest godigest.Digest, descriptorBlob []byte, imageStore storage.ImageStore, repoDB RepoDB, log log.Logger, ) error { switch mediaType { diff --git a/pkg/meta/repodb/storage_parsing_test.go b/pkg/meta/repodb/storage_parsing_test.go index 9852dfe588..3e5426df1a 100644 --- a/pkg/meta/repodb/storage_parsing_test.go +++ b/pkg/meta/repodb/storage_parsing_test.go @@ -264,7 +264,7 @@ func TestParseStorageErrors(t *testing.T) { }) } -func TestParseStorageWithStorage(t *testing.T) { +func TestParseStorageWithBoltDB(t *testing.T) { Convey("Boltdb", t, func() { rootDir := t.TempDir() @@ -313,7 +313,7 @@ func TestParseStorageDynamoWrapper(t *testing.T) { } func RunParseStorageTests(rootDir string, repoDB repodb.RepoDB) { - Convey("test", func() { + Convey("Test with simple case", func() { imageStore := local.NewImageStore(rootDir, false, 0, false, false, log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), nil, nil) @@ -408,7 +408,7 @@ func RunParseStorageTests(rootDir string, repoDB repodb.RepoDB) { } }) - Convey("Ignore orphan signatures", func() { + Convey("Accept orphan signatures", func() { imageStore := local.NewImageStore(rootDir, false, 0, false, false, log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), nil, nil) @@ -429,10 +429,13 @@ func RunParseStorageTests(rootDir string, repoDB repodb.RepoDB) { So(err, ShouldBeNil) // add mock cosign signature without pushing the signed image - _, _, manifest, err = test.GetRandomImageComponents(100) + image, err := test.GetRandomImage("") So(err, ShouldBeNil) - signatureTag, err := test.GetCosignSignatureTagForManifest(manifest) + signatureTag, err := test.GetCosignSignatureTagForManifest(image.Manifest) + So(err, ShouldBeNil) + + missingImageDigest, err := image.Digest() So(err, ShouldBeNil) // get the body of the signature @@ -460,9 +463,14 @@ func RunParseStorageTests(rootDir string, repoDB repodb.RepoDB) { ) So(err, ShouldBeNil) + for _, desc := range repos[0].Tags { + So(desc.Digest, ShouldNotResemble, missingImageDigest.String()) + } + So(len(repos), ShouldEqual, 1) So(repos[0].Tags, ShouldContainKey, "tag1") So(repos[0].Tags, ShouldNotContainKey, signatureTag) + So(repos[0].Signatures, ShouldContainKey, missingImageDigest.String()) }) Convey("Check statistics after load", func() { diff --git a/pkg/meta/update.go b/pkg/meta/update.go index d4ae82f445..2baada5171 100644 --- a/pkg/meta/update.go +++ b/pkg/meta/update.go @@ -1,11 +1,8 @@ package meta import ( - "errors" - godigest "github.com/opencontainers/go-digest" - zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/common" "zotregistry.io/zot/pkg/meta/repodb" @@ -21,15 +18,8 @@ func OnUpdateManifest(repo, reference, mediaType string, digest godigest.Digest, imgStore := storeController.GetImageStore(repo) // check if image is a signature - isSignature, signatureType, signedManifestDigest, err := storage.CheckIsImageSignature(repo, body, reference, - storeController) + isSignature, signatureType, signedManifestDigest, err := storage.CheckIsImageSignature(repo, body, reference) if err != nil { - if errors.Is(err, zerr.ErrOrphanSignature) { - log.Warn().Err(err).Msg("image has signature format but it doesn't sign any image") - - return zerr.ErrOrphanSignature - } - log.Error().Err(err).Msg("can't check if image is a signature or not") if err := imgStore.DeleteImageManifest(repo, reference, false); err != nil { @@ -53,7 +43,7 @@ func OnUpdateManifest(repo, reference, mediaType string, digest godigest.Digest, metadataSuccessfullySet = false } } else { - err := repodb.SetMetadataFromInput(repo, reference, mediaType, digest, body, + err := repodb.SetImageMetaFromInput(repo, reference, mediaType, digest, body, imgStore, repoDB, log) if err != nil { metadataSuccessfullySet = false @@ -85,14 +75,8 @@ func OnDeleteManifest(repo, reference, mediaType string, digest godigest.Digest, imgStore := storeController.GetImageStore(repo) isSignature, signatureType, signedManifestDigest, err := storage.CheckIsImageSignature(repo, manifestBlob, - reference, storeController) + reference) if err != nil { - if errors.Is(err, zerr.ErrOrphanSignature) { - log.Warn().Err(err).Msg("image has signature format but it doesn't sign any image") - - return zerr.ErrOrphanSignature - } - log.Error().Err(err).Msg("can't check if image is a signature or not") return err @@ -148,15 +132,8 @@ func OnGetManifest(name, reference string, body []byte, storeController storage.StoreController, repoDB repodb.RepoDB, log log.Logger, ) error { // check if image is a signature - isSignature, _, _, err := storage.CheckIsImageSignature(name, body, reference, - storeController) + isSignature, _, _, err := storage.CheckIsImageSignature(name, body, reference) if err != nil { - if errors.Is(err, zerr.ErrOrphanSignature) { - log.Warn().Err(err).Msg("image has signature format but it doesn't sign any image") - - return err - } - log.Error().Err(err).Msg("can't check if manifest is a signature or not") return err diff --git a/pkg/meta/update_test.go b/pkg/meta/update_test.go index 60abd33b7b..96b8fb1f3d 100644 --- a/pkg/meta/update_test.go +++ b/pkg/meta/update_test.go @@ -6,7 +6,6 @@ import ( "testing" "time" - notreg "github.com/notaryproject/notation-go/registry" godigest "github.com/opencontainers/go-digest" ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" @@ -93,32 +92,6 @@ func TestOnUpdateManifest(t *testing.T) { func TestUpdateErrors(t *testing.T) { Convey("Update operations", t, func() { - Convey("On UpdateManifest", func() { - imageStore := mocks.MockedImageStore{} - storeController := storage.StoreController{DefaultStore: &imageStore} - repoDB := mocks.RepoDBMock{} - log := log.NewLogger("debug", "") - - Convey("zerr.ErrOrphanSignature", func() { - manifestContent := ispec.Manifest{ - Subject: &ispec.Descriptor{ - Digest: "123", - }, - ArtifactType: notreg.ArtifactTypeNotation, - } - manifestBlob, err := json.Marshal(manifestContent) - So(err, ShouldBeNil) - - imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { - return []byte{}, "", "", zerr.ErrManifestNotFound - } - - err = meta.OnUpdateManifest("repo", "tag1", "", "digest", manifestBlob, - storeController, repoDB, log) - So(err, ShouldNotBeNil) - }) - }) - Convey("On DeleteManifest", func() { imageStore := mocks.MockedImageStore{} storeController := storage.StoreController{DefaultStore: &imageStore} @@ -126,36 +99,13 @@ func TestUpdateErrors(t *testing.T) { log := log.NewLogger("debug", "") Convey("CheckIsImageSignature errors", func() { - manifestContent := ispec.Manifest{ - Subject: &ispec.Descriptor{ - Digest: "123", - }, - ArtifactType: notreg.ArtifactTypeNotation, - } - manifestBlob, err := json.Marshal(manifestContent) - So(err, ShouldBeNil) + badManifestBlob := []byte("bad") imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { return []byte{}, "", "", zerr.ErrManifestNotFound } - err = meta.OnDeleteManifest("repo", "tag1", "digest", "media", manifestBlob, - storeController, repoDB, log) - So(err, ShouldNotBeNil) - - imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { - return []byte{}, "", "", ErrTestError - } - - err = meta.OnDeleteManifest("repo", "tag1", "digest", "media", manifestBlob, - storeController, repoDB, log) - So(err, ShouldNotBeNil) - - imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { - return []byte{}, "", "", zerr.ErrManifestNotFound - } - - err = meta.OnDeleteManifest("repo", "tag1", "digest", "media", manifestBlob, + err := meta.OnDeleteManifest("repo", "tag1", "digest", "media", badManifestBlob, storeController, repoDB, log) So(err, ShouldNotBeNil) }) @@ -179,39 +129,24 @@ func TestUpdateErrors(t *testing.T) { log := log.NewLogger("debug", "") Convey("CheckIsImageSignature errors", func() { - manifestContent := ispec.Manifest{ - Subject: &ispec.Descriptor{ - Digest: "123", - }, - ArtifactType: notreg.ArtifactTypeNotation, - } - manifestBlob, err := json.Marshal(manifestContent) - So(err, ShouldBeNil) + badManifestBlob := []byte("bad") imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { return []byte{}, "", "", zerr.ErrManifestNotFound } - err = meta.OnGetManifest("repo", "tag1", manifestBlob, - storeController, repoDB, log) - So(err, ShouldNotBeNil) - - imageStore.GetImageManifestFn = func(repo, reference string) ([]byte, godigest.Digest, string, error) { - return []byte{}, "", "", ErrTestError - } - - err = meta.OnGetManifest("repo", "tag1", manifestBlob, + err := meta.OnGetManifest("repo", "tag1", badManifestBlob, storeController, repoDB, log) So(err, ShouldNotBeNil) }) }) - Convey("SetMetadataFromInput", func() { + Convey("SetImageMetaFromInput", func() { imageStore := mocks.MockedImageStore{} repoDB := mocks.RepoDBMock{} log := log.NewLogger("debug", "") - err := repodb.SetMetadataFromInput("repo", "ref", ispec.MediaTypeImageManifest, "digest", + err := repodb.SetImageMetaFromInput("repo", "ref", ispec.MediaTypeImageManifest, "digest", []byte("BadManifestBlob"), imageStore, repoDB, log) So(err, ShouldNotBeNil) @@ -228,12 +163,12 @@ func TestUpdateErrors(t *testing.T) { return []byte("{}"), nil } - err = repodb.SetMetadataFromInput("repo", string(godigest.FromString("reference")), "", "digest", + err = repodb.SetImageMetaFromInput("repo", string(godigest.FromString("reference")), "", "digest", manifestBlob, imageStore, repoDB, log) So(err, ShouldBeNil) }) - Convey("SetMetadataFromInput SetData errors", func() { + Convey("SetImageMetaFromInput SetData errors", func() { imageStore := mocks.MockedImageStore{} log := log.NewLogger("debug", "") @@ -242,12 +177,12 @@ func TestUpdateErrors(t *testing.T) { return ErrTestError }, } - err := repodb.SetMetadataFromInput("repo", "ref", ispec.MediaTypeImageManifest, "digest", + err := repodb.SetImageMetaFromInput("repo", "ref", ispec.MediaTypeImageManifest, "digest", []byte("{}"), imageStore, repoDB, log) So(err, ShouldNotBeNil) }) - Convey("SetMetadataFromInput SetIndexData errors", func() { + Convey("SetImageMetaFromInput SetIndexData errors", func() { imageStore := mocks.MockedImageStore{} log := log.NewLogger("debug", "") @@ -256,12 +191,12 @@ func TestUpdateErrors(t *testing.T) { return ErrTestError }, } - err := repodb.SetMetadataFromInput("repo", "ref", ispec.MediaTypeImageIndex, "digest", + err := repodb.SetImageMetaFromInput("repo", "ref", ispec.MediaTypeImageIndex, "digest", []byte("{}"), imageStore, repoDB, log) So(err, ShouldNotBeNil) }) - Convey("SetMetadataFromInput SetReferrer errors", func() { + Convey("SetImageMetaFromInput SetReferrer errors", func() { imageStore := mocks.MockedImageStore{ GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { return []byte("{}"), nil @@ -275,7 +210,7 @@ func TestUpdateErrors(t *testing.T) { }, } - err := repodb.SetMetadataFromInput("repo", "ref", ispec.MediaTypeImageManifest, "digest", + err := repodb.SetImageMetaFromInput("repo", "ref", ispec.MediaTypeImageManifest, "digest", []byte(`{"subject": {"digest": "subjDigest"}}`), imageStore, repoDB, log) So(err, ShouldNotBeNil) }) diff --git a/pkg/storage/common.go b/pkg/storage/common.go index 8907462918..dfa190773d 100644 --- a/pkg/storage/common.go +++ b/pkg/storage/common.go @@ -26,12 +26,6 @@ const ( NotationType = "notation" ) -func SignatureMediaTypes() map[string]bool { - return map[string]bool{ - notreg.ArtifactTypeNotation: true, - } -} - func GetTagsByIndex(index ispec.Index) []string { tags := make([]string, 0) @@ -720,7 +714,6 @@ func IsNonDistributable(mediaType string) bool { // // - error: any errors that occur. func CheckIsImageSignature(repoName string, manifestBlob []byte, reference string, - storeController StoreController, ) (bool, string, godigest.Digest, error) { var manifestContent ispec.Manifest @@ -732,20 +725,8 @@ func CheckIsImageSignature(repoName string, manifestBlob []byte, reference strin manifestArtifactType := zcommon.GetManifestArtifactType(manifestContent) // check notation signature - if _, ok := SignatureMediaTypes()[manifestArtifactType]; ok && manifestContent.Subject != nil { - imgStore := storeController.GetImageStore(repoName) - - _, signedImageManifestDigest, _, err := imgStore.GetImageManifest(repoName, - manifestContent.Subject.Digest.String()) - if err != nil { - if errors.Is(err, zerr.ErrManifestNotFound) { - return true, NotationType, signedImageManifestDigest, zerr.ErrOrphanSignature - } - - return false, "", "", err - } - - return true, NotationType, signedImageManifestDigest, nil + if manifestArtifactType == notreg.ArtifactTypeNotation && manifestContent.Subject != nil { + return true, NotationType, manifestContent.Subject.Digest, nil } // check cosign @@ -759,22 +740,6 @@ func CheckIsImageSignature(repoName string, manifestBlob []byte, reference strin signedImageManifestDigest := godigest.NewDigestFromEncoded(godigest.SHA256, signedImageManifestDigestEncoded) - imgStore := storeController.GetImageStore(repoName) - - _, signedImageManifestDigest, _, err := imgStore.GetImageManifest(repoName, - signedImageManifestDigest.String()) - if err != nil { - if errors.Is(err, zerr.ErrManifestNotFound) { - return true, CosignType, signedImageManifestDigest, zerr.ErrOrphanSignature - } - - return false, "", "", err - } - - if signedImageManifestDigest.String() == "" { - return true, CosignType, signedImageManifestDigest, zerr.ErrOrphanSignature - } - return true, CosignType, signedImageManifestDigest, nil } diff --git a/pkg/test/common.go b/pkg/test/common.go index da2b184084..c77b149f98 100644 --- a/pkg/test/common.go +++ b/pkg/test/common.go @@ -646,6 +646,15 @@ func GetRandomImage(reference string) (Image, error) { return Image{}, err } + if reference == "" { + blob, err := json.Marshal(manifest) + if err != nil { + return Image{}, err + } + + reference = godigest.FromBytes(blob).String() + } + return Image{ Manifest: manifest, Layers: layers,