From cf5cd5902fcc267059878458fef704e92402edbd Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Thu, 18 Apr 2019 17:45:11 +0800 Subject: [PATCH] Fix bug in replication 1. Fix bug when creating the namespace 2. Keep the same logic for hiding access secret 3. Filter only push mode policies for event trigger Signed-off-by: Wenkai Yin --- src/core/api/registry.go | 18 ++++++--- src/core/api/replication_policy.go | 4 +- src/replication/adapter/dockerhub/adapter.go | 11 ++---- src/replication/adapter/harbor/adapter.go | 21 +++++------ .../adapter/huawei/huawei_adapter.go | 37 ++++++++++--------- src/replication/event/handler.go | 5 +++ src/replication/event/handler_test.go | 33 +++++++++++++++++ 7 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/core/api/registry.go b/src/core/api/registry.go index 2348b6fc3..a17e75282 100644 --- a/src/core/api/registry.go +++ b/src/core/api/registry.go @@ -147,14 +147,22 @@ func (t *RegistryAPI) Get() { } // Hide access secret - if r.Credential != nil && len(r.Credential.AccessSecret) != 0 { - r.Credential.AccessSecret = "*****" - } + hideAccessSecret(r.Credential) t.Data["json"] = r t.ServeJSON() } +func hideAccessSecret(credential *model.Credential) { + if credential == nil { + return + } + if len(credential.AccessSecret) == 0 { + return + } + credential.AccessSecret = "*****" +} + // List lists all registries that match a given registry name. func (t *RegistryAPI) List() { name := t.GetString("name") @@ -170,9 +178,7 @@ func (t *RegistryAPI) List() { // Hide passwords for _, r := range registries { - if r.Credential != nil && len(r.Credential.AccessSecret) != 0 { - r.Credential.AccessSecret = "*****" - } + hideAccessSecret(r.Credential) } t.Data["json"] = registries diff --git a/src/core/api/replication_policy.go b/src/core/api/replication_policy.go index b0f9617a7..fa5d4dc86 100644 --- a/src/core/api/replication_policy.go +++ b/src/core/api/replication_policy.go @@ -248,10 +248,10 @@ func populateRegistries(registryMgr registry.Manager, policy *model.Policy) erro return err } if policy.SrcRegistry != nil { - policy.SrcRegistry.Credential = nil + hideAccessSecret(policy.SrcRegistry.Credential) } if policy.DestRegistry != nil { - policy.DestRegistry.Credential = nil + hideAccessSecret(policy.DestRegistry.Credential) } return nil } diff --git a/src/replication/adapter/dockerhub/adapter.go b/src/replication/adapter/dockerhub/adapter.go index ed6f5f8e2..b5c62fc06 100644 --- a/src/replication/adapter/dockerhub/adapter.go +++ b/src/replication/adapter/dockerhub/adapter.go @@ -94,14 +94,8 @@ func (a *adapter) PrepareForPush(resources []*model.Resource) error { if len(resource.Metadata.Repository.Name) == 0 { return errors.New("the name of the namespace cannot be null") } - namespace, _ := util.ParseRepository(resource.Metadata.Repository.Name) - // Docker Hub doesn't support the repository contains no "/" - // just skip here and the following task will fail - if len(namespace) == 0 { - log.Debug("the namespace is empty, skip") - continue - } - + paths := strings.Split(resource.Metadata.Repository.Name, "/") + namespace := paths[0] namespaces[namespace] = struct{}{} } @@ -112,6 +106,7 @@ func (a *adapter) PrepareForPush(resources []*model.Resource) error { if err != nil { return fmt.Errorf("create namespace '%s' in DockerHub error: %v", namespace, err) } + log.Debugf("namespace %s created", namespace) } return nil } diff --git a/src/replication/adapter/harbor/adapter.go b/src/replication/adapter/harbor/adapter.go index dbe832d51..25d8b3e92 100644 --- a/src/replication/adapter/harbor/adapter.go +++ b/src/replication/adapter/harbor/adapter.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "net/http" + "strings" common_http "github.com/goharbor/harbor/src/common/http" "github.com/goharbor/harbor/src/common/http/modifier" @@ -143,13 +144,8 @@ func (a *adapter) PrepareForPush(resources []*model.Resource) error { if len(resource.Metadata.Repository.Name) == 0 { return errors.New("the name of the repository cannot be null") } - projectName, _ := util.ParseRepository(resource.Metadata.Repository.Name) - // harbor doesn't support the repository contains no "/" - // just skip here and the following task will fail - if len(projectName) == 0 { - log.Debug("the project name is empty, skip") - continue - } + paths := strings.Split(resource.Metadata.Repository.Name, "/") + projectName := paths[0] // TODO handle the public projects[projectName] = &project{ Name: projectName, @@ -164,11 +160,14 @@ func (a *adapter) PrepareForPush(resources []*model.Resource) error { Metadata: project.Metadata, } err := a.client.Post(a.coreServiceURL+"/api/projects", pro) - if httpErr, ok := err.(*common_http.Error); ok && httpErr.Code == http.StatusConflict { - log.Debugf("got 409 when trying to create project %s", project.Name) - return nil + if err != nil { + if httpErr, ok := err.(*common_http.Error); ok && httpErr.Code == http.StatusConflict { + log.Debugf("got 409 when trying to create project %s", project.Name) + continue + } + return err } - return err + log.Debugf("project %s created", project.Name) } return nil diff --git a/src/replication/adapter/huawei/huawei_adapter.go b/src/replication/adapter/huawei/huawei_adapter.go index 7d5c85e8e..07275f5b7 100644 --- a/src/replication/adapter/huawei/huawei_adapter.go +++ b/src/replication/adapter/huawei/huawei_adapter.go @@ -121,22 +121,30 @@ func (a *adapter) ConvertResourceMetadata(resourceMetadata *model.ResourceMetada // PrepareForPush prepare for push to Huawei SWR func (a *adapter) PrepareForPush(resources []*model.Resource) error { - // TODO optimize the logic by merging the same namesapces + namespaces := map[string]struct{}{} for _, resource := range resources { - namespace, _ := util.ParseRepository(resource.Metadata.Repository.Name) + paths := strings.Split(resource.Metadata.Repository.Name, "/") + namespace := paths[0] ns, err := a.GetNamespace(namespace) if err != nil { - // - } else { - if ns.Name == namespace { - return nil - } + return err } + if ns != nil && ns.Name == namespace { + continue + } + namespaces[namespace] = struct{}{} + } - url := fmt.Sprintf("%s/dockyard/v2/namespaces", a.registry.URL) + url := fmt.Sprintf("%s/dockyard/v2/namespaces", a.registry.URL) + client := &http.Client{ + Transport: util.GetHTTPTransport(a.registry.Insecure), + } + for namespace := range namespaces { namespacebyte, err := json.Marshal(struct { Namespace string `json:"namespace"` - }{Namespace: namespace}) + }{ + Namespace: namespace, + }) if err != nil { return err } @@ -150,25 +158,18 @@ func (a *adapter) PrepareForPush(resources []*model.Resource) error { encodeAuth := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", a.registry.Credential.AccessKey, a.registry.Credential.AccessSecret))) r.Header.Add("Authorization", "Basic "+encodeAuth) - client := &http.Client{} - if a.registry.Insecure == true { - client = &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, - } - } resp, err := client.Do(r) if err != nil { return err } - defer resp.Body.Close() code := resp.StatusCode if code >= 300 || code < 200 { body, _ := ioutil.ReadAll(resp.Body) return fmt.Errorf("[%d][%s]", code, string(body)) } + + log.Debugf("namespace %s created", namespace) } return nil } diff --git a/src/replication/event/handler.go b/src/replication/event/handler.go index 3d731a4b1..96a7dc509 100644 --- a/src/replication/event/handler.go +++ b/src/replication/event/handler.go @@ -96,6 +96,11 @@ func (h *handler) getRelatedPolicies(resource *model.Resource) ([]*model.Policy, if !policy.Enabled { continue } + // currently, the events are produced only by local Harbor, + // so they should only apply to the policies whose source registry is local Harbor + if !(policy.SrcRegistry == nil || policy.SrcRegistry.ID == 0) { + continue + } // has no trigger if policy.Trigger == nil { continue diff --git a/src/replication/event/handler_test.go b/src/replication/event/handler_test.go index be5c7efb2..372a198eb 100644 --- a/src/replication/event/handler_test.go +++ b/src/replication/event/handler_test.go @@ -71,6 +71,9 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol Value: "test/*", }, }, + DestRegistry: &model.Registry{ + ID: 1, + }, }, // nil trigger { @@ -84,6 +87,9 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol Value: "library/*", }, }, + DestRegistry: &model.Registry{ + ID: 1, + }, }, // doesn't replicate deletion { @@ -99,6 +105,9 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol Value: "library/*", }, }, + DestRegistry: &model.Registry{ + ID: 1, + }, }, // replicate deletion { @@ -114,6 +123,9 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol Value: "library/*", }, }, + DestRegistry: &model.Registry{ + ID: 1, + }, }, // disabled { @@ -129,6 +141,27 @@ func (f *fakedPolicyController) List(...*model.PolicyQuery) (int64, []*model.Pol Value: "library/*", }, }, + DestRegistry: &model.Registry{ + ID: 1, + }, + }, + // the source registry is not local Harbor + { + ID: 6, + Enabled: true, + Deletion: true, + Trigger: &model.Trigger{ + Type: model.TriggerTypeEventBased, + }, + Filters: []*model.Filter{ + { + Type: model.FilterTypeName, + Value: "library/*", + }, + }, + SrcRegistry: &model.Registry{ + ID: 1, + }, }, } return int64(len(polices)), polices, nil