From 3c0c51fcbe5c1be0e0d2da83d0351d268f34f8e2 Mon Sep 17 00:00:00 2001 From: peusebiu Date: Thu, 6 Oct 2022 19:41:16 +0300 Subject: [PATCH] fix(sync): also sync image index mediatype (#847) Signed-off-by: Petu Eusebiu --- pkg/extensions/sync/sync.go | 1 + pkg/extensions/sync/sync_internal_test.go | 271 ++++++++++++++++------ pkg/extensions/sync/sync_test.go | 104 +++++++++ pkg/extensions/sync/utils.go | 77 +++++- 4 files changed, 373 insertions(+), 80 deletions(-) diff --git a/pkg/extensions/sync/sync.go b/pkg/extensions/sync/sync.go index cd130cae..481a52ad 100644 --- a/pkg/extensions/sync/sync.go +++ b/pkg/extensions/sync/sync.go @@ -267,6 +267,7 @@ func getCopyOptions(upstreamCtx, localCtx *types.SystemContext) copy.Options { ReportWriter: io.Discard, ForceManifestMIMEType: ispec.MediaTypeImageManifest, // force only oci manifest MIME type PreserveDigests: true, + ImageListSelection: copy.CopyAllImages, } return options diff --git a/pkg/extensions/sync/sync_internal_test.go b/pkg/extensions/sync/sync_internal_test.go index d6510c24..b39445cc 100644 --- a/pkg/extensions/sync/sync_internal_test.go +++ b/pkg/extensions/sync/sync_internal_test.go @@ -1,6 +1,7 @@ package sync import ( + "bytes" "context" "encoding/json" "fmt" @@ -26,6 +27,7 @@ import ( "zotregistry.io/zot/pkg/storage" "zotregistry.io/zot/pkg/storage/local" "zotregistry.io/zot/pkg/test" + "zotregistry.io/zot/pkg/test/mocks" ) const ( @@ -454,99 +456,226 @@ func TestSyncInternal(t *testing.T) { manifestContent, _, _, err := testImageStore.GetImageManifest(testImage, testImageTag) So(err, ShouldBeNil) - var manifest ispec.Manifest + Convey("index image errors", func() { + // create an image index on upstream + repo := "index" - if err := json.Unmarshal(manifestContent, &manifest); err != nil { - panic(err) - } + var index ispec.Index + index.SchemaVersion = 2 + index.MediaType = ispec.MediaTypeImageIndex - if err := os.Chmod(storageDir, 0o000); err != nil { - panic(err) - } + // upload multiple manifests + for i := 0; i < 2; i++ { + config, layers, manifest, err := test.GetImageComponents(1000 + i) + So(err, ShouldBeNil) - if os.Geteuid() != 0 { - So(func() { - _ = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) - }, ShouldPanic) - } + for _, layer := range layers { + // upload layer + _, _, err := testImageStore.FullBlobUpload(repo, bytes.NewReader(layer), godigest.FromBytes(layer).String()) + So(err, ShouldBeNil) + } - if err := os.Chmod(storageDir, 0o755); err != nil { - panic(err) - } + configContent, err := json.Marshal(config) + So(err, ShouldBeNil) - if err := os.Chmod(path.Join(testRootDir, testImage, "blobs", "sha256", - manifest.Layers[0].Digest.Hex()), 0o000); err != nil { - panic(err) - } + configDigest := godigest.FromBytes(configContent) - err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) - So(err, ShouldNotBeNil) + _, _, err = testImageStore.FullBlobUpload(repo, bytes.NewReader(configContent), configDigest.String()) + So(err, ShouldBeNil) - if err := os.Chmod(path.Join(testRootDir, testImage, "blobs", "sha256", - manifest.Layers[0].Digest.Hex()), 0o755); err != nil { - panic(err) - } + manifestContent, err := json.Marshal(manifest) + So(err, ShouldBeNil) - cachedManifestConfigPath := path.Join(imageStore.RootDir(), testImage, SyncBlobUploadDir, - testImage, "blobs", "sha256", manifest.Config.Digest.Hex()) - if err := os.Chmod(cachedManifestConfigPath, 0o000); err != nil { - panic(err) - } + manifestDigest := godigest.FromBytes(manifestContent) - err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) - So(err, ShouldNotBeNil) + _, err = testImageStore.PutImageManifest(repo, manifestDigest.String(), + ispec.MediaTypeImageManifest, manifestContent) + So(err, ShouldBeNil) - if err := os.Chmod(cachedManifestConfigPath, 0o755); err != nil { - panic(err) - } + index.Manifests = append(index.Manifests, ispec.Descriptor{ + Digest: manifestDigest, + MediaType: ispec.MediaTypeImageManifest, + Size: int64(len(manifestContent)), + }) + } - cachedManifestBackup, err := os.ReadFile(cachedManifestConfigPath) - if err != nil { - panic(err) - } + content, err := json.Marshal(index) + So(err, ShouldBeNil) + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) - configDigestBackup := manifest.Config.Digest - manifest.Config.Digest = "not what it needs to be" - manifestBuf, err := json.Marshal(manifest) - if err != nil { - panic(err) - } + // upload index image + _, err = testImageStore.PutImageManifest(repo, "latest", ispec.MediaTypeImageIndex, content) + So(err, ShouldBeNil) - if err = os.WriteFile(cachedManifestConfigPath, manifestBuf, 0o600); err != nil { - panic(err) - } + err = pushSyncedLocalImage(repo, "latest", testRootDir, imageStore, log) + So(err, ShouldBeNil) - if err = os.Chmod(cachedManifestConfigPath, 0o755); err != nil { - panic(err) - } + // trigger error on manifest pull + err = os.Chmod(path.Join(testRootDir, repo, "blobs", + index.Manifests[0].Digest.Algorithm().String(), index.Manifests[0].Digest.Encoded()), 0o000) + So(err, ShouldBeNil) - err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) - So(err, ShouldNotBeNil) + err = pushSyncedLocalImage(repo, "latest", testRootDir, imageStore, log) + So(err, ShouldNotBeNil) - manifest.Config.Digest = configDigestBackup - manifestBuf = cachedManifestBackup + err = os.Chmod(path.Join(testRootDir, repo, "blobs", + index.Manifests[0].Digest.Algorithm().String(), index.Manifests[0].Digest.Encoded()), local.DefaultDirPerms) + So(err, ShouldBeNil) - if err := os.Remove(cachedManifestConfigPath); err != nil { - panic(err) - } + // trigger linter error on manifest push + imageStoreWithLinter := local.NewImageStore(t.TempDir(), false, storage.DefaultGCDelay, + false, false, log, metrics, &mocks.MockedLint{ + LintFn: func(repo string, manifestDigest godigest.Digest, imageStore storage.ImageStore) (bool, error) { + return false, nil + }, + }, + ) - if err = os.WriteFile(cachedManifestConfigPath, manifestBuf, 0o600); err != nil { - panic(err) - } + err = pushSyncedLocalImage(repo, "latest", testRootDir, imageStoreWithLinter, log) + // linter error will be ignored by sync + So(err, ShouldBeNil) - if err = os.Chmod(cachedManifestConfigPath, 0o755); err != nil { - panic(err) - } + // trigger error on blob + var manifest ispec.Manifest - mDigest := godigest.FromBytes(manifestContent) + manifestContent, _, mediaType, err := testImageStore.GetImageManifest(repo, index.Manifests[0].Digest.String()) + So(err, ShouldBeNil) + So(mediaType, ShouldEqual, ispec.MediaTypeImageManifest) - manifestPath := path.Join(imageStore.RootDir(), testImage, "blobs", mDigest.Algorithm().String(), mDigest.Encoded()) - if err := os.MkdirAll(manifestPath, 0o000); err != nil { - panic(err) - } + err = json.Unmarshal(manifestContent, &manifest) + So(err, ShouldBeNil) - err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) - So(err, ShouldNotBeNil) + configBlobPath := path.Join(testRootDir, repo, "blobs", + manifest.Config.Digest.Algorithm().String(), manifest.Config.Digest.Encoded()) + err = os.Chmod(configBlobPath, 0o000) + So(err, ShouldBeNil) + + // remove destination blob, so that pushSyncedLocalImage will try to push it again + err = os.Remove(path.Join(imageStore.RootDir(), repo, "blobs", + manifest.Config.Digest.Algorithm().String(), manifest.Config.Digest.Encoded())) + So(err, ShouldBeNil) + + err = pushSyncedLocalImage(repo, "latest", testRootDir, imageStore, log) + So(err, ShouldNotBeNil) + + err = os.Chmod(configBlobPath, local.DefaultDirPerms) + So(err, ShouldBeNil) + + err = os.RemoveAll(path.Join(imageStore.RootDir(), repo, "index.json")) + So(err, ShouldBeNil) + + // remove destination blob, so that pushSyncedLocalImage will try to push it again + indexManifestPath := path.Join(imageStore.RootDir(), repo, "blobs", + digest.Algorithm().String(), digest.Encoded()) + err = os.Remove(indexManifestPath) + So(err, ShouldBeNil) + + err = os.MkdirAll(indexManifestPath, 0o000) + So(err, ShouldBeNil) + + err = pushSyncedLocalImage(repo, "latest", testRootDir, imageStore, log) + So(err, ShouldNotBeNil) + + err = os.Remove(indexManifestPath) + So(err, ShouldBeNil) + }) + + Convey("manifest image errors", func() { + var manifest ispec.Manifest + + if err := json.Unmarshal(manifestContent, &manifest); err != nil { + panic(err) + } + + if err := os.Chmod(storageDir, 0o000); err != nil { + panic(err) + } + + if os.Geteuid() != 0 { + So(func() { + _ = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) + }, ShouldPanic) + } + + if err := os.Chmod(storageDir, 0o755); err != nil { + panic(err) + } + + if err := os.Chmod(path.Join(testRootDir, testImage, "blobs", "sha256", + manifest.Layers[0].Digest.Hex()), 0o000); err != nil { + panic(err) + } + + err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) + So(err, ShouldNotBeNil) + + if err := os.Chmod(path.Join(testRootDir, testImage, "blobs", "sha256", + manifest.Layers[0].Digest.Hex()), 0o755); err != nil { + panic(err) + } + + cachedManifestConfigPath := path.Join(imageStore.RootDir(), testImage, SyncBlobUploadDir, + testImage, "blobs", "sha256", manifest.Config.Digest.Hex()) + if err := os.Chmod(cachedManifestConfigPath, 0o000); err != nil { + panic(err) + } + + err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) + So(err, ShouldNotBeNil) + + if err := os.Chmod(cachedManifestConfigPath, 0o755); err != nil { + panic(err) + } + + cachedManifestBackup, err := os.ReadFile(cachedManifestConfigPath) + if err != nil { + panic(err) + } + + configDigestBackup := manifest.Config.Digest + manifest.Config.Digest = "not what it needs to be" + manifestBuf, err := json.Marshal(manifest) + if err != nil { + panic(err) + } + + if err = os.WriteFile(cachedManifestConfigPath, manifestBuf, 0o600); err != nil { + panic(err) + } + + if err = os.Chmod(cachedManifestConfigPath, 0o755); err != nil { + panic(err) + } + + err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) + So(err, ShouldNotBeNil) + + manifest.Config.Digest = configDigestBackup + manifestBuf = cachedManifestBackup + + if err := os.Remove(cachedManifestConfigPath); err != nil { + panic(err) + } + + if err = os.WriteFile(cachedManifestConfigPath, manifestBuf, 0o600); err != nil { + panic(err) + } + + if err = os.Chmod(cachedManifestConfigPath, 0o755); err != nil { + panic(err) + } + + mDigest := godigest.FromBytes(manifestContent) + + manifestPath := path.Join(imageStore.RootDir(), testImage, "blobs", mDigest.Algorithm().String(), mDigest.Encoded()) + if err := os.MkdirAll(manifestPath, 0o000); err != nil { + panic(err) + } + + err = pushSyncedLocalImage(testImage, testImageTag, testRootDir, imageStore, log) + So(err, ShouldNotBeNil) + }) }) } diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index 01767227..ea40e7c5 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -4087,6 +4087,110 @@ func TestSyncWithDestination(t *testing.T) { }) } +func TestSyncImageIndex(t *testing.T) { + Convey("Verify syncing image indexes works", t, func() { + updateDuration, _ := time.ParseDuration("30m") + + sctlr, srcBaseURL, _, _, _ := startUpstreamServer(t, false, false) + + defer func() { + sctlr.Shutdown() + }() + + regex := ".*" + var semver bool + tlsVerify := false + + syncRegistryConfig := sync.RegistryConfig{ + Content: []sync.Content{ + { + Prefix: "index", + Tags: &sync.Tags{ + Regex: ®ex, + Semver: &semver, + }, + }, + }, + URLs: []string{srcBaseURL}, + OnDemand: false, + PollInterval: updateDuration, + TLSVerify: &tlsVerify, + } + + defaultVal := true + syncConfig := &sync.Config{ + Enable: &defaultVal, + Registries: []sync.RegistryConfig{syncRegistryConfig}, + } + + // 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(), + }, + srcBaseURL, + "index") + 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) + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + resp, err := resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + SetBody(content).Put(srcBaseURL + "/v2/index/manifests/latest") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + Get(srcBaseURL + "/v2/index/manifests/latest") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Body(), ShouldNotBeEmpty) + So(resp.Header().Get("Content-Type"), ShouldNotBeEmpty) + + // start downstream server + dctlr, destBaseURL, _, _ := startDownstreamServer(t, false, syncConfig) + + defer func() { + dctlr.Shutdown() + }() + + // give it time to set up sync + t.Logf("waitsync(%s, %s)", dctlr.Config.Storage.RootDirectory, "index") + waitSync(dctlr.Config.Storage.RootDirectory, "index") + + resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex). + Get(destBaseURL + "/v2/index/manifests/latest") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Body(), ShouldNotBeEmpty) + So(resp.Header().Get("Content-Type"), ShouldNotBeEmpty) + }) +} + func generateKeyPairs(tdir string) { // generate a keypair os.Setenv("COSIGN_PASSWORD", "") diff --git a/pkg/extensions/sync/utils.go b/pkg/extensions/sync/utils.go index 7aee7f1b..34972cdf 100644 --- a/pkg/extensions/sync/utils.go +++ b/pkg/extensions/sync/utils.go @@ -274,7 +274,7 @@ func pushSyncedLocalImage(localRepo, tag, localCachePath string, cacheImageStore := local.NewImageStore(localCachePath, false, storage.DefaultGCDelay, false, false, log, metrics, nil) - manifestContent, _, _, err := cacheImageStore.GetImageManifest(localRepo, tag) + manifestContent, _, mediaType, err := cacheImageStore.GetImageManifest(localRepo, tag) if err != nil { log.Error().Str("errorType", TypeOf(err)). Err(err).Str("dir", path.Join(cacheImageStore.RootDir(), localRepo)). @@ -283,8 +283,74 @@ func pushSyncedLocalImage(localRepo, tag, localCachePath string, return err } + // is image manifest + if mediaType == ispec.MediaTypeImageManifest { + if err := copyManifest(localRepo, manifestContent, tag, cacheImageStore, imageStore, log); err != nil { + if errors.Is(err, zerr.ErrImageLintAnnotations) { + log.Error().Str("errorType", TypeOf(err)). + Err(err).Msg("couldn't upload manifest because of missing annotations") + + return nil + } + + return err + } + + return nil + } + + // is image index + var indexManifest ispec.Index + + if err := json.Unmarshal(manifestContent, &indexManifest); err != nil { + log.Error().Str("errorType", TypeOf(err)). + Err(err).Str("dir", path.Join(cacheImageStore.RootDir(), localRepo)). + Msg("invalid JSON") + + return err + } + + for _, manifest := range indexManifest.Manifests { + manifestBuf, err := cacheImageStore.GetBlobContent(localRepo, manifest.Digest.String()) + if err != nil { + log.Error().Str("errorType", TypeOf(err)). + Err(err).Str("dir", path.Join(cacheImageStore.RootDir(), localRepo)).Str("digest", manifest.Digest.String()). + Msg("couldn't find manifest which is part of an image index") + + return err + } + + if err := copyManifest(localRepo, manifestBuf, manifest.Digest.String(), + cacheImageStore, imageStore, log); err != nil { + if errors.Is(err, zerr.ErrImageLintAnnotations) { + log.Error().Str("errorType", TypeOf(err)). + Err(err).Msg("couldn't upload manifest because of missing annotations") + + return nil + } + + return err + } + } + + _, err = imageStore.PutImageManifest(localRepo, tag, mediaType, manifestContent) + if err != nil { + log.Error().Str("errorType", TypeOf(err)). + Err(err).Msg("couldn't upload manifest") + + return err + } + + return nil +} + +func copyManifest(localRepo string, manifestContent []byte, reference string, + cacheImageStore, imageStore storage.ImageStore, log log.Logger, +) error { var manifest ispec.Manifest + var err error + if err := json.Unmarshal(manifestContent, &manifest); err != nil { log.Error().Str("errorType", TypeOf(err)). Err(err).Str("dir", path.Join(cacheImageStore.RootDir(), localRepo)). @@ -307,16 +373,9 @@ func pushSyncedLocalImage(localRepo, tag, localCachePath string, return err } - _, err = imageStore.PutImageManifest(localRepo, tag, + _, err = imageStore.PutImageManifest(localRepo, reference, ispec.MediaTypeImageManifest, manifestContent) if err != nil { - if errors.Is(err, zerr.ErrImageLintAnnotations) { - log.Error().Str("errorType", TypeOf(err)). - Err(err).Msg("couldn't upload manifest because of missing annotations") - - return nil - } - log.Error().Str("errorType", TypeOf(err)). Err(err).Msg("couldn't upload manifest")