diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 397020b5..ea70a40e 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -39,6 +39,7 @@ import ( "github.com/sigstore/cosign/cmd/cosign/cli/options" "github.com/sigstore/cosign/cmd/cosign/cli/sign" "github.com/sigstore/cosign/cmd/cosign/cli/verify" + "github.com/sigstore/cosign/pkg/oci/remote" . "github.com/smartystreets/goconvey/convey" "github.com/stretchr/testify/assert" "golang.org/x/crypto/bcrypt" @@ -2251,8 +2252,8 @@ func TestAuthorizationWithBasicAuth(t *testing.T) { So(err, ShouldBeNil) So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusOK) - var e api.Error - err = json.Unmarshal(resp.Body(), &e) + var apiErr api.Error + err = json.Unmarshal(resp.Body(), &apiErr) So(err, ShouldBeNil) // should get 403 without create @@ -2457,6 +2458,27 @@ func TestAuthorizationWithBasicAuth(t *testing.T) { }, }, DefaultPolicy: []string{}} + /* we have 4 images(authz/image, golang, zot-test, zot-cve-test) in storage, + but because at this point we only have read access + in authz/image and zot-test, we should get only that when listing repositories*/ + resp, err = resty.R().SetBasicAuth(username, passphrase). + Get(baseURL + constants.RoutePrefix + constants.ExtCatalogPrefix) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + err = json.Unmarshal(resp.Body(), &apiErr) + So(err, ShouldBeNil) + + catalog := struct { + Repositories []string `json:"repositories"` + }{} + + err = json.Unmarshal(resp.Body(), &catalog) + So(err, ShouldBeNil) + So(len(catalog.Repositories), ShouldEqual, 2) + So(catalog.Repositories, ShouldContain, "zot-test") + So(catalog.Repositories, ShouldContain, AuthorizationNamespace) + // get manifest should get 200 now resp, err = resty.R().SetBasicAuth(username, passphrase). Get(baseURL + "/v2/zot-test/manifests/0.0.1") @@ -6490,6 +6512,257 @@ func TestInjectTooManyOpenFiles(t *testing.T) { }) } +func TestGCSignaturesAndUntaggedManifests(t *testing.T) { + Convey("Make controller", t, func() { + repoName := "testrepo" + tag := "0.0.1" + + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + + ctlr := api.NewController(conf) + + Convey("Garbage collect signatures without subject and manifests without tags", func(c C) { + dir := t.TempDir() + ctlr.Config.Storage.RootDirectory = dir + ctlr.Config.Storage.GC = true + ctlr.Config.Storage.GCDelay = 1 * time.Millisecond + + err := test.CopyFiles("../../test/data/zot-test", path.Join(dir, repoName)) + if err != nil { + panic(err) + } + + go startServer(ctlr) + defer stopServer(ctlr) + test.WaitTillServerReady(baseURL) + + resp, err := resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, tag)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + digest := godigest.FromBytes(resp.Body()) + So(digest, ShouldNotBeEmpty) + + cwd, err := os.Getwd() + So(err, ShouldBeNil) + defer func() { _ = os.Chdir(cwd) }() + tdir := t.TempDir() + _ = os.Chdir(tdir) + + // generate a keypair + os.Setenv("COSIGN_PASSWORD", "") + err = generate.GenerateKeyPairCmd(context.TODO(), "", nil) + So(err, ShouldBeNil) + + image := fmt.Sprintf("localhost:%s/%s@%s", port, repoName, digest.String()) + + // sign the image + err = sign.SignCmd(&options.RootOptions{Verbose: true, Timeout: 1 * time.Minute}, + options.KeyOpts{KeyRef: path.Join(tdir, "cosign.key"), PassFunc: generate.GetPass}, + options.RegistryOptions{AllowInsecure: true}, + map[string]interface{}{"tag": tag}, + []string{image}, + "", "", true, "", "", "", false, false, "", true) + + So(err, ShouldBeNil) + + // "notation" (notaryv2) doesn't yet support exported apis, so use the binary instead + notPath, err := exec.LookPath("notation") + So(notPath, ShouldNotBeNil) + So(err, ShouldBeNil) + + os.Setenv("XDG_CONFIG_HOME", tdir) + + // generate a keypair + cmd := exec.Command("notation", "cert", "generate-test", "--trust", "good") + output, err := cmd.CombinedOutput() + t.Log(string(output)) + So(err, ShouldBeNil) + + // sign the image + cmd = exec.Command("notation", "sign", "--key", "good", "--plain-http", image) + output, err = cmd.CombinedOutput() + t.Log(string(output)) + So(err, ShouldBeNil) + + // get cosign signature manifest + cosignTag := strings.Replace(digest.String(), ":", "-", 1) + "." + remote.SignatureTagSuffix + + resp, err = resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, cosignTag)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // get notation signature manifest + resp, err = resty.R().SetQueryParam("artifactType", notreg.ArtifactTypeNotation).Get( + fmt.Sprintf("%s/oras/artifacts/v1/%s/manifests/%s/referrers", baseURL, repoName, digest.String())) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + Convey("Trigger gcNotationSignatures() error", func() { + var refs api.ReferenceList + err = json.Unmarshal(resp.Body(), &refs) + + err := os.Chmod(path.Join(dir, repoName, "blobs", "sha256", refs.References[0].Digest.Encoded()), 0o000) + So(err, ShouldBeNil) + + // trigger gc + cfg, layers, manifest, err := test.GetImageComponents(3) + So(err, ShouldBeNil) + + err = test.UploadImage( + test.Image{ + Config: cfg, + Layers: layers, + Manifest: manifest, + Tag: tag, + }, baseURL, repoName) + So(err, ShouldBeNil) + + err = os.Chmod(path.Join(dir, repoName, "blobs", "sha256", refs.References[0].Digest.Encoded()), 0o755) + So(err, ShouldBeNil) + }) + + // push an image without tag + cfg, layers, manifest, err := test.GetImageComponents(2) + So(err, ShouldBeNil) + + manifestBuf, err := json.Marshal(manifest) + So(err, ShouldBeNil) + untaggedManifestDigest := godigest.FromBytes(manifestBuf) + + err = test.UploadImage( + test.Image{ + Config: cfg, + Layers: layers, + Manifest: manifest, + Tag: untaggedManifestDigest.String(), + }, baseURL, repoName) + So(err, ShouldBeNil) + + // overwrite image so that signatures will get invalidated and gc'ed + cfg, layers, manifest, err = test.GetImageComponents(3) + So(err, ShouldBeNil) + + err = test.UploadImage( + test.Image{ + Config: cfg, + Layers: layers, + Manifest: manifest, + Tag: tag, + }, baseURL, repoName) + So(err, ShouldBeNil) + + manifestBuf, err = json.Marshal(manifest) + So(err, ShouldBeNil) + newManifestDigest := godigest.FromBytes(manifestBuf) + + // both signatures should be gc'ed + resp, err = resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, cosignTag)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + resp, err = resty.R().SetQueryParam("artifactType", notreg.ArtifactTypeNotation).Get( + fmt.Sprintf("%s/oras/artifacts/v1/%s/manifests/%s/referrers", baseURL, repoName, digest.String())) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + resp, err = resty.R().SetQueryParam("artifactType", notreg.ArtifactTypeNotation).Get( + fmt.Sprintf("%s/oras/artifacts/v1/%s/manifests/%s/referrers", baseURL, repoName, newManifestDigest.String())) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + // untagged image should also be gc'ed + resp, err = resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, untaggedManifestDigest)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + }) + + Convey("Do not gc manifests which are part of a multiarch image", func(c C) { + dir := t.TempDir() + ctlr.Config.Storage.RootDirectory = dir + ctlr.Config.Storage.GC = true + ctlr.Config.Storage.GCDelay = 500 * time.Millisecond + + err := test.CopyFiles("../../test/data/zot-test", path.Join(dir, repoName)) + if err != nil { + panic(err) + } + + go startServer(ctlr) + defer stopServer(ctlr) + test.WaitTillServerReady(baseURL) + + resp, err := resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, tag)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + digest := godigest.FromBytes(resp.Body()) + So(digest, ShouldNotBeEmpty) + + // push an image index and make sure manifests contained by it are not gc'ed + // create an image index on upstream + var index ispec.Index + index.SchemaVersion = 2 + index.MediaType = ispec.MediaTypeImageIndex + + // upload multiple manifests + for i := 0; i < 4; i++ { + config, layers, manifest, err := test.GetImageComponents(1000 + i) + So(err, ShouldBeNil) + + manifestContent, err := json.Marshal(manifest) + So(err, ShouldBeNil) + + manifestDigest := godigest.FromBytes(manifestContent) + + err = test.UploadImage( + test.Image{ + Manifest: manifest, + Config: config, + Layers: layers, + Tag: manifestDigest.String(), + }, + baseURL, + repoName) + So(err, ShouldBeNil) + + index.Manifests = append(index.Manifests, ispec.Descriptor{ + Digest: manifestDigest, + MediaType: ispec.MediaTypeImageManifest, + Size: int64(len(manifestContent)), + }) + } + + content, err := json.Marshal(index) + So(err, ShouldBeNil) + indexDigest := godigest.FromBytes(content) + So(indexDigest, ShouldNotBeNil) + + time.Sleep(1 * time.Second) + // upload image index + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + SetBody(content).Put(baseURL + fmt.Sprintf("/v2/%s/manifests/latest", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + Get(baseURL + fmt.Sprintf("/v2/%s/manifests/latest", repoName)) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Body(), ShouldNotBeEmpty) + So(resp.Header().Get("Content-Type"), ShouldNotBeEmpty) + + // make sure manifests which are part of image index are not gc'ed + for _, manifest := range index.Manifests { + resp, err = resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, manifest.Digest.String())) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + } + }) + }) +} + func TestPeriodicGC(t *testing.T) { Convey("Periodic gc enabled for default store", t, func() { repoName := "testRepo" diff --git a/pkg/storage/common.go b/pkg/storage/common.go index 1e8487bb..03aa687d 100644 --- a/pkg/storage/common.go +++ b/pkg/storage/common.go @@ -250,6 +250,7 @@ func CheckIfIndexNeedsUpdate(index *ispec.Index, desc *ispec.Descriptor, return updateIndex, oldDgst, nil } +// GetIndex returns the contents of index.json. func GetIndex(imgStore ImageStore, repo string, log zerolog.Logger) (ispec.Index, error) { var index ispec.Index @@ -267,8 +268,33 @@ func GetIndex(imgStore ImageStore, repo string, log zerolog.Logger) (ispec.Index return index, nil } +// GetImageIndex returns a multiarch type image. +func GetImageIndex(imgStore ImageStore, repo string, digest godigest.Digest, log zerolog.Logger) (ispec.Index, error) { + var imageIndex ispec.Index + + if err := digest.Validate(); err != nil { + return imageIndex, err + } + + buf, err := imgStore.GetBlobContent(repo, digest) + if err != nil { + return imageIndex, err + } + + indexPath := path.Join(imgStore.RootDir(), repo, "blobs", + digest.Algorithm().String(), digest.Encoded()) + + if err := json.Unmarshal(buf, &imageIndex); err != nil { + log.Error().Err(err).Str("path", indexPath).Msg("invalid JSON") + + return imageIndex, err + } + + return imageIndex, nil +} + func RemoveManifestDescByReference(index *ispec.Index, reference string, detectCollisions bool, -) (ispec.Descriptor, bool, error) { +) (ispec.Descriptor, error) { var removedManifest ispec.Descriptor var found bool @@ -297,12 +323,14 @@ func RemoveManifestDescByReference(index *ispec.Index, reference string, detectC } if foundCount > 1 && detectCollisions { - return ispec.Descriptor{}, false, zerr.ErrManifestConflict + return ispec.Descriptor{}, zerr.ErrManifestConflict + } else if !found { + return ispec.Descriptor{}, zerr.ErrManifestNotFound } index.Manifests = outIndex.Manifests - return removedManifest, found, nil + return removedManifest, nil } /* @@ -369,21 +397,11 @@ func PruneImageManifestsFromIndex(imgStore ImageStore, repo string, digest godig } for _, otherIndex := range otherImgIndexes { - buf, err := imgStore.GetBlobContent(repo, otherIndex.Digest) + oindex, err := GetImageIndex(imgStore, repo, otherIndex.Digest, log) if err != nil { return nil, err } - indexPath := path.Join(imgStore.RootDir(), repo, "blobs", - otherIndex.Digest.Algorithm().String(), otherIndex.Digest.Encoded()) - - var oindex ispec.Index - if err := json.Unmarshal(buf, &oindex); err != nil { - log.Error().Err(err).Str("path", indexPath).Msg("invalid JSON") - - return nil, err - } - for _, omanifest := range oindex.Manifests { _, ok := inUse[omanifest.Digest.Encoded()] if ok { @@ -479,21 +497,8 @@ func GetOrasReferrers(imgStore ImageStore, repo string, gdigest godigest.Digest, continue } - buf, err := imgStore.GetBlobContent(repo, manifest.Digest) + artManifest, err := GetOrasManifestByDigest(imgStore, repo, manifest.Digest, log) if err != nil { - log.Error().Err(err).Str("blob", imgStore.BlobPath(repo, manifest.Digest)).Msg("failed to read manifest") - - if os.IsNotExist(err) || errors.Is(err, driver.PathNotFoundError{}) { - return nil, zerr.ErrManifestNotFound - } - - return nil, err - } - - var artManifest oras.Manifest - if err := json.Unmarshal(buf, &artManifest); err != nil { - log.Error().Err(err).Str("dir", dir).Msg("invalid JSON") - return nil, err } @@ -635,6 +640,32 @@ func GetReferrers(imgStore ImageStore, repo string, gdigest godigest.Digest, art return index, nil } +func GetOrasManifestByDigest(imgStore ImageStore, repo string, digest godigest.Digest, log zerolog.Logger, +) (oras.Manifest, error) { + var artManifest oras.Manifest + + blobPath := imgStore.BlobPath(repo, digest) + + buf, err := imgStore.GetBlobContent(repo, digest) + if err != nil { + log.Error().Err(err).Str("blob", blobPath).Msg("failed to read manifest") + + if os.IsNotExist(err) || errors.Is(err, driver.PathNotFoundError{}) { + return artManifest, zerr.ErrManifestNotFound + } + + return artManifest, err + } + + if err := json.Unmarshal(buf, &artManifest); err != nil { + log.Error().Err(err).Str("blob", blobPath).Msg("invalid JSON") + + return artManifest, err + } + + return artManifest, nil +} + func IsSupportedMediaType(mediaType string) bool { return mediaType == ispec.MediaTypeImageIndex || mediaType == ispec.MediaTypeImageManifest || diff --git a/pkg/storage/common_test.go b/pkg/storage/common_test.go index 5f16b99f..c9e3a05f 100644 --- a/pkg/storage/common_test.go +++ b/pkg/storage/common_test.go @@ -303,3 +303,40 @@ func TestGetReferrersErrors(t *testing.T) { }) }) } + +func TestGetImageIndexErrors(t *testing.T) { + log := zerolog.New(os.Stdout) + + Convey("Trigger invalid digest error", t, func(c C) { + imgStore := &mocks.MockedImageStore{} + + _, err := storage.GetImageIndex(imgStore, "zot-test", "invalidDigest", log) + So(err, ShouldNotBeNil) + }) + + Convey("Trigger GetBlobContent error", t, func(c C) { + imgStore := &mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + return []byte{}, errors.ErrBlobNotFound + }, + } + + validDigest := godigest.FromBytes([]byte("blob")) + + _, err := storage.GetImageIndex(imgStore, "zot-test", validDigest, log) + So(err, ShouldNotBeNil) + }) + + Convey("Trigger unmarshal error", t, func(c C) { + imgStore := &mocks.MockedImageStore{ + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + return []byte{}, nil + }, + } + + validDigest := godigest.FromBytes([]byte("blob")) + + _, err := storage.GetImageIndex(imgStore, "zot-test", validDigest, log) + So(err, ShouldNotBeNil) + }) +} diff --git a/pkg/storage/local/local.go b/pkg/storage/local/local.go index 0e04b18e..e498fff6 100644 --- a/pkg/storage/local/local.go +++ b/pkg/storage/local/local.go @@ -10,6 +10,7 @@ import ( "os" "path" "path/filepath" + "strings" "sync" "syscall" "time" @@ -18,6 +19,7 @@ import ( apexlog "github.com/apex/log" guuid "github.com/gofrs/uuid" "github.com/minio/sha256-simd" + notreg "github.com/notaryproject/notation-go/registry" godigest "github.com/opencontainers/go-digest" imeta "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -25,8 +27,10 @@ import ( "github.com/opencontainers/umoci/oci/casext" oras "github.com/oras-project/artifacts-spec/specs-go/v1" "github.com/rs/zerolog" + "github.com/sigstore/cosign/pkg/oci/remote" zerr "zotregistry.io/zot/errors" + "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/extensions/monitoring" zlog "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/scheduler" @@ -565,15 +569,11 @@ func (is *ImageStoreLocal) DeleteImageManifest(repo, reference string, detectCol return err } - manifestDesc, found, err := storage.RemoveManifestDescByReference(&index, reference, detectCollision) + manifestDesc, err := storage.RemoveManifestDescByReference(&index, reference, detectCollision) if err != nil { return err } - if !found { - return zerr.ErrManifestNotFound - } - err = storage.UpdateIndexWithPrunedImageManifests(is, &index, repo, manifestDesc, manifestDesc.Digest, is.log) if err != nil { return err @@ -1267,14 +1267,18 @@ func (is *ImageStoreLocal) GetBlobContent(repo string, digest godigest.Digest) ( blobPath := is.BlobPath(repo, digest) blob, err := os.ReadFile(blobPath) - if os.IsNotExist(err) { - is.log.Error().Err(err).Str("blob", blobPath).Msg("blob doesn't exist") + if err != nil { + if os.IsNotExist(err) { + is.log.Error().Err(err).Str("blob", blobPath).Msg("blob doesn't exist") - return []byte{}, zerr.ErrBlobNotFound + return []byte{}, zerr.ErrBlobNotFound + } + + is.log.Error().Err(err).Str("blob", blobPath).Msg("failed to read blob") + + return []byte{}, err } - is.log.Error().Err(err).Str("blob", blobPath).Msg("failed to read blob") - return blob, nil } @@ -1429,6 +1433,65 @@ func (is *ImageStoreLocal) garbageCollect(dir string, repo string) error { } defer oci.Close() + // gc untagged manifests and signatures + index, err := oci.GetIndex(context.Background()) + if err != nil { + return err + } + + referencedByImageIndex := []string{} + cosignDescriptors := []ispec.Descriptor{} + notationDescriptors := []ispec.Descriptor{} + + /* gather manifests references by multiarch images (to skip gc) + gather cosign and notation signatures descriptors */ + for _, desc := range index.Manifests { + switch desc.MediaType { + case ispec.MediaTypeImageIndex: + indexImage, err := storage.GetImageIndex(is, repo, desc.Digest, is.log) + if err != nil { + is.log.Error().Err(err).Str("repo", repo).Str("digest", desc.Digest.String()). + Msg("gc: failed to read multiarch(index) image") + + return err + } + + for _, indexDesc := range indexImage.Manifests { + referencedByImageIndex = append(referencedByImageIndex, indexDesc.Digest.String()) + } + case ispec.MediaTypeImageManifest: + tag, ok := desc.Annotations[ispec.AnnotationRefName] + if ok { + // gather cosign signatures + if strings.HasPrefix(tag, "sha256-") && strings.HasSuffix(tag, remote.SignatureTagSuffix) { + cosignDescriptors = append(cosignDescriptors, desc) + } + } + case oras.MediaTypeArtifactManifest: + notationDescriptors = append(notationDescriptors, desc) + } + } + + is.log.Info().Msg("gc: untagged manifests") + + if err := gcUntaggedManifests(is, oci, &index, repo, referencedByImageIndex); err != nil { + return err + } + + is.log.Info().Msg("gc: cosign signatures") + + if err := gcCosignSignatures(is, oci, &index, repo, cosignDescriptors); err != nil { + return err + } + + is.log.Info().Msg("gc: notation signatures") + + if err := gcNotationSignatures(is, oci, &index, repo, notationDescriptors); err != nil { + return err + } + + is.log.Info().Msg("gc: blobs") + err = oci.GC(context.Background(), ifOlderThan(is, repo, is.gcDelay)) if err := test.Error(err); err != nil { return err @@ -1437,25 +1500,176 @@ func (is *ImageStoreLocal) garbageCollect(dir string, repo string) error { return nil } +func gcUntaggedManifests(imgStore *ImageStoreLocal, oci casext.Engine, index *ispec.Index, repo string, + referencedByImageIndex []string, +) error { + for _, desc := range index.Manifests { + // skip manifests referenced in image indexex + if common.Contains(referencedByImageIndex, desc.Digest.String()) { + continue + } + + // remove untagged images + if desc.MediaType == ispec.MediaTypeImageManifest { + _, ok := desc.Annotations[ispec.AnnotationRefName] + if !ok { + // check if is indeed an image and not an artifact by checking it's config blob + buf, err := imgStore.GetBlobContent(repo, desc.Digest) + if err != nil { + imgStore.log.Error().Err(err).Str("repo", repo).Str("digest", desc.Digest.String()). + Msg("gc: failed to read image manifest") + + return err + } + + manifest := ispec.Manifest{} + + err = json.Unmarshal(buf, &manifest) + if err != nil { + return err + } + + // skip manifests which are not of type image + if manifest.Config.MediaType != ispec.MediaTypeImageConfig { + imgStore.log.Info().Str("config mediaType", manifest.Config.MediaType). + Msg("skipping gc untagged manifest, because config blob is not application/vnd.oci.image.config.v1+json") + + continue + } + + // remove manifest if it's older than gc.delay + canGC, err := isBlobOlderThan(imgStore, repo, desc.Digest, imgStore.gcDelay) + if err != nil { + imgStore.log.Error().Err(err).Str("repo", repo).Str("digest", desc.Digest.String()). + Msgf("gc: failed to check if blob is older than %s", imgStore.gcDelay.String()) + + return err + } + + if canGC { + imgStore.log.Info().Str("repo", repo).Str("digest", desc.Digest.String()).Msg("gc: removing manifest without tag") + + _, err = storage.RemoveManifestDescByReference(index, desc.Digest.String(), true) + if errors.Is(err, zerr.ErrManifestConflict) { + imgStore.log.Info().Str("repo", repo).Str("digest", desc.Digest.String()). + Msg("gc: skipping removing manifest due to conflict") + + continue + } + + err := oci.PutIndex(context.Background(), *index) + if err != nil { + return err + } + } + } + } + } + + return nil +} + +func gcCosignSignatures(imgStore *ImageStoreLocal, oci casext.Engine, index *ispec.Index, repo string, + cosignDescriptors []ispec.Descriptor, +) error { + for _, cosignDesc := range cosignDescriptors { + foundSubject := false + // check if we can find the manifest which the signature points to + for _, desc := range index.Manifests { + subject := fmt.Sprintf("sha256-%s.%s", desc.Digest.Encoded(), remote.SignatureTagSuffix) + if subject == cosignDesc.Annotations[ispec.AnnotationRefName] { + foundSubject = true + } + } + + if !foundSubject { + // remove manifest + imgStore.log.Info().Str("repo", repo).Str("digest", cosignDesc.Digest.String()). + Msg("gc: removing cosign signature without subject") + + // no need to check for manifest conflict, if one doesn't have a subject, then none with same digest will have + _, _ = storage.RemoveManifestDescByReference(index, cosignDesc.Digest.String(), false) + + err := oci.PutIndex(context.Background(), *index) + if err != nil { + return err + } + } + } + + return nil +} + +func gcNotationSignatures(imgStore *ImageStoreLocal, oci casext.Engine, index *ispec.Index, repo string, + notationDescriptors []ispec.Descriptor, +) error { + for _, notationDesc := range notationDescriptors { + foundSubject := false + // check if we can find the manifest which the signature points to + artManifest, err := storage.GetOrasManifestByDigest(imgStore, repo, notationDesc.Digest, imgStore.log) + if err != nil { + imgStore.log.Error().Err(err).Str("repo", repo).Str("digest", notationDesc.Digest.String()). + Msg("gc: failed to get oras artifact manifest") + + return err + } + + // skip oras artifacts which are not signatures + if artManifest.ArtifactType != notreg.ArtifactTypeNotation { + continue + } + + for _, desc := range index.Manifests { + if desc.Digest == artManifest.Subject.Digest { + foundSubject = true + } + } + + if !foundSubject { + // remove manifest + imgStore.log.Info().Str("repo", repo).Str("digest", notationDesc.Digest.String()). + Msg("gc: removing notation signature without subject") + + // no need to check for manifest conflict, if one doesn't have a subject, then none with same digest will have + _, _ = storage.RemoveManifestDescByReference(index, notationDesc.Digest.String(), false) + + err := oci.PutIndex(context.Background(), *index) + if err != nil { + return err + } + } + } + + return nil +} + func ifOlderThan(imgStore *ImageStoreLocal, repo string, delay time.Duration) casext.GCPolicy { return func(ctx context.Context, digest godigest.Digest) (bool, error) { - blobPath := imgStore.BlobPath(repo, digest) - - fi, err := os.Stat(blobPath) - if err != nil { - return false, err - } - - if fi.ModTime().Add(delay).After(time.Now()) { - return false, nil - } - - imgStore.log.Info().Str("digest", digest.String()).Str("blobPath", blobPath).Msg("perform GC on blob") - - return true, nil + return isBlobOlderThan(imgStore, repo, digest, delay) } } +func isBlobOlderThan(imgStore *ImageStoreLocal, repo string, digest godigest.Digest, delay time.Duration, +) (bool, error) { + blobPath := imgStore.BlobPath(repo, digest) + + fileInfo, err := os.Stat(blobPath) + if err != nil { + imgStore.log.Error().Err(err).Str("digest", digest.String()).Str("blobPath", blobPath). + Msg("gc: failed to stat blob") + + return false, err + } + + if fileInfo.ModTime().Add(delay).After(time.Now()) { + return false, nil + } + + imgStore.log.Info().Str("digest", digest.String()).Str("blobPath", blobPath).Msg("perform GC on blob") + + return true, nil +} + func DirExists(d string) bool { if !utf8.ValidString(d) { return false diff --git a/pkg/storage/local/local_test.go b/pkg/storage/local/local_test.go index c47b9aea..f070271d 100644 --- a/pkg/storage/local/local_test.go +++ b/pkg/storage/local/local_test.go @@ -1073,7 +1073,7 @@ func TestDedupeLinks(t *testing.T) { Name: "cache", UseRelPaths: true, }, log) - imgStore := local.NewImageStore(dir, true, storage.DefaultGCDelay, + imgStore := local.NewImageStore(dir, false, storage.DefaultGCDelay, true, true, log, metrics, nil, cacheDriver) Convey("Dedupe", t, func(c C) { @@ -2205,6 +2205,225 @@ func TestGarbageCollectForImageStore(t *testing.T) { }) } +func TestGarbageCollectErrors(t *testing.T) { + Convey("Make image store", t, func(c C) { + dir := t.TempDir() + + log := log.NewLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, log) + cacheDriver, _ := storage.Create("boltdb", cache.BoltDBDriverParameters{ + RootDir: dir, + Name: "cache", + UseRelPaths: true, + }, log) + imgStore := local.NewImageStore(dir, true, 500*time.Millisecond, true, true, log, metrics, nil, cacheDriver) + repoName := "gc-index" + + // create a blob/layer + upload, err := imgStore.NewBlobUpload(repoName) + So(err, ShouldBeNil) + So(upload, ShouldNotBeEmpty) + + content := []byte("this is a blob1") + buf := bytes.NewBuffer(content) + buflen := buf.Len() + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + blob, err := imgStore.PutBlobChunkStreamed(repoName, upload, buf) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + bdgst1 := digest + bsize1 := len(content) + + err = imgStore.FinishBlobUpload(repoName, upload, buf, digest) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + Convey("Trigger error on GetImageIndex", func() { + var index ispec.Index + index.SchemaVersion = 2 + index.MediaType = ispec.MediaTypeImageIndex + + for i := 0; i < 4; i++ { + // upload image config blob + upload, err = imgStore.NewBlobUpload(repoName) + So(err, ShouldBeNil) + So(upload, ShouldNotBeEmpty) + + cblob, cdigest := test.GetRandomImageConfig() + buf = bytes.NewBuffer(cblob) + buflen = buf.Len() + blob, err = imgStore.PutBlobChunkStreamed(repoName, upload, buf) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + err = imgStore.FinishBlobUpload(repoName, upload, buf, cdigest) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Digest: cdigest, + Size: int64(len(cblob)), + }, + Layers: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageLayer, + Digest: bdgst1, + Size: int64(bsize1), + }, + }, + } + manifest.SchemaVersion = 2 + content, err = json.Marshal(manifest) + So(err, ShouldBeNil) + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + _, err = imgStore.PutImageManifest(repoName, digest.String(), ispec.MediaTypeImageManifest, content) + So(err, ShouldBeNil) + + index.Manifests = append(index.Manifests, ispec.Descriptor{ + Digest: digest, + MediaType: ispec.MediaTypeImageManifest, + Size: int64(len(content)), + }) + } + + // upload index image + indexContent, err := json.Marshal(index) + So(err, ShouldBeNil) + indexDigest := godigest.FromBytes(indexContent) + So(indexDigest, ShouldNotBeNil) + + _, err = imgStore.PutImageManifest(repoName, "1.0", ispec.MediaTypeImageIndex, indexContent) + So(err, ShouldBeNil) + + err = os.Chmod(imgStore.BlobPath(repoName, indexDigest), 0o000) + So(err, ShouldBeNil) + + time.Sleep(500 * time.Millisecond) + + err = imgStore.RunGCRepo(repoName) + So(err, ShouldNotBeNil) + }) + + Convey("Trigger error on GetBlobContent and Unmarshal for untagged manifest", func() { + // upload image config blob + upload, err = imgStore.NewBlobUpload(repoName) + So(err, ShouldBeNil) + So(upload, ShouldNotBeEmpty) + + cblob, cdigest := test.GetRandomImageConfig() + buf = bytes.NewBuffer(cblob) + buflen = buf.Len() + blob, err = imgStore.PutBlobChunkStreamed(repoName, upload, buf) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + err = imgStore.FinishBlobUpload(repoName, upload, buf, cdigest) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Digest: cdigest, + Size: int64(len(cblob)), + }, + Layers: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageLayer, + Digest: bdgst1, + Size: int64(bsize1), + }, + }, + } + manifest.SchemaVersion = 2 + content, err = json.Marshal(manifest) + So(err, ShouldBeNil) + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + + _, err = imgStore.PutImageManifest(repoName, digest.String(), ispec.MediaTypeImageManifest, content) + So(err, ShouldBeNil) + + // trigger GetBlobContent error + err = os.Remove(imgStore.BlobPath(repoName, digest)) + So(err, ShouldBeNil) + + time.Sleep(500 * time.Millisecond) + + err = imgStore.RunGCRepo(repoName) + So(err, ShouldNotBeNil) + + // trigger Unmarshal error + _, err = os.Create(imgStore.BlobPath(repoName, digest)) + So(err, ShouldBeNil) + + err = imgStore.RunGCRepo(repoName) + So(err, ShouldNotBeNil) + }) + + Convey("Trigger manifest conflict error", func() { + // upload image config blob + upload, err = imgStore.NewBlobUpload(repoName) + So(err, ShouldBeNil) + So(upload, ShouldNotBeEmpty) + + cblob, cdigest := test.GetRandomImageConfig() + buf = bytes.NewBuffer(cblob) + buflen = buf.Len() + blob, err = imgStore.PutBlobChunkStreamed(repoName, upload, buf) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + err = imgStore.FinishBlobUpload(repoName, upload, buf, cdigest) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: ispec.MediaTypeImageConfig, + Digest: cdigest, + Size: int64(len(cblob)), + }, + Layers: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageLayer, + Digest: bdgst1, + Size: int64(bsize1), + }, + }, + } + manifest.SchemaVersion = 2 + content, err = json.Marshal(manifest) + So(err, ShouldBeNil) + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + + _, err = imgStore.PutImageManifest(repoName, digest.String(), ispec.MediaTypeImageManifest, content) + So(err, ShouldBeNil) + // upload again same manifest so that we trigger manifest conflict + _, err = imgStore.PutImageManifest(repoName, "1.0", ispec.MediaTypeImageManifest, content) + So(err, ShouldBeNil) + + time.Sleep(500 * time.Millisecond) + + err = imgStore.RunGCRepo(repoName) + So(err, ShouldBeNil) + + // blob shouldn't be gc'ed + found, _, err := imgStore.CheckBlob(repoName, digest) + So(err, ShouldBeNil) + So(found, ShouldEqual, true) + }) + }) +} + func randSeq(n int) string { letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") diff --git a/pkg/storage/s3/s3.go b/pkg/storage/s3/s3.go index 0d7037a9..6ec9f597 100644 --- a/pkg/storage/s3/s3.go +++ b/pkg/storage/s3/s3.go @@ -463,15 +463,11 @@ func (is *ObjectStorage) DeleteImageManifest(repo, reference string, detectColli return err } - manifestDesc, found, err := storage.RemoveManifestDescByReference(&index, reference, detectCollisions) + manifestDesc, err := storage.RemoveManifestDescByReference(&index, reference, detectCollisions) if err != nil { return err } - if !found { - return zerr.ErrManifestNotFound - } - err = storage.UpdateIndexWithPrunedImageManifests(is, &index, repo, manifestDesc, manifestDesc.Digest, is.log) if err != nil { return err