diff --git a/src/controller/proxy/controller.go b/src/controller/proxy/controller.go index 133853fe3..25d1162fd 100644 --- a/src/controller/proxy/controller.go +++ b/src/controller/proxy/controller.go @@ -16,6 +16,7 @@ package proxy import ( "context" + "fmt" "io" "strings" "sync" @@ -53,13 +54,13 @@ 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) (bool, error) + UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, 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) // ProxyManifest proxy the manifest request to the remote server, p is the proxy project, // art is the ArtifactInfo which includes the tag or digest of the manifest - ProxyManifest(ctx context.Context, p *models.Project, art lib.ArtifactInfo) (distribution.Manifest, error) + ProxyManifest(ctx context.Context, p *models.Project, art lib.ArtifactInfo, remote RemoteInterface) (distribution.Manifest, error) } type controller struct { blobCtl blob.Controller @@ -93,22 +94,37 @@ func (c *controller) UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) boo return exist } -func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo) (bool, error) { - if len(art.Digest) == 0 { +func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, error) { + a, err := c.local.GetManifest(ctx, art) + if err != nil { + return false, err + } + if a == nil { return false, nil } - a, err := c.local.GetManifest(ctx, art) - return a != nil, err + // 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 + if err != nil { + return false, 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 dig == a.Digest, nil // digest matches } -func (c *controller) ProxyManifest(ctx context.Context, p *models.Project, art lib.ArtifactInfo) (distribution.Manifest, error) { +func (c *controller) ProxyManifest(ctx context.Context, p *models.Project, art lib.ArtifactInfo, remote RemoteInterface) (distribution.Manifest, error) { var man distribution.Manifest remoteRepo := getRemoteRepo(art) ref := getReference(art) - remote, err := newRemoteHelper(p.RegistryID) - if err != nil { - return man, err - } man, dig, err := remote.Manifest(remoteRepo, ref) if err != nil { if errors.IsNotFoundErr(err) { @@ -154,7 +170,7 @@ func (c *controller) ProxyManifest(ctx context.Context, p *models.Project, art l func (c *controller) ProxyBlob(ctx context.Context, p *models.Project, art lib.ArtifactInfo) (int64, io.ReadCloser, error) { remoteRepo := getRemoteRepo(art) log.Debugf("The blob doesn't exist, proxy the request to the target server, url:%v", remoteRepo) - rHelper, err := newRemoteHelper(p.RegistryID) + rHelper, err := NewRemoteHelper(p.RegistryID) if err != nil { return 0, nil, err } @@ -174,7 +190,7 @@ func (c *controller) ProxyBlob(ctx context.Context, p *models.Project, art lib.A return size, bReader, nil } -func (c *controller) putBlobToLocal(remoteRepo string, localRepo string, desc distribution.Descriptor, r remoteInterface) error { +func (c *controller) putBlobToLocal(remoteRepo string, localRepo string, desc distribution.Descriptor, r RemoteInterface) error { log.Debugf("Put blob to local registry!, sourceRepo:%v, localRepo:%v, digest: %v", remoteRepo, localRepo, desc.Digest) _, bReader, err := r.BlobReader(remoteRepo, string(desc.Digest)) if err != nil { @@ -186,7 +202,7 @@ func (c *controller) putBlobToLocal(remoteRepo string, localRepo string, desc di return err } -func (c *controller) waitAndPushManifest(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, contType string, r remoteInterface) { +func (c *controller) waitAndPushManifest(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, contType string, r RemoteInterface) { if contType == manifestlist.MediaTypeManifestList || contType == v1.MediaTypeImageIndex { err := c.local.PushManifestList(ctx, art.Repository, getReference(art), man) if err != nil { diff --git a/src/controller/proxy/controller_test.go b/src/controller/proxy/controller_test.go index 77ce351c2..92c98142c 100644 --- a/src/controller/proxy/controller_test.go +++ b/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/errors" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "io" @@ -88,7 +89,6 @@ func (l *localInterfaceMock) CheckDependencies(ctx context.Context, repo string, } func (l *localInterfaceMock) DeleteManifest(repo, ref string) { - panic("implement me") } type proxyControllerTestSuite struct { @@ -116,7 +116,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) + result, err := p.ctr.UseLocalManifest(ctx, art, p.remote) p.Assert().Nil(err) p.Assert().True(result) } @@ -126,7 +126,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_False() { dig := "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b" art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig} p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil) - result, err := p.ctr.UseLocalManifest(ctx, art) + result, err := p.ctr.UseLocalManifest(ctx, art, p.remote) p.Assert().Nil(err) p.Assert().False(result) } @@ -135,8 +135,9 @@ func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_False() { ctx := context.Background() art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"} p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil) - result, err := p.ctr.UseLocalManifest(ctx, art) - p.Assert().Nil(err) + p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, "", nil) + result, err := p.ctr.UseLocalManifest(ctx, art, p.remote) + p.Assert().True(errors.IsNotFoundErr(err)) p.Assert().False(result) } diff --git a/src/controller/proxy/remote.go b/src/controller/proxy/remote.go index db3a8d273..94197a8c3 100644 --- a/src/controller/proxy/remote.go +++ b/src/controller/proxy/remote.go @@ -23,12 +23,14 @@ import ( "io" ) -// remoteInterface defines operations related to remote repository under proxy -type remoteInterface interface { +// RemoteInterface defines operations related to remote repository under proxy +type RemoteInterface interface { // BlobReader create a reader for remote blob BlobReader(repo, dig string) (int64, io.ReadCloser, error) // Manifest get manifest by reference Manifest(repo string, ref string) (distribution.Manifest, string, error) + // ManifestExist checks manifest exist, if exist, return digest + ManifestExist(repo string, ref string) (bool, string, error) } // remoteHelper defines operations related to remote repository under proxy @@ -38,8 +40,8 @@ type remoteHelper struct { registryMgr registry.Manager } -// newRemoteHelper create a remoteHelper interface -func newRemoteHelper(regID int64) (*remoteHelper, error) { +// NewRemoteHelper create a remote interface +func NewRemoteHelper(regID int64) (RemoteInterface, error) { r := &remoteHelper{ regID: regID, registryMgr: registry.NewDefaultManager()} @@ -83,3 +85,7 @@ func (r *remoteHelper) BlobReader(repo, dig string) (int64, io.ReadCloser, error func (r *remoteHelper) Manifest(repo string, ref string) (distribution.Manifest, string, error) { return r.registry.PullManifest(repo, ref) } + +func (r *remoteHelper) ManifestExist(repo string, ref string) (bool, string, error) { + return r.registry.ManifestExist(repo, ref) +} diff --git a/src/server/middleware/repoproxy/proxy.go b/src/server/middleware/repoproxy/proxy.go index dc7266d7d..1877e12d1 100644 --- a/src/server/middleware/repoproxy/proxy.go +++ b/src/server/middleware/repoproxy/proxy.go @@ -102,7 +102,11 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e next.ServeHTTP(w, r) return nil } - useLocal, err := proxyCtl.UseLocalManifest(ctx, art) + remote, err := proxy.NewRemoteHelper(p.RegistryID) + if err != nil { + return err + } + useLocal, err := proxyCtl.UseLocalManifest(ctx, art, remote) if err != nil { return err } @@ -111,7 +115,7 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e return nil } log.Debugf("the tag is %v, digest is %v", art.Tag, art.Digest) - err = proxyManifest(ctx, w, r, next, proxyCtl, p, art) + err = proxyManifest(ctx, w, proxyCtl, p, art, remote) if err != nil { if errors.IsNotFoundErr(err) { return err @@ -122,8 +126,8 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e return nil } -func proxyManifest(ctx context.Context, w http.ResponseWriter, r *http.Request, next http.Handler, ctl proxy.Controller, p *models.Project, art lib.ArtifactInfo) error { - man, err := ctl.ProxyManifest(ctx, p, art) +func proxyManifest(ctx context.Context, w http.ResponseWriter, ctl proxy.Controller, p *models.Project, art lib.ArtifactInfo, remote proxy.RemoteInterface) error { + man, err := ctl.ProxyManifest(ctx, p, art, remote) if err != nil { return err }