Cache manifest list for proxy cache

Fixes #13566: Quota of dockerhub is still used in v2.1.1 after the image is cached
Cache manifest list in redis cache.
Trade off between efficiency and data integrity, it might cause the proxy cache return the full content of a manifest list instead of the actual manifest list saved in the Harbor storage, which is a part of the manifest list. but this change doesn't break any /v2/ API, just caches full manifest list.

Signed-off-by: stonezdj <stonezdj@gmail.com>

src/controller/proxy/controller.go

@@ -29,6 +29,7 @@ import (
 	"github.com/goharbor/harbor/src/controller/blob"
 	"github.com/goharbor/harbor/src/controller/event/operator"
 	"github.com/goharbor/harbor/src/lib"
+	"github.com/goharbor/harbor/src/lib/cache"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/orm"
@@ -41,6 +42,8 @@ const (
 	maxManifestListWait = 20
 	maxManifestWait     = 10
 	sleepIntervalSec    = 20
+	// keep manifest list in cache for one week
+	manifestListCacheIntervalSec = 7 * 24 * 60 * 60
 )
 
 var (
@@ -54,7 +57,7 @@ type Controller interface {
 	// UseLocalBlob check if the blob should use local copy
 	UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) bool
 	// UseLocalManifest check manifest should use local copy
-	UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, error)
+	UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *ManifestList, error)
 	// ProxyBlob proxy the blob request to the remote server, p is the proxy project
 	// art is the ArtifactInfo which includes the digest of the blob
 	ProxyBlob(ctx context.Context, p *models.Project, art lib.ArtifactInfo) (int64, io.ReadCloser, error)
@@ -68,6 +71,7 @@ type controller struct {
 	blobCtl     blob.Controller
 	artifactCtl artifact.Controller
 	local       localInterface
+	cache       cache.Cache
 }
 
 // ControllerInstance -- Get the proxy controller instance
@@ -79,6 +83,7 @@ func ControllerInstance() Controller {
 			blobCtl:     blob.Ctl,
 			artifactCtl: artifact.Ctl,
 			local:       newLocalHelper(),
+			cache:       cache.Default(),
 		}
 	})
 
@@ -96,33 +101,55 @@ func (c *controller) UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) boo
 	return exist
 }
 
