Merge pull request #8425 from ywk253100/190726_acr

Fix #8319, got error when replicating image with Azure container registry

src/common/models/token.go

@@ -17,10 +17,20 @@ package models
 // Token represents the json returned by registry token service
 type Token struct {
 	Token       string `json:"token"`
+	AccessToken string `json:"access_token"` // the token returned by azure container registry is called "access_token"
 	ExpiresIn   int    `json:"expires_in"`
 	IssuedAt    string `json:"issued_at"`
 }
 
+// GetToken returns the content of the token
+func (t *Token) GetToken() string {
+	token := t.Token
+	if len(token) == 0 {
+		token = t.AccessToken
+	}
+	return token
+}
+
 // ResourceActions ...
 type ResourceActions struct {
 	Name    string   `json:"name"`

src/common/utils/registry/auth/tokenauthorizer.go

@@ -15,6 +15,7 @@
 package auth
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -111,7 +112,12 @@ func (t *tokenAuthorizer) Modify(req *http.Request) error {
 		}
 	}
 
-	req.Header.Add(http.CanonicalHeaderKey("Authorization"), fmt.Sprintf("Bearer %s", token.Token))
+	tk := token.GetToken()
+	if len(tk) == 0 {
+		return errors.New("empty token content")
+	}
+
+	req.Header.Add(http.CanonicalHeaderKey("Authorization"), fmt.Sprintf("Bearer %s", tk))
 
 	return nil
 }

src/common/utils/registry/auth/util.go

@@ -30,7 +30,7 @@ const (
 	service = "harbor-registry"
 )
 
