Skip to content

Add content type and length in header #13805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stonezdj merged 2 commits into from
Jan 6, 2021
Merged

Add content type and length in header #13805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa3002e
Add content type and length in header
stonezdj Dec 11, 2020
1d50be3
Refactor Manifest cache process
stonezdj Dec 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 53 additions & 63 deletions src/controller/proxy/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package proxy
import (
"context"
"fmt"
"github.com/goharbor/harbor/src/controller/tag"
"io"
"strings"
"sync"
Expand All @@ -34,7 +35,6 @@ import (
"github.com/goharbor/harbor/src/lib/log"
"github.com/goharbor/harbor/src/lib/orm"
"github.com/opencontainers/go-digest"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
)

const (
Expand All @@ -43,7 +43,7 @@ const (
maxManifestWait = 10
sleepIntervalSec = 20
// keep manifest list in cache for one week
manifestListCacheIntervalSec = 7 * 24 * 60 * 60
manifestListCacheInterval = 7 * 24 * 60 * 60 * time.Second
)

var (
Expand All @@ -65,31 +65,60 @@ type Controller interface {
// art is the ArtifactInfo which includes the tag or digest of the manifest
ProxyManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (distribution.Manifest, error)
// HeadManifest send manifest head request to the remote server
HeadManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, string, error)
HeadManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *distribution.Descriptor, error)
// EnsureTag ensure tag for digest
EnsureTag(ctx context.Context, art lib.ArtifactInfo, tagName string) error
}
type controller struct {
blobCtl blob.Controller
artifactCtl artifact.Controller
local localInterface
cache cache.Cache
blobCtl blob.Controller
artifactCtl artifact.Controller
local localInterface
cache cache.Cache
handlerRegistry map[string]ManifestCacheHandler
}

// ControllerInstance -- Get the proxy controller instance
func ControllerInstance() Controller {
// Lazy load the controller
// Because LocalHelper is not ready unless core startup completely
once.Do(func() {
l := newLocalHelper()
ctl = &controller{
blobCtl: blob.Ctl,
artifactCtl: artifact.Ctl,
local: newLocalHelper(),
cache: cache.Default(),
blobCtl: blob.Ctl,
artifactCtl: artifact.Ctl,
local: newLocalHelper(),
cache: cache.Default(),
handlerRegistry: NewCacheHandlerRegistry(l),
}
})

return ctl
}

func (c *controller) EnsureTag(ctx context.Context, art lib.ArtifactInfo, tagName string) error {
// search the digest in cache and query with trimmed digest
var trimmedDigest string
err := c.cache.Fetch(TrimmedManifestlist+art.Digest, &trimmedDigest)
if err == cache.ErrNotFound {
// skip to update digest, continue
} else if err != nil {
// for other error, return
return err
} else {
// found in redis, update the digest
art.Digest = trimmedDigest
log.Debugf("Found trimmed digest: %v", trimmedDigest)
}
a, err := c.local.GetManifest(ctx, art)
if err != nil {
return err
}
if a == nil {
return fmt.Errorf("the artifact is not ready yet, failed to tag it to %v", tagName)
}
return tag.Ctl.Ensure(ctx, a.RepositoryID, a.Artifact.ID, tagName)
}

func (c *controller) UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) bool {
if len(art.Digest) == 0 {
return false
Expand Down Expand Up @@ -119,11 +148,11 @@ func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo,
}

remoteRepo := getRemoteRepo(art)
exist, dig, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
exist, desc, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
if err != nil {
return false, nil, err
}
if !exist {
if !exist || desc == nil {
go func() {
c.local.DeleteManifest(remoteRepo, art.Tag)
}()
Expand All @@ -132,18 +161,18 @@ func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo,

var content []byte
if c.cache != nil {
err = c.cache.Fetch(getManifestListKey(art.Repository, dig), &content)
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, dig))
return true, &ManifestList{content, dig, manifestlist.MediaTypeManifestList}, 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
}
if err == cache.ErrNotFound {
log.Debugf("Digest is not found in manifest list cache, key=cache:%v", getManifestListKey(art.Repository, dig))
log.Debugf("Digest is not found in manifest list cache, key=cache:%v", getManifestListKey(art.Repository, string(desc.Digest)))
} else {
log.Errorf("Failed to get manifest list from cache, error: %v", err)
}
}
return a != nil && dig == a.Digest, nil, nil // digest matches
return a != nil && string(desc.Digest) == a.Digest, nil, nil // digest matches
}

