From 4dac036013b203dd40a2b8433504127bdf74a099 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Fri, 26 Jul 2019 13:46:07 +0800 Subject: [PATCH] Fix #8319, got error when replicating image with Azure container registry Fix #8319, got error when replicating image with Azure container registry Signed-off-by: Wenkai Yin --- src/common/models/token.go | 16 ++- .../utils/registry/auth/tokenauthorizer.go | 8 +- src/common/utils/registry/auth/util.go | 2 +- src/replication/adapter/azurecr/adapter.go | 62 +--------- .../adapter/azurecr/adapter_test.go | 115 +----------------- src/replication/adapter/dockerhub/adapter.go | 21 +--- .../adapter/dockerhub/adapter_test.go | 4 +- src/replication/adapter/googlegcr/adapter.go | 21 +--- 8 files changed, 29 insertions(+), 220 deletions(-) diff --git a/src/common/models/token.go b/src/common/models/token.go index f5bbd797b..ac50fba42 100644 --- a/src/common/models/token.go +++ b/src/common/models/token.go @@ -16,9 +16,19 @@ package models // Token represents the json returned by registry token service type Token struct { - Token string `json:"token"` - ExpiresIn int `json:"expires_in"` - IssuedAt string `json:"issued_at"` + 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 ... diff --git a/src/common/utils/registry/auth/tokenauthorizer.go b/src/common/utils/registry/auth/tokenauthorizer.go index cac7c1c4a..1f1c95569 100644 --- a/src/common/utils/registry/auth/tokenauthorizer.go +++ b/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 } diff --git a/src/common/utils/registry/auth/util.go b/src/common/utils/registry/auth/util.go index ad86229d8..c3e8e217e 100644 --- a/src/common/utils/registry/auth/util.go +++ b/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{ diff --git a/src/replication/adapter/azurecr/adapter.go b/src/replication/adapter/azurecr/adapter.go index 94c0ae46b..5dc89d56b 100644 --- a/src/replication/adapter/azurecr/adapter.go +++ b/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 || - 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) + dockerRegistryAdapter, err := native.NewAdapter(registry) if err != nil { return nil, err } - return &adapter{ - registry: registry, - Adapter: dockerRegistryAdapter, + 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 -} diff --git a/src/replication/adapter/azurecr/adapter_test.go b/src/replication/adapter/azurecr/adapter_test.go index 08c8c3f8b..6c1bfb365 100644 --- a/src/replication/adapter/azurecr/adapter_test.go +++ b/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) - defer s.Close() + a := &adapter{} 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) -} diff --git a/src/replication/adapter/dockerhub/adapter.go b/src/replication/adapter/dockerhub/adapter.go index 86e4d96a2..908b62843 100644 --- a/src/replication/adapter/dockerhub/adapter.go +++ b/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 - // 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) + dockerRegistryAdapter, err := native.NewAdapter(registry) if err != nil { return nil, err } diff --git a/src/replication/adapter/dockerhub/adapter_test.go b/src/replication/adapter/dockerhub/adapter_test.go index 7d73da49d..9cf9ed650 100644 --- a/src/replication/adapter/dockerhub/adapter_test.go +++ b/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, diff --git a/src/replication/adapter/googlegcr/adapter.go b/src/replication/adapter/googlegcr/adapter.go index 4e7f40357..4dd47d834 100644 --- a/src/replication/adapter/googlegcr/adapter.go +++ b/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 - 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) + dockerRegistryAdapter, err := native.NewAdapter(registry) 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 -}