Revert "Add new registry type: LocalHarbor"

This reverts commit 94cacf762a.

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

src/core/api/registry.go

@@ -11,6 +11,7 @@ import (
 	"github.com/goharbor/harbor/src/core/api/models"
 	"github.com/goharbor/harbor/src/replication"
 	"github.com/goharbor/harbor/src/replication/adapter"
+	"github.com/goharbor/harbor/src/replication/event"
 	"github.com/goharbor/harbor/src/replication/model"
 	"github.com/goharbor/harbor/src/replication/policy"
 	"github.com/goharbor/harbor/src/replication/registry"
@@ -185,25 +186,12 @@ func (t *RegistryAPI) Post() {
 		t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
 		return
 	}
+
 	if reg != nil {
 		t.HandleConflict(fmt.Sprintf("name '%s' is already used", r.Name))
 		return
 	}
 
-	if r.Type == model.RegistryTypeLocalHarbor {
-		n, _, err := t.manager.List(&model.RegistryQuery{
-			Type: string(model.RegistryTypeLocalHarbor),
-		})
-		if err != nil {
-			t.HandleInternalServerError(fmt.Sprintf("failed to list registries: %v", err))
-			return
-		}
-		if n > 0 {
-			t.HandleBadRequest(fmt.Sprintf("can only add one registry whose type is %s", model.RegistryTypeLocalHarbor))
-			return
-		}
-	}
-
 	status, err := registry.CheckHealthStatus(r)
 	if err != nil {
 		t.HandleBadRequest(fmt.Sprintf("health check to registry %s failed: %v", r.URL, err))
@@ -362,19 +350,26 @@ func (t *RegistryAPI) Delete() {
 // GetInfo returns the base info and capability declarations of the registry
 func (t *RegistryAPI) GetInfo() {
 	id, err := t.GetInt64FromPath(":id")
-	if err != nil || id <= 0 {
+	// "0" is used for the ID of the local Harbor registry
+	if err != nil || id < 0 {
 		t.HandleBadRequest(fmt.Sprintf("invalid registry ID %s", t.GetString(":id")))
 		return
 	}
-	registry, err := t.manager.Get(id)
-	if err != nil {
-		t.HandleInternalServerError(fmt.Sprintf("failed to get registry %d: %v", id, err))
-		return
-	}
-	if registry == nil {
-		t.HandleNotFound(fmt.Sprintf("registry %d not found", id))
-		return
+	var registry *model.Registry
+	if id == 0 {
+		registry = event.GetLocalRegistry()
+	} else {
+		registry, err = t.manager.Get(id)
+		if err != nil {
+			t.HandleInternalServerError(fmt.Sprintf("failed to get registry %d: %v", id, err))
+			return
+		}
+		if registry == nil {
+			t.HandleNotFound(fmt.Sprintf("registry %d not found", id))
+			return
+		}
 	}
+
 	factory, err := adapter.GetFactory(registry.Type)
 	if err != nil {
 		t.HandleInternalServerError(fmt.Sprintf("failed to get the adapter factory for registry type %s: %v", registry.Type, err))

src/core/api/replication_execution_test.go

@@ -89,9 +89,6 @@ func (f *fakedPolicyManager) Get(id int64) (*model.Policy, error) {
 			SrcRegistry: &model.Registry{
 				ID: 1,
 			},
-			DestRegistry: &model.Registry{
-				ID: 2,
-			},
 		}, nil
 	}
 	if id == 2 {
@@ -202,7 +199,7 @@ func TestCreateExecution(t *testing.T) {
 			},
 			code: http.StatusNotFound,
 		},
-		// 400 the policy is disabled
+		// 400
 		{
 			request: &testingRequest{
 				method: http.MethodPost,

src/core/api/replication_policy.go

@@ -26,6 +26,8 @@ import (
 	"github.com/goharbor/harbor/src/replication/registry"
 )
 
+// TODO rename the file to "replication.go"
+
 // ReplicationPolicyAPI handles the replication policy requests
 type ReplicationPolicyAPI struct {
 	BaseController
@@ -103,30 +105,21 @@ func (r *ReplicationPolicyAPI) validateName(policy *model.Policy) bool {
 
 // make sure the registry referred exists
 func (r *ReplicationPolicyAPI) validateRegistry(policy *model.Policy) bool {
-	srcRegistry, err := replication.RegistryMgr.Get(policy.SrcRegistry.ID)
+	var registryID int64
+	if policy.SrcRegistry != nil && policy.SrcRegistry.ID > 0 {
+		registryID = policy.SrcRegistry.ID
+	} else {
+		registryID = policy.DestRegistry.ID
+	}
+	registry, err := replication.RegistryMgr.Get(registryID)
 	if err != nil {
-		r.HandleInternalServerError(fmt.Sprintf("failed to get source registry %d: %v", policy.SrcRegistry.ID, err))
+		r.HandleInternalServerError(fmt.Sprintf("failed to get registry %d: %v", registryID, err))
 		return false
 	}
-	if srcRegistry == nil {
-		r.HandleNotFound(fmt.Sprintf("source registry %d not found", policy.SrcRegistry.ID))
+	if registry == nil {
+		r.HandleNotFound(fmt.Sprintf("registry %d not found", registryID))
 		return false
 	}
-	dstRegistry, err := replication.RegistryMgr.Get(policy.DestRegistry.ID)
-	if err != nil {
-		r.HandleInternalServerError(fmt.Sprintf("failed to get destination registry %d: %v", policy.DestRegistry.ID, err))
-		return false
-	}
-	if dstRegistry == nil {
-		r.HandleNotFound(fmt.Sprintf("destination registry %d not found", policy.DestRegistry.ID))
-		return false
-	}
-	// one of the source registry or destination registry must be local Harbor
-	if srcRegistry.Type != model.RegistryTypeLocalHarbor && dstRegistry.Type != model.RegistryTypeLocalHarbor {
-		r.HandleBadRequest(fmt.Sprintf("at least one of the registries' type is %s", model.RegistryTypeLocalHarbor))
-		return false
-	}
-
 	return true
 }
 

src/core/api/replication_policy_test.go

@@ -22,6 +22,8 @@ import (
 	"github.com/goharbor/harbor/src/replication/model"
 )
 
+// TODO rename the file to "replication.go"
+
 type fakedRegistryManager struct{}
 
 func (f *fakedRegistryManager) Add(*model.Registry) (int64, error) {
@@ -33,12 +35,7 @@ func (f *fakedRegistryManager) List(...*model.RegistryQuery) (int64, []*model.Re
 func (f *fakedRegistryManager) Get(id int64) (*model.Registry, error) {
 	if id == 1 {
 		return &model.Registry{
-			Type: model.RegistryTypeLocalHarbor,
-		}, nil
-	}
-	if id == 2 {
-		return &model.Registry{
-			Type: model.RegistryTypeHarbor,
+			Type: "faked_registry",
 		}, nil
 	}
 	return nil, nil
@@ -131,14 +128,11 @@ func TestReplicationPolicyAPICreate(t *testing.T) {
 					SrcRegistry: &model.Registry{
 						ID: 1,
 					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
 				},
 			},
 			code: http.StatusBadRequest,
 		},
-		// 400 empty source registry
+		// 400 empty registry
 		{
 			request: &testingRequest{
 				method:     http.MethodPost,
@@ -146,29 +140,10 @@ func TestReplicationPolicyAPICreate(t *testing.T) {
 				credential: sysAdmin,
 				bodyJSON: &model.Policy{
 					Name: "policy01",
-					DestRegistry: &model.Registry{
-						ID: 1,
-					},
 				},
 			},
 			code: http.StatusBadRequest,
 		},
-		// 400 empty destination registry
-		{
-			request: &testingRequest{
-				method:     http.MethodPost,
-				url:        "/api/replication/policies",
-				credential: sysAdmin,
-				bodyJSON: &model.Policy{
-					Name: "policy01",
-					SrcRegistry: &model.Registry{
-						ID: 1,
-					},
-				},
-			},
-			code: http.StatusBadRequest,
-		},
-
 		// 409, duplicate policy name
 		{
 			request: &testingRequest{
@@ -180,9 +155,6 @@ func TestReplicationPolicyAPICreate(t *testing.T) {
 					SrcRegistry: &model.Registry{
 						ID: 1,
 					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
 				},
 			},
 			code: http.StatusConflict,
@@ -196,33 +168,12 @@ func TestReplicationPolicyAPICreate(t *testing.T) {
 				bodyJSON: &model.Policy{
 					Name: "policy01",
 					SrcRegistry: &model.Registry{
-						ID: 1,
-					},
-					DestRegistry: &model.Registry{
-						ID: 3,
+						ID: 2,
 					},
 				},
 			},
 			code: http.StatusNotFound,
 		},
-		// 400 both registry types are not local harbor
-		{
-			request: &testingRequest{
-				method:     http.MethodPost,
-				url:        "/api/replication/policies",
-				credential: sysAdmin,
-				bodyJSON: &model.Policy{
-					Name: "policy01",
-					SrcRegistry: &model.Registry{
-						ID: 2,
-					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
-				},
-			},
-			code: http.StatusBadRequest,
-		},
 		// 201
 		{
 			request: &testingRequest{
@@ -234,9 +185,6 @@ func TestReplicationPolicyAPICreate(t *testing.T) {
 					SrcRegistry: &model.Registry{
 						ID: 1,
 					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
 				},
 			},
 			code: http.StatusCreated,
@@ -343,9 +291,6 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) {
 					SrcRegistry: &model.Registry{
 						ID: 1,
 					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
 				},
 			},
 			code: http.StatusBadRequest,
@@ -361,9 +306,6 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) {
 					SrcRegistry: &model.Registry{
 						ID: 1,
 					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
 				},
 			},
 			code: http.StatusConflict,
@@ -377,33 +319,12 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) {
 				bodyJSON: &model.Policy{
 					Name: "policy01",
 					SrcRegistry: &model.Registry{
-						ID: 3,
-					},
-					DestRegistry: &model.Registry{
 						ID: 2,
 					},
 				},
 			},
 			code: http.StatusNotFound,
 		},
-		// 400 both registry types are not local harbor
-		{
-			request: &testingRequest{
-				method:     http.MethodPut,
-				url:        "/api/replication/policies/1",
-				credential: sysAdmin,
-				bodyJSON: &model.Policy{
-					Name: "policy01",
-					SrcRegistry: &model.Registry{
-						ID: 2,
-					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
-				},
-			},
-			code: http.StatusBadRequest,
-		},
 		// 200
 		{
 			request: &testingRequest{
@@ -415,9 +336,6 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) {
 					SrcRegistry: &model.Registry{
 						ID: 1,
 					},
-					DestRegistry: &model.Registry{
-						ID: 2,
-					},
 				},
 			},
 			code: http.StatusOK,

src/replication/adapter/harbor/adapter.go

@@ -37,14 +37,6 @@ func init() {
 		return
 	}
 	log.Infof("the factory for adapter %s registered", model.RegistryTypeHarbor)
-
-	if err := adp.RegisterFactory(model.RegistryTypeLocalHarbor, func(registry *model.Registry) (adp.Adapter, error) {
-		return newAdapter(registry)
-	}); err != nil {
-		log.Errorf("failed to register factory for %s: %v", model.RegistryTypeLocalHarbor, err)
-		return
-	}
-	log.Infof("the factory for adapter %s registered", model.RegistryTypeLocalHarbor)
 }
 
 type adapter struct {

src/replication/dao/registry.go

@@ -10,7 +10,6 @@ import (
 type ListRegistryQuery struct {
 	// Query is name query
 	Query string
-	Type  string
 	// Offset specifies the offset in the registry list to return
 	Offset int64
 	// Limit specifies the maximum registries to return
@@ -62,13 +61,8 @@ func ListRegistries(query ...*ListRegistryQuery) (int64, []*models.Registry, err
 	o := dao.GetOrmer()
 
 	q := o.QueryTable(&models.Registry{})
-	if len(query) > 0 && query[0] != nil {
-		if len(query[0].Query) > 0 {
-			q = q.Filter("name__contains", query[0].Query)
-		}
-		if len(query[0].Type) > 0 {
-			q = q.Filter("type", query[0].Type)
-		}
+	if len(query) > 0 && len(query[0].Query) > 0 {
+		q = q.Filter("name__contains", query[0].Query)
 	}
 
 	total, err := q.Count()

src/replication/event/handler.go

@@ -21,6 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/replication/util"
 
 	"github.com/goharbor/harbor/src/common/utils/log"
+	"github.com/goharbor/harbor/src/replication/config"
 	"github.com/goharbor/harbor/src/replication/model"
 	"github.com/goharbor/harbor/src/replication/operation"
 	"github.com/goharbor/harbor/src/replication/policy"
@@ -159,8 +160,8 @@ func PopulateRegistries(registryMgr registry.Manager, policy *model.Policy) erro
 }
 
 func getRegistry(registryMgr registry.Manager, registry *model.Registry) (*model.Registry, error) {
-	if registry == nil {
-		return nil, errors.New("empty registry")
+	if registry == nil || registry.ID == 0 {
+		return GetLocalRegistry(), nil
 	}
 	reg, err := registryMgr.Get(registry.ID)
 	if err != nil {
@@ -171,3 +172,20 @@ func getRegistry(registryMgr registry.Manager, registry *model.Registry) (*model
 	}
 	return reg, nil
 }
+
+// GetLocalRegistry returns the info of the local Harbor registry
+func GetLocalRegistry() *model.Registry {
+	return &model.Registry{
+		Type:    model.RegistryTypeHarbor,
+		Name:    "Local",
+		URL:     config.Config.RegistryURL,
+		CoreURL: config.Config.CoreURL,
+		Status:  "healthy",
+		Credential: &model.Credential{
+			Type: model.CredentialTypeSecret,
+			// use secret to do the auth for the local Harbor
+			AccessSecret: config.Config.JobserviceSecret,
+		},
+		Insecure: true,
+	}
+}

src/replication/event/handler_test.go

@@ -62,12 +62,6 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol
 			ID:       1,
 			Enabled:  true,
 			Deletion: true,
-			SrcRegistry: &model.Registry{
-				ID: 1,
-			},
-			DestRegistry: &model.Registry{
-				ID: 1,
-			},
 			Trigger: &model.Trigger{
 				Type: model.TriggerTypeEventBased,
 			},
@@ -83,13 +77,7 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol
 			ID:       2,
 			Enabled:  true,
 			Deletion: true,
-			SrcRegistry: &model.Registry{
-				ID: 1,
-			},
-			DestRegistry: &model.Registry{
-				ID: 1,
-			},
-			Trigger: nil,
+			Trigger:  nil,
 			Filters: []*model.Filter{
 				{
 					Type:  model.FilterTypeName,
@@ -102,12 +90,6 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol
 			ID:       3,
 			Enabled:  true,
 			Deletion: false,
-			SrcRegistry: &model.Registry{
-				ID: 1,
-			},
-			DestRegistry: &model.Registry{
-				ID: 1,
-			},
 			Trigger: &model.Trigger{
 				Type: model.TriggerTypeEventBased,
 			},
@@ -123,12 +105,6 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol
 			ID:       4,
 			Enabled:  true,
 			Deletion: true,
-			SrcRegistry: &model.Registry{
-				ID: 1,
-			},
-			DestRegistry: &model.Registry{
-				ID: 1,
-			},
 			Trigger: &model.Trigger{
 				Type: model.TriggerTypeEventBased,
 			},
@@ -144,12 +120,6 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol
 			ID:       5,
 			Enabled:  false,
 			Deletion: true,
-			SrcRegistry: &model.Registry{
-				ID: 1,
-			},
-			DestRegistry: &model.Registry{
-				ID: 1,
-			},
 			Trigger: &model.Trigger{
 				Type: model.TriggerTypeEventBased,
 			},

src/replication/model/policy.go

@@ -72,12 +72,20 @@ func (p *Policy) Valid(v *validation.Validation) {
 	if len(p.Name) == 0 {
 		v.SetError("name", "cannot be empty")
 	}
-	if p.SrcRegistry == nil || p.SrcRegistry.ID == 0 {
-		v.SetError("src_registry", "cannot be empty")
+	var srcRegistryID, dstRegistryID int64
+	if p.SrcRegistry != nil {
+		srcRegistryID = p.SrcRegistry.ID
 	}
-	if p.DestRegistry == nil || p.DestRegistry.ID == 0 {
-		v.SetError("dest_registry", "cannot be empty")
+	if p.DestRegistry != nil {
+		dstRegistryID = p.DestRegistry.ID
 	}
+
+	// one of the source registry and destination registry must be Harbor itself
+	if srcRegistryID != 0 && dstRegistryID != 0 ||
+		srcRegistryID == 0 && dstRegistryID == 0 {
+		v.SetError("src_registry, dest_registry", "one of them should be empty and the other one shouldn't be empty")
+	}
+
 	// valid the filters
 	for _, filter := range p.Filters {
 		if filter.Type != FilterTypeResource &&
@@ -88,6 +96,7 @@ func (p *Policy) Valid(v *validation.Validation) {
 			break
 		}
 	}
+
 	// valid trigger
 	if p.Trigger != nil {
 		if p.Trigger.Type != TriggerTypeManual &&

src/replication/model/policy_test.go

@@ -31,20 +31,23 @@ func TestValidOfPolicy(t *testing.T) {
 			policy: &Policy{},
 			pass:   false,
 		},
-		// empty source registry
+		// empty source registry and destination registry
 		{
 			policy: &Policy{
 				Name: "policy01",
 			},
 			pass: false,
 		},
-		// empty destination registry
+		// source registry and destination registry both not empty
 		{
 			policy: &Policy{
 				Name: "policy01",
 				SrcRegistry: &Registry{
 					ID: 1,
 				},
+				DestRegistry: &Registry{
+					ID: 2,
+				},
 			},
 			pass: false,
 		},
@@ -53,10 +56,10 @@ func TestValidOfPolicy(t *testing.T) {
 			policy: &Policy{
 				Name: "policy01",
 				SrcRegistry: &Registry{
-					ID: 1,
+					ID: 0,
 				},
 				DestRegistry: &Registry{
-					ID: 2,
+					ID: 1,
 				},
 				Filters: []*Filter{
 					{
@@ -71,10 +74,10 @@ func TestValidOfPolicy(t *testing.T) {
 			policy: &Policy{
 				Name: "policy01",
 				SrcRegistry: &Registry{
-					ID: 1,
+					ID: 0,
 				},
 				DestRegistry: &Registry{
-					ID: 2,
+					ID: 1,
 				},
 				Filters: []*Filter{
 					{
@@ -93,10 +96,10 @@ func TestValidOfPolicy(t *testing.T) {
 			policy: &Policy{
 				Name: "policy01",
 				SrcRegistry: &Registry{
-					ID: 1,
+					ID: 0,
 				},
 				DestRegistry: &Registry{
-					ID: 2,
+					ID: 1,
 				},
 				Filters: []*Filter{
 					{
@@ -115,10 +118,10 @@ func TestValidOfPolicy(t *testing.T) {
 			policy: &Policy{
 				Name: "policy01",
 				SrcRegistry: &Registry{
-					ID: 1,
+					ID: 0,
 				},
 				DestRegistry: &Registry{
-					ID: 2,
+					ID: 1,
 				},
 				Filters: []*Filter{
 					{

src/replication/model/registry.go

@@ -23,10 +23,8 @@ import (
 // const definition
 const (
 	// RegistryTypeHarbor indicates registry type harbor
-	RegistryTypeHarbor RegistryType = "harbor"
-	// Local Harbor is the type of Harbor instance where the replication service is running on
-	RegistryTypeLocalHarbor RegistryType = "localHarbor"
-	RegistryTypeDockerHub   RegistryType = "dockerHub"
+	RegistryTypeHarbor    RegistryType = "harbor"
+	RegistryTypeDockerHub RegistryType = "dockerHub"
 
 	FilterStyleTypeText  = "input"
 	FilterStyleTypeRadio = "radio"
@@ -96,7 +94,6 @@ type Registry struct {
 type RegistryQuery struct {
 	// Name is name of the registry to query
 	Name string
-	Type string
 	// Pagination specifies the pagination
 	Pagination *models.Pagination
 }

src/replication/registry/manager.go

@@ -91,7 +91,6 @@ func (m *DefaultManager) List(query ...*model.RegistryQuery) (int64, []*model.Re
 		// limit being -1 indicates no pagination specified, result in all registries matching name returned.
 		listQuery := &dao.ListRegistryQuery{
 			Query: query[0].Name,
-			Type:  query[0].Type,
 			Limit: -1,
 		}
 		if query[0].Pagination != nil {