From 6b77c1169681594995d3b5f0afe452549d1a881b Mon Sep 17 00:00:00 2001 From: stonezdj Date: Mon, 1 Nov 2021 16:57:08 +0800 Subject: [PATCH] Cache content type for manifest list and image index in perspective manifest list: application/vnd.docker.distribution.manifest.list.v2+json image index: application/vnd.oci.image.index.v1+json fixes #15837 Signed-off-by: stonezdj --- src/controller/proxy/controller.go | 38 +++++++++++++++++++-------- src/controller/proxy/manifestcache.go | 12 ++++++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/controller/proxy/controller.go b/src/controller/proxy/controller.go index c92fbdd85..e105c5f48 100644 --- a/src/controller/proxy/controller.go +++ b/src/controller/proxy/controller.go @@ -23,7 +23,6 @@ import ( "time" "github.com/docker/distribution" - "github.com/docker/distribution/manifest/manifestlist" "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/controller/blob" "github.com/goharbor/harbor/src/controller/event/operator" @@ -138,6 +137,10 @@ type ManifestList struct { ContentType string } +// UseLocalManifest check if these manifest could be found in local registry, +// the return error should be nil when it is not found in local and need to delegate to remote registry +// the return error should be NotFoundError when it is not found in remote registry +// the error will be captured by framework and return 404 to client func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *ManifestList, error) { a, err := c.local.GetManifest(ctx, art) if err != nil { @@ -161,26 +164,39 @@ func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, } var content []byte - if c.cache != nil { - err = c.cache.Fetch(getManifestListKey(art.Repository, string(desc.Digest)), &content) - if err == nil { - log.Debugf("Get the manifest list with key=cache:%v", getManifestListKey(art.Repository, string(desc.Digest))) - return true, &ManifestList{content, string(desc.Digest), manifestlist.MediaTypeManifestList}, nil - } + var contentType string + if c.cache == nil { + return a != nil && string(desc.Digest) == a.Digest, nil, nil // digest matches + } + + err = c.cache.Fetch(manifestListKey(art.Repository, string(desc.Digest)), &content) + if err != nil { if err == cache.ErrNotFound { - log.Debugf("Digest is not found in manifest list cache, key=cache:%v", getManifestListKey(art.Repository, string(desc.Digest))) + log.Debugf("Digest is not found in manifest list cache, key=cache:%v", manifestListKey(art.Repository, string(desc.Digest))) } else { log.Errorf("Failed to get manifest list from cache, error: %v", err) } + return a != nil && string(desc.Digest) == a.Digest, nil, nil } - return a != nil && string(desc.Digest) == a.Digest, nil, nil // digest matches + err = c.cache.Fetch(manifestListContentTypeKey(art.Repository, string(desc.Digest)), &contentType) + if err != nil { + log.Debugf("failed to get the manifest list content type, not use local. error:%v", err) + return false, nil, nil + } + log.Debugf("Get the manifest list with key=cache:%v", manifestListKey(art.Repository, string(desc.Digest))) + return true, &ManifestList{content, string(desc.Digest), contentType}, nil + } -func getManifestListKey(repo, dig string) string { +func manifestListKey(repo, dig string) string { // actual redis key format is cache:manifestlist::sha256:xxxx return "manifestlist:" + repo + ":" + dig } +func manifestListContentTypeKey(rep, dig string) string { + return manifestListKey(rep, dig) + ":contenttype" +} + func (c *controller) ProxyManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (distribution.Manifest, error) { var man distribution.Manifest remoteRepo := getRemoteRepo(art) @@ -279,7 +295,7 @@ func (c *controller) waitAndPushManifest(ctx context.Context, remoteRepo string, return } } - h.CacheContent(ctx, remoteRepo, man, art, r) + h.CacheContent(ctx, remoteRepo, man, art, r, contType) } // getRemoteRepo get the remote repository name, used in proxy cache diff --git a/src/controller/proxy/manifestcache.go b/src/controller/proxy/manifestcache.go index 0bee4436a..05e88826a 100644 --- a/src/controller/proxy/manifestcache.go +++ b/src/controller/proxy/manifestcache.go @@ -51,7 +51,7 @@ func NewCacheHandlerRegistry(local localInterface) map[string]ManifestCacheHandl // ManifestCacheHandler define how to cache manifest content type ManifestCacheHandler interface { // CacheContent - cache the content of the manifest - CacheContent(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, r RemoteInterface) + CacheContent(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, r RemoteInterface, contentType string) } // ManifestListCache handle Manifest list type and index type @@ -61,14 +61,18 @@ type ManifestListCache struct { } // CacheContent ... -func (m *ManifestListCache) CacheContent(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, r RemoteInterface) { +func (m *ManifestListCache) CacheContent(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, r RemoteInterface, contentType string) { _, payload, err := man.Payload() if err != nil { log.Errorf("failed to get payload, error %v", err) return } - key := getManifestListKey(art.Repository, art.Digest) + key := manifestListKey(art.Repository, art.Digest) log.Debugf("cache manifest list with key=cache:%v", key) + err = m.cache.Save(manifestListContentTypeKey(art.Repository, art.Digest), contentType, manifestListCacheInterval) + if err != nil { + log.Errorf("failed to cache content type, error %v", err) + } err = m.cache.Save(key, payload, manifestListCacheInterval) if err != nil { log.Errorf("failed to cache payload, error %v", err) @@ -164,7 +168,7 @@ type ManifestCache struct { } // CacheContent ... -func (m *ManifestCache) CacheContent(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, r RemoteInterface) { +func (m *ManifestCache) CacheContent(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, r RemoteInterface, contentType string) { var waitBlobs []distribution.Descriptor for n := 0; n < maxManifestWait; n++ { time.Sleep(sleepIntervalSec * time.Second)