diff --git a/src/core/api/registry.go b/src/core/api/registry.go index d9cc7cf1b..7ade7d87b 100644 --- a/src/core/api/registry.go +++ b/src/core/api/registry.go @@ -11,7 +11,6 @@ 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" @@ -186,12 +185,25 @@ 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)) @@ -350,26 +362,19 @@ func (t *RegistryAPI) Delete() { // GetInfo returns the base info and capability declarations of the registry func (t *RegistryAPI) GetInfo() { id, err := t.GetInt64FromPath(":id") - // "0" is used for the ID of the local Harbor registry - if err != nil || id < 0 { + if err != nil || id <= 0 { t.HandleBadRequest(fmt.Sprintf("invalid registry ID %s", t.GetString(":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 - } + 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)) diff --git a/src/core/api/replication_execution_test.go b/src/core/api/replication_execution_test.go index c6ed3fdfb..5044c2744 100644 --- a/src/core/api/replication_execution_test.go +++ b/src/core/api/replication_execution_test.go @@ -89,6 +89,9 @@ func (f *fakedPolicyManager) Get(id int64) (*model.Policy, error) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, nil } if id == 2 { @@ -199,7 +202,7 @@ func TestCreateExecution(t *testing.T) { }, code: http.StatusNotFound, }, - // 400 + // 400 the policy is disabled { request: &testingRequest{ method: http.MethodPost, diff --git a/src/core/api/replication_policy.go b/src/core/api/replication_policy.go index da4f5b64a..21cda491a 100644 --- a/src/core/api/replication_policy.go +++ b/src/core/api/replication_policy.go @@ -26,8 +26,6 @@ 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 @@ -105,21 +103,30 @@ func (r *ReplicationPolicyAPI) validateName(policy *model.Policy) bool { // make sure the registry referred exists func (r *ReplicationPolicyAPI) validateRegistry(policy *model.Policy) bool { - 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) + srcRegistry, err := replication.RegistryMgr.Get(policy.SrcRegistry.ID) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to get registry %d: %v", registryID, err)) + r.HandleInternalServerError(fmt.Sprintf("failed to get source registry %d: %v", policy.SrcRegistry.ID, err)) return false } - if registry == nil { - r.HandleNotFound(fmt.Sprintf("registry %d not found", registryID)) + if srcRegistry == nil { + r.HandleNotFound(fmt.Sprintf("source registry %d not found", policy.SrcRegistry.ID)) 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 } diff --git a/src/core/api/replication_policy_test.go b/src/core/api/replication_policy_test.go index 654eb909b..29d99e9ce 100644 --- a/src/core/api/replication_policy_test.go +++ b/src/core/api/replication_policy_test.go @@ -22,8 +22,6 @@ 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) { @@ -35,7 +33,12 @@ 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: "faked_registry", + Type: model.RegistryTypeLocalHarbor, + }, nil + } + if id == 2 { + return &model.Registry{ + Type: model.RegistryTypeHarbor, }, nil } return nil, nil @@ -128,11 +131,14 @@ func TestReplicationPolicyAPICreate(t *testing.T) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, }, code: http.StatusBadRequest, }, - // 400 empty registry + // 400 empty source registry { request: &testingRequest{ method: http.MethodPost, @@ -140,10 +146,29 @@ 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{ @@ -155,6 +180,9 @@ func TestReplicationPolicyAPICreate(t *testing.T) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, }, code: http.StatusConflict, @@ -168,12 +196,33 @@ func TestReplicationPolicyAPICreate(t *testing.T) { bodyJSON: &model.Policy{ Name: "policy01", SrcRegistry: &model.Registry{ - ID: 2, + ID: 1, + }, + DestRegistry: &model.Registry{ + ID: 3, }, }, }, 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{ @@ -185,6 +234,9 @@ func TestReplicationPolicyAPICreate(t *testing.T) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, }, code: http.StatusCreated, @@ -291,6 +343,9 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, }, code: http.StatusBadRequest, @@ -306,6 +361,9 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, }, code: http.StatusConflict, @@ -319,12 +377,33 @@ 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{ @@ -336,6 +415,9 @@ func TestReplicationPolicyAPIUpdate(t *testing.T) { SrcRegistry: &model.Registry{ ID: 1, }, + DestRegistry: &model.Registry{ + ID: 2, + }, }, }, code: http.StatusOK, diff --git a/src/replication/adapter/harbor/adapter.go b/src/replication/adapter/harbor/adapter.go index fae8852f4..fadc7bc9f 100644 --- a/src/replication/adapter/harbor/adapter.go +++ b/src/replication/adapter/harbor/adapter.go @@ -37,6 +37,14 @@ 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 { diff --git a/src/replication/dao/registry.go b/src/replication/dao/registry.go index b398b318c..9ad5b243a 100644 --- a/src/replication/dao/registry.go +++ b/src/replication/dao/registry.go @@ -10,6 +10,7 @@ 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 @@ -61,8 +62,13 @@ func ListRegistries(query ...*ListRegistryQuery) (int64, []*models.Registry, err o := dao.GetOrmer() q := o.QueryTable(&models.Registry{}) - if len(query) > 0 && len(query[0].Query) > 0 { - q = q.Filter("name__contains", query[0].Query) + 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) + } } total, err := q.Count() diff --git a/src/replication/event/handler.go b/src/replication/event/handler.go index 3d731a4b1..ec5354a90 100644 --- a/src/replication/event/handler.go +++ b/src/replication/event/handler.go @@ -21,7 +21,6 @@ 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" @@ -160,8 +159,8 @@ func PopulateRegistries(registryMgr registry.Manager, policy *model.Policy) erro } func getRegistry(registryMgr registry.Manager, registry *model.Registry) (*model.Registry, error) { - if registry == nil || registry.ID == 0 { - return GetLocalRegistry(), nil + if registry == nil { + return nil, errors.New("empty registry") } reg, err := registryMgr.Get(registry.ID) if err != nil { @@ -172,20 +171,3 @@ 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, - } -} diff --git a/src/replication/event/handler_test.go b/src/replication/event/handler_test.go index be5c7efb2..97e351bfa 100644 --- a/src/replication/event/handler_test.go +++ b/src/replication/event/handler_test.go @@ -62,6 +62,12 @@ 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, }, @@ -77,7 +83,13 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol ID: 2, Enabled: true, Deletion: true, - Trigger: nil, + SrcRegistry: &model.Registry{ + ID: 1, + }, + DestRegistry: &model.Registry{ + ID: 1, + }, + Trigger: nil, Filters: []*model.Filter{ { Type: model.FilterTypeName, @@ -90,6 +102,12 @@ 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, }, @@ -105,6 +123,12 @@ 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, }, @@ -120,6 +144,12 @@ 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, }, diff --git a/src/replication/model/policy.go b/src/replication/model/policy.go index 3751d2c74..c2a856c89 100644 --- a/src/replication/model/policy.go +++ b/src/replication/model/policy.go @@ -72,20 +72,12 @@ func (p *Policy) Valid(v *validation.Validation) { if len(p.Name) == 0 { v.SetError("name", "cannot be empty") } - var srcRegistryID, dstRegistryID int64 - if p.SrcRegistry != nil { - srcRegistryID = p.SrcRegistry.ID + if p.SrcRegistry == nil || p.SrcRegistry.ID == 0 { + v.SetError("src_registry", "cannot be empty") } - if p.DestRegistry != nil { - dstRegistryID = p.DestRegistry.ID + if p.DestRegistry == nil || p.DestRegistry.ID == 0 { + v.SetError("dest_registry", "cannot be empty") } - - // 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 && @@ -96,7 +88,6 @@ func (p *Policy) Valid(v *validation.Validation) { break } } - // valid trigger if p.Trigger != nil { if p.Trigger.Type != TriggerTypeManual && diff --git a/src/replication/model/policy_test.go b/src/replication/model/policy_test.go index 9b06a5f31..db35de65f 100644 --- a/src/replication/model/policy_test.go +++ b/src/replication/model/policy_test.go @@ -31,23 +31,20 @@ func TestValidOfPolicy(t *testing.T) { policy: &Policy{}, pass: false, }, - // empty source registry and destination registry + // empty source registry { policy: &Policy{ Name: "policy01", }, pass: false, }, - // source registry and destination registry both not empty + // empty destination registry { policy: &Policy{ Name: "policy01", SrcRegistry: &Registry{ ID: 1, }, - DestRegistry: &Registry{ - ID: 2, - }, }, pass: false, }, @@ -56,10 +53,10 @@ func TestValidOfPolicy(t *testing.T) { policy: &Policy{ Name: "policy01", SrcRegistry: &Registry{ - ID: 0, + ID: 1, }, DestRegistry: &Registry{ - ID: 1, + ID: 2, }, Filters: []*Filter{ { @@ -74,10 +71,10 @@ func TestValidOfPolicy(t *testing.T) { policy: &Policy{ Name: "policy01", SrcRegistry: &Registry{ - ID: 0, + ID: 1, }, DestRegistry: &Registry{ - ID: 1, + ID: 2, }, Filters: []*Filter{ { @@ -96,10 +93,10 @@ func TestValidOfPolicy(t *testing.T) { policy: &Policy{ Name: "policy01", SrcRegistry: &Registry{ - ID: 0, + ID: 1, }, DestRegistry: &Registry{ - ID: 1, + ID: 2, }, Filters: []*Filter{ { @@ -118,10 +115,10 @@ func TestValidOfPolicy(t *testing.T) { policy: &Policy{ Name: "policy01", SrcRegistry: &Registry{ - ID: 0, + ID: 1, }, DestRegistry: &Registry{ - ID: 1, + ID: 2, }, Filters: []*Filter{ { diff --git a/src/replication/model/registry.go b/src/replication/model/registry.go index 0d9e10390..31976ecf3 100644 --- a/src/replication/model/registry.go +++ b/src/replication/model/registry.go @@ -23,8 +23,10 @@ import ( // const definition const ( // RegistryTypeHarbor indicates registry type harbor - RegistryTypeHarbor RegistryType = "harbor" - RegistryTypeDockerHub RegistryType = "dockerHub" + RegistryTypeHarbor RegistryType = "harbor" + // Local Harbor is the type of Harbor instance where the replication service is running on + RegistryTypeLocalHarbor RegistryType = "localHarbor" + RegistryTypeDockerHub RegistryType = "dockerHub" FilterStyleTypeText = "input" FilterStyleTypeRadio = "radio" @@ -94,6 +96,7 @@ 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 } diff --git a/src/replication/registry/manager.go b/src/replication/registry/manager.go index c6d56289d..1f24ace05 100644 --- a/src/replication/registry/manager.go +++ b/src/replication/registry/manager.go @@ -91,6 +91,7 @@ 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 {