Update replication

1. Refine the health check of docker hub
2. Remove the GetNamespace method from adapter interface

Signed-off-by: Wenkai Yin <yinw@vmware.com>

src/common/utils/registry/registry.go

@@ -165,15 +165,13 @@ func (r *Registry) PingSimple() error {
 	if err == nil {
 		return nil
 	}
-
 	httpErr, ok := err.(*commonhttp.Error)
 	if !ok {
 		return err
 	}
-
+	if httpErr.Code == http.StatusUnauthorized ||
-	if httpErr.Code < 500 {
+		httpErr.Code == http.StatusForbidden {
 		return nil
 	}
-
 	return httpErr
 }

src/core/api/registry_test.go

@@ -15,13 +15,13 @@ import (
 var (
 	testRegistry = &model.Registry{
 		Name:       "test1",
-		URL:        "https://goharbor.io",
+		URL:        "https://registry-1.docker.io",
 		Type:       "harbor",
 		Credential: nil,
 	}
 	testRegistry2 = &model.Registry{
 		Name:       "test2",
-		URL:        "https://goharbor.io",
+		URL:        "https://registry-1.docker.io",
 		Type:       "harbor",
 		Credential: nil,
 	}

src/replication/adapter/adapter.go

@@ -39,10 +39,6 @@ type Adapter interface {
 	// PrepareForPush does the prepare work that needed for pushing/uploading the resource
 	// eg: create the namespace or repository
 	PrepareForPush(*model.Resource) error
-	// Get the namespace specified by the name, the returning value should
-	// contain the metadata about the namespace if it has
-	// TODO remove this method?
-	GetNamespace(string) (*model.Namespace, error)
 	// HealthCheck checks health status of registry
 	HealthCheck() (model.HealthStatus, error)
 }

src/replication/adapter/dockerhub/adapter.go

@@ -21,16 +21,20 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
+		reg, err := adp.NewDefaultImageRegistry(&model.Registry{
+			Name:       registry.Name,
+			URL:        registryURL,
+			Credential: registry.Credential,
+			Insecure:   registry.Insecure,
+		})
+		if err != nil {
+			return nil, err
+		}
 
 		return &adapter{
-			client:   client,
+			client:               client,
-			registry: registry,
+			registry:             registry,
-			DefaultImageRegistry: adp.NewDefaultImageRegistry(&model.Registry{
+			DefaultImageRegistry: reg,
-				Name:       registry.Name,
-				URL:        registryURL,
-				Credential: registry.Credential,
-				Insecure:   registry.Insecure,
-			}),
 		}, nil
 	}); err != nil {
 		log.Errorf("Register adapter factory for %s error: %v", model.RegistryTypeDockerHub, err)
@@ -184,13 +188,6 @@ func (a *adapter) CreateNamespace(namespace *model.Namespace) error {
 	return nil
 }
 
-// GetNamespace gets a namespace from DockerHub.
-func (a *adapter) GetNamespace(namespace string) (*model.Namespace, error) {
-	return &model.Namespace{
-		Name: namespace,
-	}, nil
-}
-
 // getNamespace get namespace from DockerHub, if the namespace not found, two nil would be returned.
 func (a *adapter) getNamespace(namespace string) (*model.Namespace, error) {
 	resp, err := a.client.Do(http.MethodGet, getNamespacePath(namespace), nil)

src/replication/adapter/harbor/adapter.go

@@ -32,7 +32,7 @@ import (
 
 func init() {
 	if err := adp.RegisterFactory(model.RegistryTypeHarbor, func(registry *model.Registry) (adp.Adapter, error) {
-		return newAdapter(registry), nil
+		return newAdapter(registry)
 	}); err != nil {
 		log.Errorf("failed to register factory for %s: %v", model.RegistryTypeHarbor, err)
 		return
@@ -47,7 +47,7 @@ type adapter struct {
 	client         *common_http.Client
 }
 
-func newAdapter(registry *model.Registry) *adapter {
+func newAdapter(registry *model.Registry) (*adapter, error) {
 	transport := util.GetHTTPTransport(registry.Insecure)
 	modifiers := []modifier.Modifier{
 		&auth.UserAgentModifier{
@@ -74,6 +74,10 @@ func newAdapter(registry *model.Registry) *adapter {
 		url = registry.CoreURL
 	}
 
+	reg, err := adp.NewDefaultImageRegistry(registry)
+	if err != nil {
+		return nil, err
+	}
 	return &adapter{
 		registry:       registry,
 		coreServiceURL: url,
@@ -81,8 +85,8 @@ func newAdapter(registry *model.Registry) *adapter {
 			&http.Client{
 				Transport: transport,
 			}, modifiers...),
-		DefaultImageRegistry: adp.NewDefaultImageRegistry(registry),
+		DefaultImageRegistry: reg,
-	}
+	}, nil
 }
 
 func (a *adapter) Info() (*model.RegistryInfo, error) {
@@ -233,18 +237,6 @@ func (a *adapter) PrepareForPush(resource *model.Resource) error {
 	return err
 }
 
-// TODO remove this method
-func (a *adapter) GetNamespace(namespace string) (*model.Namespace, error) {
-	project, err := a.getProject(namespace)
-	if err != nil {
-		return nil, err
-	}
-	return &model.Namespace{
-		Name:     namespace,
-		Metadata: project.Metadata,
-	}, nil
-}
-
 type project struct {
 	ID       int64                  `json:"project_id"`
 	Name     string                 `json:"name"`

src/replication/adapter/harbor/adapter_test.go

@@ -37,7 +37,8 @@ func TestInfo(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
+	require.Nil(t, err)
 	info, err := adapter.Info()
 	require.Nil(t, err)
 	assert.Equal(t, model.RegistryTypeHarbor, info.Type)
@@ -60,7 +61,8 @@ func TestInfo(t *testing.T) {
 	registry = &model.Registry{
 		URL: server.URL,
 	}
-	adapter = newAdapter(registry)
+	adapter, err = newAdapter(registry)
+	require.Nil(t, err)
 	info, err = adapter.Info()
 	require.Nil(t, err)
 	assert.Equal(t, model.RegistryTypeHarbor, info.Type)
@@ -91,7 +93,8 @@ func TestListNamespaces(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
+	require.Nil(t, err)
 	npQuery := &model.NamespaceQuery{
 		Name: "lib",
 	}
@@ -113,9 +116,10 @@ func TestPrepareForPush(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
+	require.Nil(t, err)
 	// nil resource
-	err := adapter.PrepareForPush(nil)
+	err = adapter.PrepareForPush(nil)
 	require.NotNil(t, err)
 	// nil metadata
 	err = adapter.PrepareForPush(&model.Resource{})
@@ -162,7 +166,8 @@ func TestPrepareForPush(t *testing.T) {
 	registry = &model.Registry{
 		URL: server.URL,
 	}
-	adapter = newAdapter(registry)
+	adapter, err = newAdapter(registry)
+	require.Nil(t, err)
 	err = adapter.PrepareForPush(&model.Resource{
 		Metadata: &model.ResourceMetadata{
 			Namespace: &model.Namespace{
@@ -172,32 +177,3 @@ func TestPrepareForPush(t *testing.T) {
 	})
 	require.Nil(t, err)
 }
-
-func TestGetNamespace(t *testing.T) {
-	// project exists
-	server := test.NewServer(&test.RequestHandlerMapping{
-		Method:  http.MethodGet,
-		Pattern: "/api/projects",
-		Handler: func(w http.ResponseWriter, r *http.Request) {
-			data := `[{
-				"name": "library",
-				"metadata": {"public":true}
-
-			}]`
-			w.Write([]byte(data))
-		},
-	})
-	defer server.Close()
-	registry := &model.Registry{
-		URL: server.URL,
-	}
-	adapter := newAdapter(registry)
-	namespace, err := adapter.GetNamespace("library")
-	require.Nil(t, err)
-	assert.Equal(t, "library", namespace.Name)
-	assert.True(t, namespace.Metadata["public"].(bool))
-
-	// project doesn't exists
-	namespace, err = adapter.GetNamespace("test")
-	require.NotNil(t, err)
-}

src/replication/adapter/harbor/chart_registry_test.go

@@ -56,7 +56,8 @@ func TestFetchCharts(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
+	require.Nil(t, err)
 	resources, err := adapter.FetchCharts([]string{"library"}, nil)
 	require.Nil(t, err)
 	assert.Equal(t, 2, len(resources))
@@ -84,7 +85,8 @@ func TestChartExist(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
+	require.Nil(t, err)
 	exist, err := adapter.ChartExist("library/harbor", "1.0")
 	require.Nil(t, err)
 	require.True(t, exist)
@@ -116,8 +118,9 @@ func TestDownloadChart(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
-	_, err := adapter.DownloadChart("library/harbor", "1.0")
+	require.Nil(t, err)
+	_, err = adapter.DownloadChart("library/harbor", "1.0")
 	require.Nil(t, err)
 }
 
@@ -133,8 +136,9 @@ func TestUploadChart(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
-	err := adapter.UploadChart("library/harbor", "1.0", bytes.NewBuffer(nil))
+	require.Nil(t, err)
+	err = adapter.UploadChart("library/harbor", "1.0", bytes.NewBuffer(nil))
 	require.Nil(t, err)
 }
 
@@ -150,7 +154,8 @@ func TestDeleteChart(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
-	err := adapter.DeleteChart("library/harbor", "1.0")
+	require.Nil(t, err)
+	err = adapter.DeleteChart("library/harbor", "1.0")
 	require.Nil(t, err)
 }

src/replication/adapter/harbor/image_registry_test.go

@@ -65,7 +65,8 @@ func TestFetchImages(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
+	require.Nil(t, err)
 	resources, err := adapter.FetchImages([]string{"library"}, nil)
 	require.Nil(t, err)
 	assert.Equal(t, 1, len(resources))
@@ -88,7 +89,8 @@ func TestDeleteManifest(t *testing.T) {
 	registry := &model.Registry{
 		URL: server.URL,
 	}
-	adapter := newAdapter(registry)
+	adapter, err := newAdapter(registry)
-	err := adapter.DeleteManifest("library/hello-world", "1.0")
+	require.Nil(t, err)
+	err = adapter.DeleteManifest("library/hello-world", "1.0")
 	require.Nil(t, err)
 }

src/replication/adapter/huawei/huawei_adapter_test.go

@@ -94,17 +94,3 @@ func TestAdapter_PrepareForPush(t *testing.T) {
 		t.Log("success prepare for push")
 	}
 }
-
-func TestAdapter_GetNamespace(t *testing.T) {
-	ns, err := hwAdapter.GetNamespace("huaweicloud_namespace_name")
-	if err != nil {
-		if strings.HasPrefix(err.Error(), "[401]") {
-			t.Log("huawei ak/sk is not available", err.Error())
-		} else {
-			t.Error(err)
-		}
-	} else {
-		t.Log(ns)
-	}
-
-}

src/replication/adapter/image_registry.go

@@ -28,7 +28,6 @@ import (
 	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"
-	util_registry "github.com/goharbor/harbor/src/common/utils/registry"
 	"github.com/goharbor/harbor/src/common/utils/registry/auth"
 	"github.com/goharbor/harbor/src/replication/model"
 	"github.com/goharbor/harbor/src/replication/util"
@@ -55,14 +54,14 @@ type ImageRegistry interface {
 // DefaultImageRegistry provides a default implementation for interface ImageRegistry
 type DefaultImageRegistry struct {
 	sync.RWMutex
+	*registry_pkg.Registry
 	registry *model.Registry
 	client   *http.Client
-	url      string
 	clients  map[string]*registry_pkg.Repository
 }
 
 // NewDefaultImageRegistry returns an instance of DefaultImageRegistry
-func NewDefaultImageRegistry(registry *model.Registry) *DefaultImageRegistry {
+func NewDefaultImageRegistry(registry *model.Registry) (*DefaultImageRegistry, error) {
 	transport := util.GetHTTPTransport(registry.Insecure)
 	modifiers := []modifier.Modifier{
 		&auth.UserAgentModifier{
@@ -93,31 +92,27 @@ func NewDefaultImageRegistry(registry *model.Registry) *DefaultImageRegistry {
 	client := &http.Client{
 		Transport: registry_pkg.NewTransport(transport, modifiers...),
 	}
+	reg, err := registry_pkg.NewRegistry(registry.URL, client)
+	if err != nil {
+		return nil, err
+	}
 	return &DefaultImageRegistry{
+		Registry: reg,
 		client:   client,
 		registry: registry,
 		clients:  map[string]*registry_pkg.Repository{},
-		url:      registry.URL,
+	}, nil
-	}
 }
 
 func (d *DefaultImageRegistry) getClient(repository string) (*registry_pkg.Repository, error) {
-	client := d.get(repository)
-	if client != nil {
-		return client, nil
-	}
-
-	return d.create(repository)
-}
-
-func (d *DefaultImageRegistry) get(repository string) *registry_pkg.Repository {
 	d.RLock()
 	defer d.RUnlock()
 	client, exist := d.clients[repository]
 	if exist {
-		return client
+		return client, nil
 	}
-	return nil
+
+	return d.create(repository)
 }
 
 func (d *DefaultImageRegistry) create(repository string) (*registry_pkg.Repository, error) {
@@ -129,7 +124,7 @@ func (d *DefaultImageRegistry) create(repository string) (*registry_pkg.Reposito
 		return client, nil
 	}
 
-	client, err := registry_pkg.NewRepository(repository, d.url, d.client)
+	client, err := registry_pkg.NewRepository(repository, d.registry.URL, d.client)
 	if err != nil {
 		return nil, err
 	}
@@ -139,47 +134,16 @@ func (d *DefaultImageRegistry) create(repository string) (*registry_pkg.Reposito
 
 // HealthCheck checks health status of a registry
 func (d *DefaultImageRegistry) HealthCheck() (model.HealthStatus, error) {
-	if d.registry.Credential == nil || (len(d.registry.Credential.AccessKey) == 0 && len(d.registry.Credential.AccessSecret) == 0) {
+	var err error
-		return d.pingAnonymously()
+	if d.registry.Credential == nil {
+		err = d.PingSimple()
+	} else {
+		err = d.Ping()
 	}
-
-	// TODO(ChenDe): Support other credential type like OAuth, for the moment, only basic auth is supported.
-	if d.registry.Credential.Type != model.CredentialTypeBasic {
-		return model.Unknown, fmt.Errorf("unknown credential type '%s', only '%s' supported yet", d.registry.Credential.Type, model.CredentialTypeBasic)
-	}
-
-	transport := util.GetHTTPTransport(d.registry.Insecure)
-	credential := auth.NewBasicAuthCredential(d.registry.Credential.AccessKey, d.registry.Credential.AccessSecret)
-	authorizer := auth.NewStandardTokenAuthorizer(&http.Client{
-		Transport: transport,
-	}, credential)
-	registry, err := util_registry.NewRegistry(d.registry.URL, &http.Client{
-		Transport: util_registry.NewTransport(transport, authorizer),
-	})
 	if err != nil {
-		return model.Unknown, err
+		log.Errorf("failed to ping registry %s: %v", d.registry.URL, err)
+		return model.Unhealthy, nil
 	}
-
-	err = registry.Ping()
-	if err != nil {
-		return model.Unhealthy, err
-	}
-	return model.Healthy, nil
-}
-
-func (d *DefaultImageRegistry) pingAnonymously() (model.HealthStatus, error) {
-	registry, err := util_registry.NewRegistry(d.registry.URL, &http.Client{
-		Transport: util_registry.NewTransport(util.GetHTTPTransport(d.registry.Insecure)),
-	})
-	if err != nil {
-		return model.Unknown, err
-	}
-
-	err = registry.PingSimple()
-	if err != nil {
-		return model.Unhealthy, err
-	}
-
 	return model.Healthy, nil
 }
 
@@ -289,13 +253,3 @@ func (d *DefaultImageRegistry) ListTag(repository string) ([]string, error) {
 
 	return client.ListTag()
 }
-
-// Catalog ...
-func (d *DefaultImageRegistry) Catalog() ([]string, error) {
-	client, err := registry_pkg.NewRegistry(d.registry.URL, d.client)
-	if err != nil {
-		return nil, err
-	}
-
-	return client.Catalog()
-}

src/replication/adapter/native/adapter.go

@@ -12,9 +12,13 @@ const registryTypeNative model.RegistryType = "native"
 
 func init() {
 	if err := adp.RegisterFactory(registryTypeNative, func(registry *model.Registry) (adp.Adapter, error) {
+		reg, err := adp.NewDefaultImageRegistry(registry)
+		if err != nil {
+			return nil, err
+		}
 		return &native{
 			registry:             registry,
-			DefaultImageRegistry: adp.NewDefaultImageRegistry(registry),
+			DefaultImageRegistry: reg,
 		}, nil
 	}); err != nil {
 		log.Errorf("failed to register factory for %s: %v", registryTypeNative, err)
@@ -79,11 +83,6 @@ func (native) ConvertResourceMetadata(metadata *model.ResourceMetadata, namespac
 // PrepareForPush nothing need to do.
 func (native) PrepareForPush(*model.Resource) error { return nil }
 
-// GetNamespace naitve registry no namespace.
-func (native) GetNamespace(name string) (*model.Namespace, error) {
-	return &model.Namespace{Name: name}, nil
-}
-
 // ListNamespaces native registry no namespaces, so list empty array.
 func (native) ListNamespaces(*model.NamespaceQuery) ([]*model.Namespace, error) {
 	return []*model.Namespace{}, nil