-func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, error) {
+// ManifestList ...
+type ManifestList struct {
+	Content     []byte
+	Digest      string
+	ContentType string
+}
+
+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 {
-		return false, err
+		return false, nil, err
 	}
-	if a == nil {
-		return false, nil
+	// Pull by digest when artifact exist in local
+	if a != nil && len(art.Digest) > 0 {
+		return true, nil, nil
 	}
-	// Pull by digest
-	if len(art.Digest) > 0 {
-		return true, nil
-	}
-	// Pull by tag
+
 	remoteRepo := getRemoteRepo(art)
-	exist, dig, err := remote.ManifestExist(remoteRepo, art.Tag) // HEAD
+	exist, dig, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
 	if err != nil {
-		return false, err
+		return false, nil, err
 	}
 	if !exist {
 		go func() {
 			c.local.DeleteManifest(remoteRepo, art.Tag)
 		}()
-		return false, errors.NotFoundError(fmt.Errorf("repo %v, tag %v not found", art.Repository, art.Tag))
+		return false, nil, errors.NotFoundError(fmt.Errorf("repo %v, tag %v not found", art.Repository, art.Tag))
 	}
-	return dig == a.Digest, nil // digest matches
+
+	var content []byte
+	if c.cache != nil {
+		err = c.cache.Fetch(getManifestListKey(art.Repository, dig), &content)
+		if err == nil {
+			log.Debugf("Get the manifest list with key=cache:%v", getManifestListKey(art.Repository, dig))
+			return true, &ManifestList{content, dig, manifestlist.MediaTypeManifestList}, nil
+		}
+		if err == cache.ErrNotFound {
+			log.Debugf("Digest is not found in manifest list cache, key=cache:%v", getManifestListKey(art.Repository, dig))
+		} else {
+			log.Errorf("Failed to get manifest list from cache, error: %v", err)
+		}
+	}
+	return a != nil && dig == a.Digest, nil, nil // digest matches
 }
 
+func getManifestListKey(repo, dig string) string {
+	// actual redis key format is cache:manifestlist:<repo name>:sha256:xxxx
+	return "manifestlist:" + repo + ":" + dig
+}
 func (c *controller) ProxyManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (distribution.Manifest, error) {
 	var man distribution.Manifest
 	remoteRepo := getRemoteRepo(art)
@@ -150,8 +177,11 @@ func (c *controller) ProxyManifest(ctx context.Context, art lib.ArtifactInfo, re
 		}
 		// Push manifest to local when pull with digest, or artifact not found, or digest mismatch
 		if len(art.Tag) == 0 || a == nil || a.Digest != dig {
-			// pull with digest
-			c.waitAndPushManifest(ctx, remoteRepo, man, art, ct, remote)
+			artInfo := art
+			if len(artInfo.Digest) == 0 {
+				artInfo.Digest = dig
+			}
+			c.waitAndPushManifest(ctx, remoteRepo, man, artInfo, ct, remote)
 		}
 
 		// Query artifact after push
@@ -209,6 +239,19 @@ func (c *controller) putBlobToLocal(remoteRepo string, localRepo string, desc di
 }
 
 func (c *controller) waitAndPushManifest(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, contType string, r RemoteInterface) {
+	if contType == manifestlist.MediaTypeManifestList {
+		_, payload, err := man.Payload()
+		if err != nil {
+			log.Errorf("failed to get payload, error %v", err)
+			return
+		}
+		key := getManifestListKey(art.Repository, art.Digest)
+		log.Debugf("Cache manifest list with key=cache:%v", key)
+		err = c.cache.Save(key, payload, manifestListCacheIntervalSec)
+		if err != nil {
+			log.Errorf("failed to cache payload, error %v", err)
+		}
+	}
 	if contType == manifestlist.MediaTypeManifestList || contType == v1.MediaTypeImageIndex {
 		err := c.local.PushManifestList(ctx, art.Repository, getReference(art), man)
 		if err != nil {

src/controller/proxy/controller_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/controller/artifact"
 	"github.com/goharbor/harbor/src/controller/blob"
 	"github.com/goharbor/harbor/src/lib"
+	_ "github.com/goharbor/harbor/src/lib/cache"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
@@ -116,7 +117,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_True() {
 	art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
 	p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
 
-	result, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
+	result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
 	p.Assert().Nil(err)
 	p.Assert().True(result)
 }
@@ -125,8 +126,9 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_False() {
 	ctx := context.Background()
 	dig := "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"
 	art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
+	p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(true, dig, nil)
 	p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil)
-	result, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
+	result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
 	p.Assert().Nil(err)
 	p.Assert().False(result)
 }
@@ -136,7 +138,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_False() {
 	art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"}
 	p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
 	p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, "", nil)
-	result, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
+	result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
 	p.Assert().True(errors.IsNotFoundErr(err))
 	p.Assert().False(result)
 }

src/controller/proxy/local.go

@@ -127,7 +127,7 @@ func (l *localHelper) PushManifest(repo string, ref string, manifest distributio
 
 // DeleteManifest cleanup delete tag from local repo
 func (l *localHelper) DeleteManifest(repo, ref string) {
-	log.Debug("Remove tag from repo if it is exist")
+	log.Debugf("Remove tag from repo if it is exist, repo: %v ref: %v", repo, ref)
 	if err := l.registry.DeleteManifest(repo, ref); err != nil {
 		// sometimes user pull a non-exist image
 		log.Warningf("failed to remove artifact, error %v", err)

src/server/middleware/repoproxy/proxy.go

@@ -36,6 +36,13 @@ import (
 
 var registryMgr = registry.NewDefaultManager()
 
+const (
+	contentLength       = "Content-Length"
+	contentType         = "Content-Type"
+	dockerContentDigest = "Docker-Content-Digest"
+	etag                = "Etag"
+)
+
 // BlobGetMiddleware handle get blob request
 func BlobGetMiddleware() func(http.Handler) http.Handler {
 	return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
@@ -106,14 +113,28 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e
 	if err != nil {
 		return err
 	}
-	useLocal, err := proxyCtl.UseLocalManifest(ctx, art, remote)
+	useLocal, man, err := proxyCtl.UseLocalManifest(ctx, art, remote)
+
 	if err != nil {
 		return err
 	}
 	if useLocal {
+		if man != nil {
+			w.Header().Set(contentLength, fmt.Sprintf("%v", len(man.Content)))
+			w.Header().Set(contentType, man.ContentType)
+			w.Header().Set(dockerContentDigest, man.Digest)
+			w.Header().Set(etag, man.Digest)
+			if r.Method == http.MethodGet {
+				w.Write(man.Content)
+			}
+			return nil
+		}
 		next.ServeHTTP(w, r)
 		return nil
 	}
+
+	log.Warningf("Artifact: %v:%v, digest:%v is not found in proxy cache, fetch it from remote repo", art.Repository, art.Tag, art.Digest)
+
 	log.Debugf("the tag is %v, digest is %v", art.Tag, art.Digest)
 	if r.Method == http.MethodHead {
 		err = proxyManifestHead(ctx, w, proxyCtl, p, art, remote)
@@ -163,12 +184,12 @@ func canProxy(p *models.Project) bool {
 
 func setHeaders(w http.ResponseWriter, size int64, mediaType string, dig string) {
 	h := w.Header()
-	h.Set("Content-Length", fmt.Sprintf("%v", size))
+	h.Set(contentLength, fmt.Sprintf("%v", size))
 	if len(mediaType) > 0 {
-		h.Set("Content-Type", mediaType)
+		h.Set(contentType, mediaType)
 	}
-	h.Set("Docker-Content-Digest", dig)
-	h.Set("Etag", dig)
+	h.Set(dockerContentDigest, dig)
+	h.Set(etag, dig)
 }
 
 // isProxySession check if current security context is proxy session
@@ -213,7 +234,7 @@ func proxyManifestHead(ctx context.Context, w http.ResponseWriter, ctl proxy.Con
 	if !exist {
 		return errors.NotFoundError(fmt.Errorf("The tag %v:%v is not found", art.Repository, art.Tag))
 	}
-	w.Header().Set("Docker-Content-Digest", dig)
-	w.Header().Set("Etag", dig)
+	w.Header().Set(dockerContentDigest, dig)
+	w.Header().Set(etag, dig)
 	return nil
 }