Merge pull request #13803 from stonezdj/201212_revert_add_contenttype

Revert "Add content type and length in header"
This commit is contained in:
stonezdj(Daojun Zhang) 2020-12-18 13:22:30 +08:00 committed by GitHub
commit 98f3c5d452
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 74 additions and 190 deletions

View File

@ -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, *distribution.Descriptor, error)
HeadManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, string, 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, desc, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
exist, dig, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
if err != nil {
return false, nil, err
}
if !exist || desc == nil {
if !exist {
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, string(desc.Digest)), &content)
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, string(desc.Digest)))
return true, &ManifestList{content, string(desc.Digest), manifestlist.MediaTypeManifestList}, 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, string(desc.Digest)))
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 && string(desc.Digest) == a.Digest, nil, nil // digest matches
return a != nil && dig == 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, *distribution.Descriptor, error) {
func (c *controller) HeadManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, string, error) {
remoteRepo := getRemoteRepo(art)
ref := getReference(art)
return remote.ManifestExist(remoteRepo, ref)

View File

@ -23,14 +23,29 @@ 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
}
@ -80,14 +95,14 @@ func (l *localInterfaceMock) DeleteManifest(repo, ref string) {
type proxyControllerTestSuite struct {
suite.Suite
local *localInterfaceMock
remote *testproxy.RemoteInterface
remote *remoteInterfaceMock
ctr Controller
proj *models.Project
}
func (p *proxyControllerTestSuite) SetupTest() {
p.local = &localInterfaceMock{}
p.remote = &testproxy.RemoteInterface{}
p.remote = &remoteInterfaceMock{}
p.proj = &models.Project{RegistryID: 1}
p.ctr = &controller{
blobCtl: blob.Ctl,
@ -110,9 +125,8 @@ 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, desc, nil)
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)
p.Assert().Nil(err)
@ -122,9 +136,8 @@ 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, desc, nil)
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)

View File

