diff --git a/pkg/extensions/scrub/scrub_test.go b/pkg/extensions/scrub/scrub_test.go index 2939722fe..a12b9ae9e 100644 --- a/pkg/extensions/scrub/scrub_test.go +++ b/pkg/extensions/scrub/scrub_test.go @@ -111,9 +111,9 @@ func TestScrubExtension(t *testing.T) { err = WriteImageToFileSystem(image, repoName, "0.0.1", srcStorageCtlr) So(err, ShouldBeNil) - manifestDigest := image.ManifestDescriptor.Digest + layerDigest := image.Manifest.Layers[0].Digest - err = os.Remove(path.Join(dir, repoName, "blobs/sha256", manifestDigest.Encoded())) + err = os.Remove(path.Join(dir, repoName, "blobs/sha256", layerDigest.Encoded())) if err != nil { panic(err) } @@ -240,9 +240,9 @@ func TestRunScrubRepo(t *testing.T) { err = WriteImageToFileSystem(image, repoName, "0.0.1", srcStorageCtlr) So(err, ShouldBeNil) - manifestDigest := image.ManifestDescriptor.Digest + layerDigest := image.Manifest.Layers[0].Digest - err = os.Remove(path.Join(dir, repoName, "blobs/sha256", manifestDigest.Encoded())) + err = os.Remove(path.Join(dir, repoName, "blobs/sha256", layerDigest.Encoded())) if err != nil { panic(err) } diff --git a/pkg/storage/scrub.go b/pkg/storage/scrub.go index 1b9d1b761..a7a8b95ea 100644 --- a/pkg/storage/scrub.go +++ b/pkg/storage/scrub.go @@ -3,6 +3,7 @@ package storage import ( "context" "encoding/json" + "errors" "fmt" "io" "strings" @@ -12,7 +13,7 @@ import ( godigest "github.com/opencontainers/go-digest" ispec "github.com/opencontainers/image-spec/specs-go/v1" - "zotregistry.io/zot/errors" + zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/common" storageTypes "zotregistry.io/zot/pkg/storage/types" ) @@ -85,33 +86,20 @@ func CheckImageStoreBlobsIntegrity(ctx context.Context, imgStore storageTypes.Im return results, nil } +// CheckRepo is the main entry point for the scrub task +// We aim for eventual consistency (locks, etc) since this task contends with data path. func CheckRepo(ctx context.Context, imageName string, imgStore storageTypes.ImageStore) ([]ScrubImageResult, error) { results := []ScrubImageResult{} - var lockLatency time.Time - - imgStore.RLock(&lockLatency) - defer imgStore.RUnlock(&lockLatency) - - // check image structure / layout - ok, err := imgStore.ValidateRepo(imageName) - if err != nil { - return results, err - } - - if !ok { - return results, errors.ErrRepoBadLayout - } - - // check "index.json" content - indexContent, err := imgStore.GetIndexContent(imageName) + // getIndex holds the lock + indexContent, err := getIndex(imageName, imgStore) if err != nil { return results, err } var index ispec.Index if err := json.Unmarshal(indexContent, &index); err != nil { - return results, errors.ErrRepoNotFound + return results, zerr.ErrRepoNotFound } scrubbedManifests := make(map[godigest.Digest]ScrubImageResult) @@ -122,46 +110,107 @@ func CheckRepo(ctx context.Context, imageName string, imgStore storageTypes.Imag } tag := manifest.Annotations[ispec.AnnotationRefName] - scrubManifest(ctx, manifest, imgStore, imageName, tag, scrubbedManifests) - results = append(results, scrubbedManifests[manifest.Digest]) + + // checkImage holds the lock + layers, err := checkImage(manifest, imgStore, imageName, tag, scrubbedManifests) + if err == nil && len(layers) > 0 { + // CheckLayers doesn't use locks + imgRes := CheckLayers(imageName, tag, layers, imgStore) + scrubbedManifests[manifest.Digest] = imgRes + } + + // ignore the manifest if it isn't found + if !errors.Is(err, zerr.ErrManifestNotFound) { + results = append(results, scrubbedManifests[manifest.Digest]) + } } return results, nil } -func scrubManifest( - ctx context.Context, manifest ispec.Descriptor, imgStore storageTypes.ImageStore, imageName, tag string, +func checkImage( + manifest ispec.Descriptor, imgStore storageTypes.ImageStore, imageName, tag string, scrubbedManifests map[godigest.Digest]ScrubImageResult, -) { +) ([]ispec.Descriptor, error) { + var lockLatency time.Time + + imgStore.RLock(&lockLatency) + defer imgStore.RUnlock(&lockLatency) + + manifestContent, err := imgStore.GetBlobContent(imageName, manifest.Digest) + if err != nil { + // ignore if the manifest is not found(probably it was deleted after we got the list of manifests) + return []ispec.Descriptor{}, zerr.ErrManifestNotFound + } + + return scrubManifest(manifest, imgStore, imageName, tag, manifestContent, scrubbedManifests) +} + +func getIndex(imageName string, imgStore storageTypes.ImageStore) ([]byte, error) { + var lockLatency time.Time + + imgStore.RLock(&lockLatency) + defer imgStore.RUnlock(&lockLatency) + + // check image structure / layout + ok, err := imgStore.ValidateRepo(imageName) + if err != nil { + return []byte{}, err + } + + if !ok { + return []byte{}, zerr.ErrRepoBadLayout + } + + // check "index.json" content + indexContent, err := imgStore.GetIndexContent(imageName) + if err != nil { + return []byte{}, err + } + + return indexContent, nil +} + +func scrubManifest( + manifest ispec.Descriptor, imgStore storageTypes.ImageStore, imageName, tag string, + manifestContent []byte, scrubbedManifests map[godigest.Digest]ScrubImageResult, +) ([]ispec.Descriptor, error) { + layers := []ispec.Descriptor{} + res, ok := scrubbedManifests[manifest.Digest] if ok { scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, res.Status, res.AffectedBlob, res.Error) - return + return layers, nil } switch manifest.MediaType { case ispec.MediaTypeImageIndex: - buf, err := imgStore.GetBlobContent(imageName, manifest.Digest) - if err != nil { - imgRes := getResult(imageName, tag, manifest.Digest, errors.ErrBadBlobDigest) - scrubbedManifests[manifest.Digest] = imgRes - - return - } - var idx ispec.Index - if err := json.Unmarshal(buf, &idx); err != nil { - imgRes := getResult(imageName, tag, manifest.Digest, errors.ErrBadBlobDigest) + if err := json.Unmarshal(manifestContent, &idx); err != nil { + imgRes := getResult(imageName, tag, manifest.Digest, zerr.ErrBadBlobDigest) scrubbedManifests[manifest.Digest] = imgRes - return + return layers, err } // check all manifests for _, man := range idx.Manifests { - scrubManifest(ctx, man, imgStore, imageName, tag, scrubbedManifests) + buf, err := imgStore.GetBlobContent(imageName, man.Digest) + if err != nil { + imgRes := getResult(imageName, tag, man.Digest, zerr.ErrBadBlobDigest) + scrubbedManifests[man.Digest] = imgRes + scrubbedManifests[manifest.Digest] = imgRes + + return layers, err + } + + layersToScrub, err := scrubManifest(man, imgStore, imageName, tag, buf, scrubbedManifests) + + if err == nil { + layers = append(layers, layersToScrub...) + } // if the manifest is affected then this index is also affected if scrubbedManifests[man.Digest].Error != "" { @@ -170,115 +219,116 @@ func scrubManifest( scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, mRes.Status, mRes.AffectedBlob, mRes.Error) - return + return layers, err } } - // at this point, before starting to check de subject we can consider the index is ok + // at this point, before starting to check the subject we can consider the index is ok scrubbedManifests[manifest.Digest] = getResult(imageName, tag, "", nil) // check subject if exists if idx.Subject != nil { - scrubManifest(ctx, *idx.Subject, imgStore, imageName, tag, scrubbedManifests) + buf, err := imgStore.GetBlobContent(imageName, idx.Subject.Digest) + if err != nil { + imgRes := getResult(imageName, tag, idx.Subject.Digest, zerr.ErrBadBlobDigest) + scrubbedManifests[idx.Subject.Digest] = imgRes + scrubbedManifests[manifest.Digest] = imgRes + + return layers, err + } + + layersToScrub, err := scrubManifest(*idx.Subject, imgStore, imageName, tag, buf, scrubbedManifests) + + if err == nil { + layers = append(layers, layersToScrub...) + } subjectRes := scrubbedManifests[idx.Subject.Digest] scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, subjectRes.Status, subjectRes.AffectedBlob, subjectRes.Error) + + return layers, err } + + return layers, nil case ispec.MediaTypeImageManifest: - imgRes := CheckIntegrity(ctx, imageName, tag, manifest, imgStore) - scrubbedManifests[manifest.Digest] = imgRes + affectedBlob, man, err := CheckManifestAndConfig(imageName, manifest, manifestContent, imgStore) + if err == nil { + layers = append(layers, man.Layers...) + } + + scrubbedManifests[manifest.Digest] = getResult(imageName, tag, affectedBlob, err) // if integrity ok then check subject if exists - if imgRes.Error == "" { - manifestContent, _ := imgStore.GetBlobContent(imageName, manifest.Digest) + if err == nil && man.Subject != nil { + buf, err := imgStore.GetBlobContent(imageName, man.Subject.Digest) + if err != nil { + imgRes := getResult(imageName, tag, man.Subject.Digest, zerr.ErrBadBlobDigest) + scrubbedManifests[man.Subject.Digest] = imgRes + scrubbedManifests[manifest.Digest] = imgRes + + return layers, err + } - var man ispec.Manifest + layersToScrub, err := scrubManifest(*man.Subject, imgStore, imageName, tag, buf, scrubbedManifests) - _ = json.Unmarshal(manifestContent, &man) + if err == nil { + layers = append(layers, layersToScrub...) + } - if man.Subject != nil { - scrubManifest(ctx, *man.Subject, imgStore, imageName, tag, scrubbedManifests) + subjectRes := scrubbedManifests[man.Subject.Digest] - subjectRes := scrubbedManifests[man.Subject.Digest] + scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, subjectRes.Status, + subjectRes.AffectedBlob, subjectRes.Error) - scrubbedManifests[manifest.Digest] = newScrubImageResult(imageName, tag, subjectRes.Status, - subjectRes.AffectedBlob, subjectRes.Error) - } + return layers, err } + + return layers, err default: - scrubbedManifests[manifest.Digest] = getResult(imageName, tag, manifest.Digest, errors.ErrBadManifest) - } -} + scrubbedManifests[manifest.Digest] = getResult(imageName, tag, manifest.Digest, zerr.ErrBadManifest) -func CheckIntegrity( - ctx context.Context, imageName, tagName string, manifest ispec.Descriptor, imgStore storageTypes.ImageStore, -) ScrubImageResult { - // check manifest and config - if affectedBlob, err := CheckManifestAndConfig(imageName, manifest, imgStore); err != nil { - return getResult(imageName, tagName, affectedBlob, err) + return layers, zerr.ErrBadManifest } - - // check layers - return CheckLayers(ctx, imageName, tagName, manifest, imgStore) } func CheckManifestAndConfig( - imageName string, manifestDesc ispec.Descriptor, imgStore storageTypes.ImageStore, -) (godigest.Digest, error) { + imageName string, manifestDesc ispec.Descriptor, manifestContent []byte, imgStore storageTypes.ImageStore, +) (godigest.Digest, ispec.Manifest, error) { // Q oras artifacts? if manifestDesc.MediaType != ispec.MediaTypeImageManifest { - return manifestDesc.Digest, errors.ErrBadManifest - } - - manifestContent, err := imgStore.GetBlobContent(imageName, manifestDesc.Digest) - if err != nil { - return manifestDesc.Digest, err + return manifestDesc.Digest, ispec.Manifest{}, zerr.ErrBadManifest } var manifest ispec.Manifest - err = json.Unmarshal(manifestContent, &manifest) + err := json.Unmarshal(manifestContent, &manifest) if err != nil { - return manifestDesc.Digest, errors.ErrBadManifest + return manifestDesc.Digest, ispec.Manifest{}, zerr.ErrBadManifest } configContent, err := imgStore.GetBlobContent(imageName, manifest.Config.Digest) if err != nil { - return manifest.Config.Digest, err + return manifest.Config.Digest, ispec.Manifest{}, err } var config ispec.Image err = json.Unmarshal(configContent, &config) if err != nil { - return manifest.Config.Digest, errors.ErrBadConfig + return manifest.Config.Digest, ispec.Manifest{}, zerr.ErrBadConfig } - return "", nil + return "", manifest, nil } func CheckLayers( - ctx context.Context, imageName, tagName string, manifest ispec.Descriptor, imgStore storageTypes.ImageStore, + imageName, tagName string, layers []ispec.Descriptor, imgStore storageTypes.ImageStore, ) ScrubImageResult { imageRes := ScrubImageResult{} - buf, err := imgStore.GetBlobContent(imageName, manifest.Digest) - if err != nil { - imageRes = getResult(imageName, tagName, manifest.Digest, err) - - return imageRes - } - - var man ispec.Manifest - if err := json.Unmarshal(buf, &man); err != nil { - imageRes = getResult(imageName, tagName, manifest.Digest, errors.ErrBadManifest) - - return imageRes - } - - for _, layer := range man.Layers { + for _, layer := range layers { if err := imgStore.VerifyBlobDigestValue(imageName, layer.Digest); err != nil { imageRes = getResult(imageName, tagName, layer.Digest, err) diff --git a/pkg/storage/scrub_test.go b/pkg/storage/scrub_test.go index 7f96eed79..03c87d2bf 100644 --- a/pkg/storage/scrub_test.go +++ b/pkg/storage/scrub_test.go @@ -108,6 +108,20 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper actual := strings.TrimSpace(str) So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") So(actual, ShouldContainSubstring, "test 1.0 ok") + + err = WriteMultiArchImageToFileSystem(CreateMultiarchWith().RandomImages(0).Build(), repoName, "2.0", storeCtlr) + So(err, ShouldBeNil) + + buff = bytes.NewBufferString("") + + res, err = storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + str = space.ReplaceAllString(buff.String(), " ") + actual = strings.TrimSpace(str) + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 1.0 ok") + So(actual, ShouldContainSubstring, "test 2.0 ok") }) Convey("Blobs integrity with context done", func() { @@ -153,18 +167,12 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper str := space.ReplaceAllString(buff.String(), " ") actual := strings.TrimSpace(str) So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") - // verify error message - So(actual, ShouldContainSubstring, fmt.Sprintf("test 1.0 affected %s blob not found", manifestDig)) + So(actual, ShouldNotContainSubstring, "affected") index, err := common.GetIndex(imgStore, repoName, log) So(err, ShouldBeNil) So(len(index.Manifests), ShouldEqual, 1) - manifestDescriptor := index.Manifests[0] - - imageRes := storage.CheckLayers(context.Background(), repoName, tag, manifestDescriptor, imgStore) - So(imageRes.Status, ShouldEqual, "affected") - So(imageRes.Error, ShouldEqual, "blob not found") _, err = driver.WriteFile(manifestFile, []byte("invalid content")) So(err, ShouldBeNil) @@ -185,11 +193,10 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(err, ShouldBeNil) So(len(index.Manifests), ShouldEqual, 1) - manifestDescriptor = index.Manifests[0] + manifestDescriptor := index.Manifests[0] - imageRes = storage.CheckLayers(context.Background(), repoName, tag, manifestDescriptor, imgStore) - So(imageRes.Status, ShouldEqual, "affected") - So(imageRes.Error, ShouldEqual, "invalid manifest content") + _, _, err = storage.CheckManifestAndConfig(repoName, manifestDescriptor, []byte("invalid content"), imgStore) + So(err, ShouldNotBeNil) }) Convey("Config integrity affected", func() { @@ -268,7 +275,8 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper Convey("Layer not found", func() { // get content of layer - content, err := imgStore.GetBlobContent(repoName, image.Manifest.Layers[0].Digest) + digest := image.Manifest.Layers[0].Digest + content, err := imgStore.GetBlobContent(repoName, digest) So(err, ShouldBeNil) // change layer file permissions @@ -287,9 +295,9 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(err, ShouldBeNil) So(len(index.Manifests), ShouldEqual, 1) - manifestDescriptor := index.Manifests[0] - imageRes := storage.CheckLayers(context.Background(), repoName, tag, manifestDescriptor, imgStore) + // get content of layer + imageRes := storage.CheckLayers(repoName, tag, []ispec.Descriptor{{Digest: digest}}, imgStore) So(imageRes.Status, ShouldEqual, "affected") So(imageRes.Error, ShouldEqual, "blob not found") @@ -365,8 +373,22 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(actual, ShouldNotContainSubstring, "test ok") // test scrub index - errors - // delete content of manifest file manifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", newManifestDigest.Encoded()) + _, err = driver.WriteFile(manifestFile, []byte("invalid content")) + So(err, ShouldBeNil) + + buff = bytes.NewBufferString("") + + res, err = storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + str = space.ReplaceAllString(buff.String(), " ") + actual = strings.TrimSpace(str) + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test affected") + + // delete content of manifest file err = driver.Delete(manifestFile) So(err, ShouldBeNil) @@ -394,7 +416,8 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper str = space.ReplaceAllString(buff.String(), " ") actual = strings.TrimSpace(str) So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") - So(actual, ShouldContainSubstring, "test affected") + So(actual, ShouldContainSubstring, "test 1.0 ok") + So(actual, ShouldNotContainSubstring, "test affected") index.Manifests[0].MediaType = "invalid" indexBlob, err = json.Marshal(index) @@ -409,7 +432,7 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper res.PrintScrubResults(buff) So(err, ShouldBeNil) - _, err = storage.CheckManifestAndConfig(repoName, index.Manifests[0], imgStore) + _, _, err = storage.CheckManifestAndConfig(repoName, index.Manifests[0], []byte{}, imgStore) So(err, ShouldNotBeNil) So(err, ShouldEqual, zerr.ErrBadManifest) @@ -455,17 +478,12 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper str := space.ReplaceAllString(buff.String(), " ") actual := strings.TrimSpace(str) So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") - So(actual, ShouldContainSubstring, fmt.Sprintf("test 1.0 affected %s blob not found", manifestDig)) + So(actual, ShouldNotContainSubstring, fmt.Sprintf("test 1.0 affected %s blob not found", manifestDig)) index, err := common.GetIndex(imgStore, repoName, log) So(err, ShouldBeNil) So(len(index.Manifests), ShouldEqual, 1) - manifestDescriptor := index.Manifests[0] - - imageRes := storage.CheckLayers(context.Background(), repoName, tag, manifestDescriptor, imgStore) - So(imageRes.Status, ShouldEqual, "affected") - So(imageRes.Error, ShouldContainSubstring, "blob not found") }) Convey("use the result of an already scrubed manifest which is the subject of the current manifest", func() { @@ -492,6 +510,86 @@ func RunCheckAllBlobsIntegrityTests( //nolint: thelper So(actual, ShouldContainSubstring, "test 1.0 ok") So(actual, ShouldContainSubstring, "test 0.0.1 ok") }) + + Convey("the subject of the current manifest doesn't exist", func() { + index, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + manifestDescriptor, ok := common.GetManifestDescByReference(index, image.ManifestDescriptor.Digest.String()) + So(ok, ShouldBeTrue) + + err = WriteImageToFileSystem(CreateDefaultImageWith().Subject(&manifestDescriptor).Build(), + repoName, "0.0.2", storeCtlr) + So(err, ShouldBeNil) + + // get content of manifest file + content, _, _, err := imgStore.GetImageManifest(repoName, manifestDescriptor.Digest.String()) + So(err, ShouldBeNil) + + // delete content of manifest file + manifestDig := image.ManifestDescriptor.Digest.Encoded() + manifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", manifestDig) + err = driver.Delete(manifestFile) + So(err, ShouldBeNil) + + defer func() { + // put manifest content back to file + _, err = driver.WriteFile(manifestFile, content) + So(err, ShouldBeNil) + }() + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 0.0.2 affected") + }) + + Convey("the subject of the current index doesn't exist", func() { + index, err := common.GetIndex(imgStore, repoName, log) + So(err, ShouldBeNil) + + manifestDescriptor, ok := common.GetManifestDescByReference(index, image.ManifestDescriptor.Digest.String()) + So(ok, ShouldBeTrue) + + err = WriteMultiArchImageToFileSystem(CreateMultiarchWith().RandomImages(1).Subject(&manifestDescriptor).Build(), + repoName, "0.0.2", storeCtlr) + So(err, ShouldBeNil) + + // get content of manifest file + content, _, _, err := imgStore.GetImageManifest(repoName, manifestDescriptor.Digest.String()) + So(err, ShouldBeNil) + + // delete content of manifest file + manifestDig := image.ManifestDescriptor.Digest.Encoded() + manifestFile := path.Join(imgStore.RootDir(), repoName, "/blobs/sha256", manifestDig) + err = driver.Delete(manifestFile) + So(err, ShouldBeNil) + + defer func() { + // put manifest content back to file + _, err = driver.WriteFile(manifestFile, content) + So(err, ShouldBeNil) + }() + + buff := bytes.NewBufferString("") + + res, err := storeCtlr.CheckAllBlobsIntegrity(context.Background()) + res.PrintScrubResults(buff) + So(err, ShouldBeNil) + + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + actual := strings.TrimSpace(str) + So(actual, ShouldContainSubstring, "REPOSITORY TAG STATUS AFFECTED BLOB ERROR") + So(actual, ShouldContainSubstring, "test 0.0.2 affected") + }) }) Convey("test errors", func() {