-// GetToken requests a token against the endpoint using credetial provided
+// GetToken requests a token against the endpoint using credential provided
 func GetToken(endpoint string, insecure bool, credential Credential,
 	scopes []*token.ResourceActions) (*models.Token, error) {
 	client := &http.Client{

src/replication/adapter/azurecr/adapter.go

@@ -1,18 +1,10 @@
 package azurecr
 
 import (
-	"fmt"
-	"net/http"
-
-	"github.com/goharbor/harbor/src/common/http/modifier"
-	common_http_auth "github.com/goharbor/harbor/src/common/http/modifier/auth"
 	"github.com/goharbor/harbor/src/common/utils/log"
-	registry_pkg "github.com/goharbor/harbor/src/common/utils/registry"
-	"github.com/goharbor/harbor/src/common/utils/registry/auth"
 	adp "github.com/goharbor/harbor/src/replication/adapter"
 	"github.com/goharbor/harbor/src/replication/adapter/native"
 	"github.com/goharbor/harbor/src/replication/model"
-	"github.com/goharbor/harbor/src/replication/util"
 )
 
 func init() {
@@ -24,27 +16,17 @@ func init() {
 }
 
 func factory(registry *model.Registry) (adp.Adapter, error) {
-	if registry.Credential == nil || len(registry.Credential.AccessKey) == 0 ||
+	dockerRegistryAdapter, err := native.NewAdapter(registry)
-		len(registry.Credential.AccessSecret) == 0 {
-		return nil, fmt.Errorf("credential is necessary for registry %s", registry.URL)
-	}
-
-	authorizer := auth.NewBasicAuthCredential(registry.Credential.AccessKey,
-		registry.Credential.AccessSecret)
-	dockerRegistryAdapter, err := native.NewAdapterWithCustomizedAuthorizer(registry, authorizer)
 	if err != nil {
 		return nil, err
 	}
-
 	return &adapter{
-		registry: registry,
 		Adapter: dockerRegistryAdapter,
 	}, nil
 }
 
 type adapter struct {
 	*native.Adapter
-	registry *model.Registry
 }
 
 // Ensure '*adapter' implements interface 'Adapter'.
@@ -73,43 +55,3 @@ func (a *adapter) Info() (*model.RegistryInfo, error) {
 		},
 	}, nil
 }
-
-// HealthCheck checks health status of a registry
-func (a *adapter) HealthCheck() (model.HealthStatus, error) {
-	err := a.PingGet()
-	if err != nil {
-		return model.Unhealthy, nil
-	}
-
-	return model.Healthy, nil
-}
-
-func getClient(registry *model.Registry) (*http.Client, error) {
-	if registry.Credential == nil ||
-		len(registry.Credential.AccessKey) == 0 || len(registry.Credential.AccessSecret) == 0 {
-		return nil, fmt.Errorf("no credential to ping registry %s", registry.URL)
-	}
-
-	var cred modifier.Modifier
-	if registry.Credential.Type == model.CredentialTypeSecret {
-		cred = common_http_auth.NewSecretAuthorizer(registry.Credential.AccessSecret)
-	} else {
-		cred = auth.NewBasicAuthCredential(
-			registry.Credential.AccessKey,
-			registry.Credential.AccessSecret)
-	}
-
-	transport := util.GetHTTPTransport(registry.Insecure)
-	modifiers := []modifier.Modifier{
-		&auth.UserAgentModifier{
-			UserAgent: adp.UserAgentReplication,
-		},
-		cred,
-	}
-
-	client := &http.Client{
-		Transport: registry_pkg.NewTransport(transport, modifiers...),
-	}
-
-	return client, nil
-}

src/replication/adapter/azurecr/adapter_test.go

@@ -1,128 +1,17 @@
 package azurecr
 
 import (
-	"fmt"
-	"io"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
-	"github.com/goharbor/harbor/src/common/utils/test"
-	adp "github.com/goharbor/harbor/src/replication/adapter"
 	"github.com/goharbor/harbor/src/replication/model"
+	"github.com/stretchr/testify/assert"
 )
 
-func getMockAdapter(t *testing.T, hasCred, health bool) (*adapter, *httptest.Server) {
-	server := test.NewServer(
-		&test.RequestHandlerMapping{
-			Method:  http.MethodGet,
-			Pattern: "/v2/_catalog",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				w.WriteHeader(http.StatusOK)
-				w.Write([]byte(`{"repositories": ["test1"]}`))
-			},
-		},
-		&test.RequestHandlerMapping{
-			Method:  http.MethodGet,
-			Pattern: "/v2/{repo}/tags/list",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				w.WriteHeader(http.StatusOK)
-				w.Write([]byte(`{"name": "test1", "tags": ["latest"]}`))
-			},
-		},
-		&test.RequestHandlerMapping{
-			Method:  http.MethodGet,
-			Pattern: "/v2/",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				fmt.Println(r.Method, r.URL)
-				if health {
-					w.WriteHeader(http.StatusOK)
-				} else {
-					w.WriteHeader(http.StatusBadRequest)
-				}
-			},
-		},
-		&test.RequestHandlerMapping{
-			Method:  http.MethodGet,
-			Pattern: "/",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				fmt.Println(r.Method, r.URL)
-				w.WriteHeader(http.StatusOK)
-			},
-		},
-		&test.RequestHandlerMapping{
-			Method:  http.MethodPost,
-			Pattern: "/",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				fmt.Println(r.Method, r.URL)
-				if buf, e := ioutil.ReadAll(&io.LimitedReader{R: r.Body, N: 80}); e == nil {
-					fmt.Println("\t", string(buf))
-				}
-				w.WriteHeader(http.StatusOK)
-			},
-		},
-	)
-	registry := &model.Registry{
-		Type: model.RegistryTypeAzureAcr,
-		URL:  server.URL,
-	}
-	if hasCred {
-		registry.Credential = &model.Credential{
-			AccessKey:    "acr",
-			AccessSecret: "pwd",
-		}
-	}
-
-	factory, err := adp.GetFactory(model.RegistryTypeAzureAcr)
-	assert.Nil(t, err)
-	assert.NotNil(t, factory)
-	a, err := factory(registry)
-
-	assert.Nil(t, err)
-	return a.(*adapter), server
-}
-
 func TestInfo(t *testing.T) {
-	a, s := getMockAdapter(t, true, true)
+	a := &adapter{}
-	defer s.Close()
 	info, err := a.Info()
 	assert.Nil(t, err)
 	assert.NotNil(t, info)
 	assert.EqualValues(t, 1, len(info.SupportedResourceTypes))
 	assert.EqualValues(t, model.ResourceTypeImage, info.SupportedResourceTypes[0])
 }