@ -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, *distribution.Descriptor, error)
ManifestExist(repo string, ref string) (bool, string, 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, *distribution.Descriptor, error) {
func (r *remoteHelper) ManifestExist(repo string, ref string) (bool, string, error) {
return r.registry.ManifestExist(repo, ref)
}

View File

@ -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, *distribution.Descriptor, error) {
func (_m *mockAdapter) ManifestExist(repository string, reference string) (bool, string, error) {
ret := _m.Called(repository, reference)
var r0 bool
@ -144,13 +144,11 @@ func (_m *mockAdapter) ManifestExist(repository string, reference string) (bool,
r0 = ret.Get(0).(bool)
}
var r1 *distribution.Descriptor
if rf, ok := ret.Get(1).(func(string, string) *distribution.Descriptor); ok {
var r1 string
if rf, ok := ret.Get(1).(func(string, string) string); ok {
r1 = rf(repository, reference)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*distribution.Descriptor)
}
r1 = ret.Get(1).(string)
}
var r2 error

View File

@ -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, desc *distribution.Descriptor, err error)
ManifestExist(repository, reference string) (exist bool, digest string, 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, *distribution.Descriptor, error) {
func (c *client) ManifestExist(repository, reference string) (bool, string, error) {
req, err := http.NewRequest(http.MethodHead, buildManifestURL(c.url, repository, reference), nil)
if err != nil {
return false, nil, err
return false, "", err
}
for _, mediaType := range accepts {
req.Header.Add(http.CanonicalHeaderKey("Accept"), mediaType)
@ -253,16 +253,12 @@ func (c *client) ManifestExist(repository, reference string) (bool, *distributio
resp, err := c.do(req)
if err != nil {
if errors.IsErr(err, errors.NotFoundCode) {
return false, nil, nil
return false, "", nil
}
return false, nil, err
return false, "", err
}
defer resp.Body.Close()
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
return true, resp.Header.Get(http.CanonicalHeaderKey("Docker-Content-Digest")), nil
}
func (c *client) PullManifest(repository, reference string, acceptedMediaTypes ...string) (
@ -314,7 +310,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, desc, err := c.ManifestExist(repository, reference)
exist, digest, err := c.ManifestExist(repository, reference)
if err != nil {
return err
}
@ -322,7 +318,7 @@ func (c *client) DeleteManifest(repository, reference string) error {
return errors.New(nil).WithCode(errors.NotFoundCode).
WithMessage("%s:%s not found", repository, reference)
}
reference = string(desc.Digest)
reference = digest
}
req, err := http.NewRequest(http.MethodDelete, buildManifestURL(c.url, repository, reference), nil)
if err != nil {
@ -454,13 +450,13 @@ func (c *client) Copy(srcRepo, srcRef, dstRepo, dstRef string, override bool) er
}
// check the existence of the artifact on the destination repository
exist, desc, err := c.ManifestExist(dstRepo, dstRef)
exist, dstDgt, err := c.ManifestExist(dstRepo, dstRef)
if err != nil {
return err
}
if exist {
// the same artifact already exists
if desc != nil && srcDgt == string(desc.Digest) {
if srcDgt == dstDgt {
return nil
}
// the same name artifact exists, but not allowed to override

View File

@ -171,15 +171,15 @@ func (c *clientTestSuite) TestManifestExist() {
client := NewClient(server.URL, "", "", true)
// doesn't exist
exist, desc, err := client.ManifestExist("library/alpine", "latest")
exist, digest, err := client.ManifestExist("library/alpine", "latest")
c.Require().Nil(err)
c.False(exist)
// exist
exist, desc, err = client.ManifestExist("library/hello-world", "latest")
exist, digest, err = client.ManifestExist("library/hello-world", "latest")
c.Require().Nil(err)
c.True(exist)
c.Equal("digest", string(desc.Digest))
c.Equal("digest", digest)
}
func (c *clientTestSuite) TestPullManifest() {

View File

@ -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, desc *distribution.Descriptor, err error)
ManifestExist(repository, reference string) (exist bool, digest string, 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

View File

@ -3,12 +3,11 @@ 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
@ -55,17 +54,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, desc *distribution.Descriptor, err error) {
func (a *adapter) ManifestExist(repository, reference string) (exist bool, digest string, err error) {
token, err := getJwtToken(a, repository)
if err != nil {
return exist, nil, err
return exist, digest, 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, nil, err
return exist, digest, err
}
r.Header.Add("content-type", "application/json; charset=utf-8")
@ -73,33 +72,29 @@ func (a *adapter) ManifestExist(repository, reference string) (exist bool, desc
resp, err := a.oriClient.Do(r)
if err != nil {
return exist, nil, err
return exist, digest, err
}
defer resp.Body.Close()
code := resp.StatusCode
if code >= 300 || code < 200 {
if code == 404 {
return false, nil, nil
return false, digest, nil
}
body, _ := ioutil.ReadAll(resp.Body)
return exist, nil, fmt.Errorf("[%d][%s]", code, string(body))
return exist, digest, fmt.Errorf("[%d][%s]", code, string(body))
}
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return exist, nil, err
return exist, digest, err
}
exist = true
manifest := hwManifest{}
err = json.Unmarshal(body, &manifest)
if err != nil {
return exist, nil, err
return exist, digest, err
}
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
return exist, manifest.Config.Digest, nil
}
// DeleteManifest delete the manifest of Huawei SWR

View File

@ -44,7 +44,7 @@ func TestAdapter_FetchArtifacts(t *testing.T) {
}
func TestAdapter_ManifestExist(t *testing.T) {
exist, desc, err := HWAdapter.ManifestExist("", "")
exist, digest, err := HWAdapter.ManifestExist("", "")
if err != nil {
if strings.HasPrefix(err.Error(), "[401]") {
t.Log("huawei ak/sk is not available", err.Error())
@ -53,7 +53,7 @@ func TestAdapter_ManifestExist(t *testing.T) {
}
} else {
if exist {
t.Log(desc.Digest)
t.Log(digest)
}
}
}

View File

@ -328,17 +328,13 @@ func (t *transfer) pullManifest(repository, reference string) (
}
func (t *transfer) exist(repository, tag string) (bool, string, error) {
exist, desc, err := t.dst.ManifestExist(repository, tag)
exist, digest, 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
}
var dig string
if desc != nil {
dig = string(desc.Digest)
}
return exist, dig, nil
return exist, digest, nil
}
func (t *transfer) pushManifest(manifest distribution.Manifest, repository, tag string) error {

View File

@ -16,7 +16,6 @@ package image
import (
"bytes"
"github.com/opencontainers/go-digest"
"io"
"io/ioutil"
"testing"
@ -36,11 +35,11 @@ func (f *fakeRegistry) FetchArtifacts([]*model.Filter) ([]*model.Resource, error
return nil, nil
}
func (f *fakeRegistry) ManifestExist(repository, reference string) (bool, *distribution.Descriptor, error) {
func (f *fakeRegistry) ManifestExist(repository, reference string) (bool, string, error) {
if repository == "destination" && reference == "b1" {
return true, &distribution.Descriptor{Digest: digest.Digest("sha256:c6b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7")}, nil
return true, "sha256:c6b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", nil
}
return false, &distribution.Descriptor{Digest: digest.Digest("sha256:c6b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7")}, nil
return false, "sha256:c6b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", nil
}
func (f *fakeRegistry) PullManifest(repository, reference string, accepttedMediaTypes ...string) (distribution.Manifest, string, error) {
manifest := `{

View File

@ -227,16 +227,14 @@ 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, desc, err := ctl.HeadManifest(ctx, art, remote)
exist, dig, err := ctl.HeadManifest(ctx, art, remote)
if err != nil {
return err
}
if !exist || desc == nil {
if !exist {
return errors.NotFoundError(fmt.Errorf("The tag %v:%v is not found", art.Repository, art.Tag))
}
w.Header().Set(contentType, desc.MediaType)
w.Header().Set(contentLength, fmt.Sprintf("%v", desc.Size))
w.Header().Set(dockerContentDigest, string(desc.Digest))
w.Header().Set(etag, string(desc.Digest))
w.Header().Set(dockerContentDigest, dig)
w.Header().Set(etag, dig)
return nil
}

View File

@ -23,4 +23,3 @@ 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

View File

@ -1,106 +0,0 @@
// 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
}

View File

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