func getManifestListKey(repo, dig string) string {
Expand Down Expand Up @@ -198,7 +227,7 @@ func (c *controller) ProxyManifest(ctx context.Context, art lib.ArtifactInfo, re

return man, nil
}
func (c *controller) HeadManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, string, error) {
func (c *controller) HeadManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *distribution.Descriptor, error) {
remoteRepo := getRemoteRepo(art)
ref := getReference(art)
return remote.ManifestExist(remoteRepo, ref)
Expand Down Expand Up @@ -239,53 +268,14 @@ 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)
h, ok := c.handlerRegistry[contType]
if !ok {
h, ok = c.handlerRegistry[defaultHandler]
if !ok {
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 {
log.Errorf("error when push manifest list to local :%v", err)
}
return
}
var waitBlobs []distribution.Descriptor
for n := 0; n < maxManifestWait; n++ {
time.Sleep(sleepIntervalSec * time.Second)
waitBlobs = c.local.CheckDependencies(ctx, art.Repository, man)
if len(waitBlobs) == 0 {
break
}
log.Debugf("Current n=%v artifact: %v:%v", n, art.Repository, art.Tag)
}
if len(waitBlobs) > 0 {
// docker client will skip to pull layers exist in local
// these blobs are not exist in the proxy server
// it will cause the manifest dependency check always fail
// need to push these blobs before push manifest to avoid failure
log.Debug("Waiting blobs not empty, push it to local repo directly")
for _, desc := range waitBlobs {
err := c.putBlobToLocal(remoteRepo, art.Repository, desc, r)
if err != nil {
log.Errorf("Failed to push blob to local repo, error: %v", err)
return
}
}
}
err := c.local.PushManifest(art.Repository, getReference(art), man)
if err != nil {
log.Errorf("failed to push manifest, tag: %v, error %v", art.Tag, err)
}
h.CacheContent(ctx, remoteRepo, man, art, r)
}

// getRemoteRepo get the remote repository name, used in proxy cache
Expand Down
32 changes: 10 additions & 22 deletions src/controller/proxy/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,14 @@ import (
"github.com/goharbor/harbor/src/lib"
_ "github.com/goharbor/harbor/src/lib/cache"
"github.com/goharbor/harbor/src/lib/errors"
testproxy "github.com/goharbor/harbor/src/testing/controller/proxy"
"github.com/opencontainers/go-digest"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
"io"
"testing"
)

type remoteInterfaceMock struct {
mock.Mock
}

func (r *remoteInterfaceMock) BlobReader(repo, dig string) (int64, io.ReadCloser, error) {
panic("implement me")
}

func (r *remoteInterfaceMock) Manifest(repo string, ref string) (distribution.Manifest, string, error) {
panic("implement me")
}

func (r *remoteInterfaceMock) ManifestExist(repo, ref string) (bool, string, error) {
args := r.Called(repo, ref)
return args.Bool(0), args.String(1), args.Error(2)
}

type localInterfaceMock struct {
mock.Mock
}
Expand Down Expand Up @@ -78,7 +63,8 @@ func (l *localInterfaceMock) PushBlob(localRepo string, desc distribution.Descri
}

func (l *localInterfaceMock) PushManifest(repo string, tag string, manifest distribution.Manifest) error {
panic("implement me")
args := l.Called(repo, tag, manifest)
return args.Error(0)
}

func (l *localInterfaceMock) PushManifestList(ctx context.Context, repo string, tag string, man distribution.Manifest) error {
Expand All @@ -95,14 +81,14 @@ func (l *localInterfaceMock) DeleteManifest(repo, ref string) {
type proxyControllerTestSuite struct {
suite.Suite
local *localInterfaceMock
remote *remoteInterfaceMock
remote *testproxy.RemoteInterface
ctr Controller
proj *models.Project
}

func (p *proxyControllerTestSuite) SetupTest() {
p.local = &localInterfaceMock{}
p.remote = &remoteInterfaceMock{}
p.remote = &testproxy.RemoteInterface{}
p.proj = &models.Project{RegistryID: 1}
p.ctr = &controller{
blobCtl: blob.Ctl,
Expand All @@ -125,8 +111,9 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_True() {
func (p *proxyControllerTestSuite) TestUseLocalManifest_False() {
ctx := context.Background()
dig := "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"
desc := &distribution.Descriptor{Digest: digest.Digest(dig)}
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(true, dig, nil)
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(true, desc, nil)
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil)
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
p.Assert().Nil(err)
Expand All @@ -136,8 +123,9 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_False() {
func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_False() {
ctx := context.Background()
art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"}
desc := &distribution.Descriptor{}
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, "", nil)
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, nil)
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
p.Assert().True(errors.IsNotFoundErr(err))
p.Assert().False(result)
Expand Down
Loading