-
-func TestHealthCheck(t *testing.T) {
-	a, s := getMockAdapter(t, true, false)
-	defer s.Close()
-	status, err := a.HealthCheck()
-	assert.Nil(t, err)
-	assert.NotNil(t, status)
-	assert.EqualValues(t, model.Unhealthy, status)
-	a, s = getMockAdapter(t, true, true)
-	defer s.Close()
-	status, err = a.HealthCheck()
-	assert.Nil(t, err)
-	assert.NotNil(t, status)
-	assert.EqualValues(t, model.Healthy, status)
-}
-
-func TestPrepareForPush(t *testing.T) {
-	a, s := getMockAdapter(t, true, true)
-	defer s.Close()
-	resources := []*model.Resource{
-		{
-			Type: model.ResourceTypeImage,
-			Metadata: &model.ResourceMetadata{
-				Repository: &model.Repository{
-					Name: "busybox",
-				},
-			},
-		},
-	}
-	err := a.PrepareForPush(resources)
-	assert.Nil(t, err)
-}

src/replication/adapter/dockerhub/adapter.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/common/utils/registry/auth"
 	adp "github.com/goharbor/harbor/src/replication/adapter"
 	"github.com/goharbor/harbor/src/replication/adapter/native"
 	"github.com/goharbor/harbor/src/replication/model"
@@ -35,25 +34,7 @@ func factory(registry *model.Registry) (adp.Adapter, error) {
 		return nil, err
 	}
 
-	// if the registry.Credentail isn't specified, the credential here is nil
+	dockerRegistryAdapter, err := native.NewAdapter(registry)
-	// the client will request the token with no authentication
-	// this is needed for pulling images from public repositories
-	var credential auth.Credential
-	if registry.Credential != nil && len(registry.Credential.AccessSecret) != 0 {
-		credential = auth.NewBasicAuthCredential(
-			registry.Credential.AccessKey,
-			registry.Credential.AccessSecret)
-	}
-	authorizer := auth.NewStandardTokenAuthorizer(&http.Client{
-		Transport: util.GetHTTPTransport(registry.Insecure),
-	}, credential)
-
-	dockerRegistryAdapter, err := native.NewAdapterWithCustomizedAuthorizer(&model.Registry{
-		Name:       registry.Name,
-		URL:        registryURL, // specify the URL of Docker Hub registry service
-		Credential: registry.Credential,
-		Insecure:   registry.Insecure,
-	}, authorizer)
 	if err != nil {
 		return nil, err
 	}

src/replication/adapter/dockerhub/adapter_test.go

@@ -4,11 +4,10 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
 	adp "github.com/goharbor/harbor/src/replication/adapter"
 	"github.com/goharbor/harbor/src/replication/model"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -24,6 +23,7 @@ func getAdapter(t *testing.T) adp.Adapter {
 
 	adapter, err := factory(&model.Registry{
 		Type: model.RegistryTypeDockerHub,
+		URL:  baseURL,
 		Credential: &model.Credential{
 			AccessKey:    testUser,
 			AccessSecret: testPassword,

src/replication/adapter/googlegcr/adapter.go

@@ -15,14 +15,10 @@
 package googlegcr
 
 import (
-	"net/http"
-
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/common/utils/registry/auth"
 	adp "github.com/goharbor/harbor/src/replication/adapter"
 	"github.com/goharbor/harbor/src/replication/adapter/native"
 	"github.com/goharbor/harbor/src/replication/model"
-	"github.com/goharbor/harbor/src/replication/util"
 )
 
 func init() {
@@ -36,17 +32,7 @@ func init() {
 }
 
 func newAdapter(registry *model.Registry) (*adapter, error) {
-	var credential auth.Credential
+	dockerRegistryAdapter, err := native.NewAdapter(registry)
-	if registry.Credential != nil && len(registry.Credential.AccessSecret) != 0 {
-		credential = auth.NewBasicAuthCredential(
-			registry.Credential.AccessKey,
-			registry.Credential.AccessSecret)
-	}
-	authorizer := auth.NewStandardTokenAuthorizer(&http.Client{
-		Transport: util.GetHTTPTransport(registry.Insecure),
-	}, credential)
-
-	dockerRegistryAdapter, err := native.NewAdapterWithCustomizedAuthorizer(registry, authorizer)
 	if err != nil {
 		return nil, err
 	}
@@ -101,8 +87,3 @@ func (a adapter) HealthCheck() (model.HealthStatus, error) {
 	}
 	return model.Healthy, nil
 }
-
-// PrepareForPush nothing need to do.
-func (a adapter) PrepareForPush(resources []*model.Resource) error {
-	return nil
-}