Add content type and length in header

Fixes #13740
Update ManifestExist to return Descriptor instead of digest

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

src/controller/proxy/controller.go

@@ -65,7 +65,7 @@ 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)
 }
 type controller struct {
 	blobCtl     blob.Controller
@@ -119,11 +119,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)
 		}()
@@ -132,18 +132,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))
+			log.Debugf("Get the manifest list with key=cache:%v", getManifestListKey(art.Repository, string(desc.Digest)))
-			return true, &ManifestList{content, dig, manifestlist.MediaTypeManifestList}, nil
+			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 {
@@ -198,7 +198,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)

src/controller/proxy/controller_test.go

@@ -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
 }
@@ -95,14 +80,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,
@@ -125,8 +110,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)
@@ -136,8 +122,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)

src/controller/proxy/remote.go

@@ -30,7 +30,7 @@ type RemoteInterface interface {
 	// 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)
+	ManifestExist(repo string, ref string) (bool, *distribution.Descriptor, error)
 }
 
 // remoteHelper defines operations related to remote repository under proxy
@@ -86,6 +86,6 @@ func (r *remoteHelper) Manifest(repo string, ref string) (distribution.Manifest,
 	return r.registry.PullManifest(repo, ref)
 }
 
-func (r *remoteHelper) ManifestExist(repo string, ref string) (bool, string, error) {
+func (r *remoteHelper) ManifestExist(repo string, ref string) (bool, *distribution.Descriptor, error) {
 	return r.registry.ManifestExist(repo, ref)
 }

src/controller/replication/flow/mock_adapter_test.go

@@ -134,7 +134,7 @@ func (_m *mockAdapter) Info() (*model.RegistryInfo, error) {
 }
 
 // ManifestExist provides a mock function with given fields: repository, reference
-func (_m *mockAdapter) ManifestExist(repository string, reference string) (bool, string, error) {
+func (_m *mockAdapter) ManifestExist(repository string, reference string) (bool, *distribution.Descriptor, error) {
 	ret := _m.Called(repository, reference)
 
 	var r0 bool
@@ -144,11 +144,13 @@ func (_m *mockAdapter) ManifestExist(repository string, reference string) (bool,
 		r0 = ret.Get(0).(bool)
 	}
 
-	var r1 string
+	var r1 *distribution.Descriptor
-	if rf, ok := ret.Get(1).(func(string, string) string); ok {
+	if rf, ok := ret.Get(1).(func(string, string) *distribution.Descriptor); ok {
 		r1 = rf(repository, reference)
 	} else {
-		r1 = ret.Get(1).(string)
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*distribution.Descriptor)
+		}
 	}
 
 	var r2 error

src/pkg/registry/client.go

@@ -72,7 +72,7 @@ type Client interface {
 	// ListTags lists the tags under the specified repository
 	ListTags(repository string) (tags []string, err error)
 	// ManifestExist checks the existence of the manifest
-	ManifestExist(repository, reference string) (exist bool, digest string, err error)
+	ManifestExist(repository, reference string) (exist bool, desc *distribution.Descriptor, err error)
 	// PullManifest pulls the specified manifest
 	PullManifest(repository, reference string, acceptedMediaTypes ...string) (manifest distribution.Manifest, digest string, err error)
 	// PushManifest pushes the specified manifest
@@ -242,10 +242,10 @@ func (c *client) listTags(url string) ([]string, string, error) {
 	return tgs.Tags, next(resp.Header.Get("Link")), nil
 }
 
-func (c *client) ManifestExist(repository, reference string) (bool, string, error) {
+func (c *client) ManifestExist(repository, reference string) (bool, *distribution.Descriptor, error) {
 	req, err := http.NewRequest(http.MethodHead, buildManifestURL(c.url, repository, reference), nil)
 	if err != nil {
-		return false, "", err
+		return false, nil, err
 	}
 	for _, mediaType := range accepts {
 		req.Header.Add(http.CanonicalHeaderKey("Accept"), mediaType)
@@ -253,12 +253,16 @@ func (c *client) ManifestExist(repository, reference string) (bool, string, erro
 	resp, err := c.do(req)
 	if err != nil {
 		if errors.IsErr(err, errors.NotFoundCode) {
-			return false, "", nil
+			return false, nil, nil
 		}
-		return false, "", err
+		return false, nil, err
 	}
 	defer resp.Body.Close()
-	return true, resp.Header.Get(http.CanonicalHeaderKey("Docker-Content-Digest")), nil
+	dig := resp.Header.Get(http.CanonicalHeaderKey("Docker-Content-Digest"))
+	contentType := resp.Header.Get(http.CanonicalHeaderKey("Content-Type"))
+	contentLen := resp.Header.Get(http.CanonicalHeaderKey("Content-Length"))
+	len, _ := strconv.Atoi(contentLen)
+	return true, &distribution.Descriptor{Digest: digest.Digest(dig), MediaType: contentType, Size: int64(len)}, nil
 }
 
 func (c *client) PullManifest(repository, reference string, acceptedMediaTypes ...string) (
@@ -310,7 +314,7 @@ func (c *client) DeleteManifest(repository, reference string) error {
 	_, err := digest.Parse(reference)
 	if err != nil {
 		// the reference is tag, get the digest first
-		exist, digest, err := c.ManifestExist(repository, reference)
+		exist, desc, err := c.ManifestExist(repository, reference)
 		if err != nil {
 			return err
 		}
@@ -318,7 +322,7 @@ func (c *client) DeleteManifest(repository, reference string) error {
 			return errors.New(nil).WithCode(errors.NotFoundCode).
 				WithMessage("%s:%s not found", repository, reference)
 		}
-		reference = digest
+		reference = string(desc.Digest)
 	}
 	req, err := http.NewRequest(http.MethodDelete, buildManifestURL(c.url, repository, reference), nil)
 	if err != nil {
@@ -450,13 +454,13 @@ func (c *client) Copy(srcRepo, srcRef, dstRepo, dstRef string, override bool) er
 	}
 
 	// check the existence of the artifact on the destination repository
-	exist, dstDgt, err := c.ManifestExist(dstRepo, dstRef)
+	exist, desc, err := c.ManifestExist(dstRepo, dstRef)
 	if err != nil {
 		return err
 	}
 	if exist {
 		// the same artifact already exists
-		if srcDgt == dstDgt {
+		if desc != nil && srcDgt == string(desc.Digest) {
 			return nil
 		}
 		// the same name artifact exists, but not allowed to override

src/replication/adapter/adapter.go

@@ -54,7 +54,7 @@ type Adapter interface {
 // ArtifactRegistry defines the capabilities that an artifact registry should have
 type ArtifactRegistry interface {
 	FetchArtifacts(filters []*model.Filter) ([]*model.Resource, error)
-	ManifestExist(repository, reference string) (exist bool, digest string, err error)
+	ManifestExist(repository, reference string) (exist bool, desc *distribution.Descriptor, err error)
 	PullManifest(repository, reference string, accepttedMediaTypes ...string) (manifest distribution.Manifest, digest string, err error)
 	PushManifest(repository, reference, mediaType string, payload []byte) (string, error)
 	DeleteManifest(repository, reference string) error // the "reference" can be "tag" or "digest", the function needs to handle both

src/replication/adapter/huawei/image_registry.go

@@ -3,11 +3,12 @@ package huawei
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/docker/distribution"
+	"github.com/goharbor/harbor/src/replication/model"
 	"io/ioutil"
 	"net/http"
+	"strconv"
 	"time"
-
-	"github.com/goharbor/harbor/src/replication/model"
 )
 
 // FetchArtifacts gets resources from Huawei SWR
@@ -54,17 +55,17 @@ func (a *adapter) FetchArtifacts(filters []*model.Filter) ([]*model.Resource, er
 }
 
 // ManifestExist check the manifest of Huawei SWR
-func (a *adapter) ManifestExist(repository, reference string) (exist bool, digest string, err error) {
+func (a *adapter) ManifestExist(repository, reference string) (exist bool, desc *distribution.Descriptor, err error) {
 	token, err := getJwtToken(a, repository)
 	if err != nil {
-		return exist, digest, err
+		return exist, nil, err
 	}
 
 	urls := fmt.Sprintf("%s/v2/%s/manifests/%s", a.registry.URL, repository, reference)
 
 	r, err := http.NewRequest("GET", urls, nil)
 	if err != nil {
-		return exist, digest, err
+		return exist, nil, err
 	}
 
 	r.Header.Add("content-type", "application/json; charset=utf-8")
@@ -72,29 +73,33 @@ func (a *adapter) ManifestExist(repository, reference string) (exist bool, diges
 
 	resp, err := a.oriClient.Do(r)
 	if err != nil {
-		return exist, digest, err
+		return exist, nil, err
 	}
 
 	defer resp.Body.Close()
 	code := resp.StatusCode
 	if code >= 300 || code < 200 {
 		if code == 404 {
-			return false, digest, nil
+			return false, nil, nil
 		}
 		body, _ := ioutil.ReadAll(resp.Body)
-		return exist, digest, fmt.Errorf("[%d][%s]", code, string(body))
+		return exist, nil, fmt.Errorf("[%d][%s]", code, string(body))
 	}
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return exist, digest, err
+		return exist, nil, err
 	}
 	exist = true
 	manifest := hwManifest{}
 	err = json.Unmarshal(body, &manifest)
 	if err != nil {
-		return exist, digest, err
+		return exist, nil, err
 	}
-	return exist, manifest.Config.Digest, nil
+	contentType := resp.Header.Get(http.CanonicalHeaderKey("Content-Type"))
+	contentLen := resp.Header.Get(http.CanonicalHeaderKey("Content-Length"))
+	len, _ := strconv.Atoi(contentLen)
+
+	return exist, &distribution.Descriptor{MediaType: contentType, Size: int64(len)}, nil
 }
 
 // DeleteManifest delete the manifest of Huawei SWR

src/replication/transfer/image/transfer.go

@@ -328,13 +328,17 @@ func (t *transfer) pullManifest(repository, reference string) (
 }
 
 func (t *transfer) exist(repository, tag string) (bool, string, error) {
-	exist, digest, err := t.dst.ManifestExist(repository, tag)
+	exist, desc, err := t.dst.ManifestExist(repository, tag)
 	if err != nil {
 		t.logger.Errorf("failed to check the existence of the manifest of artifact %s:%s on the destination registry: %v",
 			repository, tag, err)
 		return false, "", err
 	}
-	return exist, digest, nil
+	var dig string
+	if desc != nil {
+		dig = string(desc.Digest)
+	}
+	return exist, dig, nil
 }
 
 func (t *transfer) pushManifest(manifest distribution.Manifest, repository, tag string) error {

src/server/middleware/repoproxy/proxy.go

@@ -227,14 +227,16 @@ func DisableBlobAndManifestUploadMiddleware() func(http.Handler) http.Handler {
 }
 
 func proxyManifestHead(ctx context.Context, w http.ResponseWriter, ctl proxy.Controller, p *models.Project, art lib.ArtifactInfo, remote proxy.RemoteInterface) error {
-	exist, dig, err := ctl.HeadManifest(ctx, art, remote)
+	exist, desc, err := ctl.HeadManifest(ctx, art, remote)
 	if err != nil {
 		return err
 	}
-	if !exist {
+	if !exist || desc == nil {
 		return errors.NotFoundError(fmt.Errorf("The tag %v:%v is not found", art.Repository, art.Tag))
 	}
-	w.Header().Set(dockerContentDigest, dig)
+	w.Header().Set(contentType, desc.MediaType)
-	w.Header().Set(etag, dig)
+	w.Header().Set(contentLength, fmt.Sprintf("%v", desc.Size))
+	w.Header().Set(dockerContentDigest, string(desc.Digest))
+	w.Header().Set(etag, string(desc.Digest))
 	return nil
 }

src/testing/controller/controller.go

@@ -23,3 +23,4 @@ package controller
 //go:generate mockery --case snake --dir ../../controller/scanner --name Controller --output ./scanner --outpkg scanner
 //go:generate mockery --case snake --dir ../../controller/replication --name Controller --output ./replication --outpkg replication
 //go:generate mockery --case snake --dir ../../controller/robot --name Controller --output ./robot --outpkg robot
+//go:generate mockery --case snake --dir ../../controller/proxy --name RemoteInterface --output ./proxy --outpkg proxy

src/testing/controller/proxy/remote_interface.go

@@ -0,0 +1,106 @@
+// Code generated by mockery v2.1.0. DO NOT EDIT.
+
+package proxy
+
+import (
+	io "io"
+
+	distribution "github.com/docker/distribution"
+
+	mock "github.com/stretchr/testify/mock"
+)
+
+// RemoteInterface is an autogenerated mock type for the RemoteInterface type
+type RemoteInterface struct {
+	mock.Mock
+}
+
+// BlobReader provides a mock function with given fields: repo, dig
+func (_m *RemoteInterface) BlobReader(repo string, dig string) (int64, io.ReadCloser, error) {
+	ret := _m.Called(repo, dig)
+
+	var r0 int64
+	if rf, ok := ret.Get(0).(func(string, string) int64); ok {
+		r0 = rf(repo, dig)
+	} else {
+		r0 = ret.Get(0).(int64)
+	}
+
+	var r1 io.ReadCloser
+	if rf, ok := ret.Get(1).(func(string, string) io.ReadCloser); ok {
+		r1 = rf(repo, dig)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(io.ReadCloser)
+		}
+	}
+
+	var r2 error
+	if rf, ok := ret.Get(2).(func(string, string) error); ok {
+		r2 = rf(repo, dig)
+	} else {
+		r2 = ret.Error(2)
+	}
+
+	return r0, r1, r2
+}
+
+// Manifest provides a mock function with given fields: repo, ref
+func (_m *RemoteInterface) Manifest(repo string, ref string) (distribution.Manifest, string, error) {
+	ret := _m.Called(repo, ref)
+
+	var r0 distribution.Manifest
+	if rf, ok := ret.Get(0).(func(string, string) distribution.Manifest); ok {
+		r0 = rf(repo, ref)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(distribution.Manifest)
+		}
+	}
+
+	var r1 string
+	if rf, ok := ret.Get(1).(func(string, string) string); ok {
+		r1 = rf(repo, ref)
+	} else {
+		r1 = ret.Get(1).(string)
+	}
+
+	var r2 error
+	if rf, ok := ret.Get(2).(func(string, string) error); ok {
+		r2 = rf(repo, ref)
+	} else {
+		r2 = ret.Error(2)
+	}
+
+	return r0, r1, r2
+}
+
+// ManifestExist provides a mock function with given fields: repo, ref
+func (_m *RemoteInterface) ManifestExist(repo string, ref string) (bool, *distribution.Descriptor, error) {
+	ret := _m.Called(repo, ref)
+
+	var r0 bool
+	if rf, ok := ret.Get(0).(func(string, string) bool); ok {
+		r0 = rf(repo, ref)
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	var r1 *distribution.Descriptor
+	if rf, ok := ret.Get(1).(func(string, string) *distribution.Descriptor); ok {
+		r1 = rf(repo, ref)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*distribution.Descriptor)
+		}
+	}
+
+	var r2 error
+	if rf, ok := ret.Get(2).(func(string, string) error); ok {
+		r2 = rf(repo, ref)
+	} else {
+		r2 = ret.Error(2)
+	}
+
+	return r0, r1, r2
+}

src/testing/pkg/registry/client.go

@@ -53,9 +53,13 @@ func (f *FakeClient) ListTags(repository string) ([]string, error) {
 }
 
 // ManifestExist ...
-func (f *FakeClient) ManifestExist(repository, reference string) (bool, string, error) {
+func (f *FakeClient) ManifestExist(repository, reference string) (bool, *distribution.Descriptor, error) {
 	args := f.Called()
-	return args.Bool(0), args.String(1), args.Error(2)
+	var desc *distribution.Descriptor
+	if args[0] != nil {
+		desc = args[0].(*distribution.Descriptor)
+	}
+	return args.Bool(0), desc, args.Error(2)
 }
 
 // PullManifest ...