From 315acbfed038eb775fe9e54b9a78691ff762262f Mon Sep 17 00:00:00 2001 From: Jian Zhu <36154065+zhujian7@users.noreply.github.com> Date: Tue, 20 Nov 2018 20:18:51 +0800 Subject: [PATCH] fix: support update svn hooks (#611) * fix: support update svn hooks - fix can not update svn hooks - get svn repo info by main repo url instead of svn server url * fix: address code review comments --- pkg/api/v1/handler/webhook.go | 4 +- pkg/scm/provider/github/github.go | 2 +- pkg/scm/provider/gitlab/gitlabv3.go | 2 +- pkg/scm/provider/gitlab/gitlabv4.go | 2 +- pkg/scm/provider/svn/svn.go | 4 +- pkg/scm/scm.go | 2 +- pkg/server/manager/pipeline.go | 111 ++++++++++++++-------------- 7 files changed, 61 insertions(+), 66 deletions(-) diff --git a/pkg/api/v1/handler/webhook.go b/pkg/api/v1/handler/webhook.go index e745cb767..0075c5811 100644 --- a/pkg/api/v1/handler/webhook.go +++ b/pkg/api/v1/handler/webhook.go @@ -468,11 +468,11 @@ func HandleSVNHooks(ctx context.Context, repoid, revision string) error { // and the pipeline has not been triggered if strings.Contains(fullPath, url) && !isAlreadyTriggered { triggeredPipelines[pipeline.ID] = struct{}{} - log.Info("SVN hooks triggered pipeline: %s, id: %s", pipeline.Name, pipeline.ID) + log.Infof("SVN hooks triggered pipeline: %s, id: %s", pipeline.Name, pipeline.ID) // Trigger the pipeline errt := triggerSVNPipelines(pc, pipeline, revision) if errt != nil { - log.Error("svn hook trigger pipeline failed as %v", errt) + log.Errorf("svn hook trigger pipeline failed as %v", errt) } } } diff --git a/pkg/scm/provider/github/github.go b/pkg/scm/provider/github/github.go index e51b578f7..dec8ef6e0 100644 --- a/pkg/scm/provider/github/github.go +++ b/pkg/scm/provider/github/github.go @@ -482,6 +482,6 @@ func (g *Github) GetPullRequestSHA(repoURL string, number int) (string, error) { return *pr.Head.SHA, nil } -func (g *Github) RetrieveRepoInfo() (*api.RepoInfo, error) { +func (g *Github) RetrieveRepoInfo(url string) (*api.RepoInfo, error) { return nil, errors.ErrorNotImplemented.Error("retrive GitHub repo id") } diff --git a/pkg/scm/provider/gitlab/gitlabv3.go b/pkg/scm/provider/gitlab/gitlabv3.go index 0a149dde6..afee62fff 100644 --- a/pkg/scm/provider/gitlab/gitlabv3.go +++ b/pkg/scm/provider/gitlab/gitlabv3.go @@ -240,6 +240,6 @@ func (g *GitlabV3) GetPullRequestSHA(repoURL string, number int) (string, error) return "", errors.ErrorNotImplemented.Error("get pull request sha") } -func (g *GitlabV3) RetrieveRepoInfo() (*api.RepoInfo, error) { +func (g *GitlabV3) RetrieveRepoInfo(url string) (*api.RepoInfo, error) { return nil, errors.ErrorNotImplemented.Error("retrive GitLab repo id") } diff --git a/pkg/scm/provider/gitlab/gitlabv4.go b/pkg/scm/provider/gitlab/gitlabv4.go index a9429afac..70275260e 100644 --- a/pkg/scm/provider/gitlab/gitlabv4.go +++ b/pkg/scm/provider/gitlab/gitlabv4.go @@ -255,6 +255,6 @@ func (g *GitlabV4) GetPullRequestSHA(repoURL string, number int) (string, error) return "", errors.ErrorNotImplemented.Error("get pull request sha") } -func (g *GitlabV4) RetrieveRepoInfo() (*api.RepoInfo, error) { +func (g *GitlabV4) RetrieveRepoInfo(url string) (*api.RepoInfo, error) { return nil, errors.ErrorNotImplemented.Error("retrive GitLab repo id") } diff --git a/pkg/scm/provider/svn/svn.go b/pkg/scm/provider/svn/svn.go index d981c679d..13564e54c 100644 --- a/pkg/scm/provider/svn/svn.go +++ b/pkg/scm/provider/svn/svn.go @@ -122,15 +122,13 @@ func (s *SVN) GetPullRequestSHA(repoURL string, number int) (string, error) { // 'svn info --show-item repos-uuid(or repos-root-url) --username {user} --password {password} // --non-interactive --trust-server-cert-failures unknown-ca,cn-mismatch,expired,not-yet-valid,other // --no-auth-cache {remote-svn-address}' -func (s *SVN) RetrieveRepoInfo() (*api.RepoInfo, error) { +func (s *SVN) RetrieveRepoInfo(url string) (*api.RepoInfo, error) { repoInfo := &api.RepoInfo{} username, password, err := s.spilitToken(s.scmCfg.Token) if err != nil { return repoInfo, errors.ErrorUnknownInternal.Error("get svn credential failed") } - url := s.scmCfg.Server - repoInfo.ID, err = retrieveSVNRepoInfo(url, username, password, "repos-uuid") if err != nil { return repoInfo, err diff --git a/pkg/scm/scm.go b/pkg/scm/scm.go index 40dda5072..33ac4126c 100644 --- a/pkg/scm/scm.go +++ b/pkg/scm/scm.go @@ -59,7 +59,7 @@ type SCMProvider interface { DeleteWebHook(repoURL string, webHookUrl string) error CreateStatus(recordStatus api.Status, targetURL, repoURL, commitSha string) error GetPullRequestSHA(repoURL string, number int) (string, error) - RetrieveRepoInfo() (*api.RepoInfo, error) + RetrieveRepoInfo(url string) (*api.RepoInfo, error) } // WebHook represents the params for SCM webhook. diff --git a/pkg/server/manager/pipeline.go b/pkg/server/manager/pipeline.go index 6887e89aa..2a9aace21 100644 --- a/pkg/server/manager/pipeline.go +++ b/pkg/server/manager/pipeline.go @@ -112,60 +112,75 @@ func (m *pipelineManager) CreatePipeline(projectName string, pipeline *api.Pipel return nil, err } + gitSource, err := api.GetGitSource(pipeline.Build.Stages.CodeCheckout.MainRepo) + if err != nil { + return nil, err + } + // Create SCM webhook if enable SCM trigger. - var webHook *scm.WebHook - var gitSource *api.GitSource - if pipeline.AutoTrigger != nil && pipeline.AutoTrigger.SCMTrigger != nil { - gitSource, err = api.GetGitSource(pipeline.Build.Stages.CodeCheckout.MainRepo) - if err != nil { - return nil, err + err = createWebhook(pipeline, provider, scmConfig.Type, gitSource.Url, "") + if err != nil { + logdog.Errorf("create webhook failed: %v", err) + return nil, err + } + + // Remove the webhook if there is error. + defer func() { + if err != nil && gitSource != nil && pipeline.AutoTrigger != nil && pipeline.AutoTrigger.SCMTrigger != nil { + if err = provider.DeleteWebHook(gitSource.Url, pipeline.AutoTrigger.SCMTrigger.Webhook); err != nil { + logdog.Errorf("Fail to delete the webhook %s", pipeline.Name) + } } + }() - if scmConfig.Type == api.SVN && pipeline.AutoTrigger.SCMTrigger.PostCommit != nil { + createdPipeline, err := m.dataStore.CreatePipeline(pipeline) + if err != nil { + return nil, err + } + + return createdPipeline, nil +} + +func createWebhook(pipeline *api.Pipeline, provider scm.SCMProvider, scmType api.SCMType, mainRepoUrl, pipelineID string) error { + // Create SCM webhook if enable SCM trigger. + if pipeline.AutoTrigger != nil && pipeline.AutoTrigger.SCMTrigger != nil { + + if scmType == api.SVN && pipeline.AutoTrigger.SCMTrigger.PostCommit != nil { // SVN post commit hooks - repoInfo, err := provider.RetrieveRepoInfo() + repoInfo, err := provider.RetrieveRepoInfo(mainRepoUrl) if err != nil { - return nil, err + return err } pipeline.AutoTrigger.SCMTrigger.PostCommit.RepoInfo = repoInfo } else { // GitHub and GitLab webhook - pipeline.ID = bson.NewObjectId().Hex() - webHook = &scm.WebHook{ - Url: generateWebhookURL(scmConfig.Type, pipeline.ID), + if pipelineID == "" { + pipeline.ID = bson.NewObjectId().Hex() + } else { + pipeline.ID = pipelineID + } + + webHook := &scm.WebHook{ + Url: generateWebhookURL(scmType, pipeline.ID), Events: collectSCMEvents(pipeline.AutoTrigger.SCMTrigger), } - if err := provider.CreateWebHook(gitSource.Url, webHook); err != nil { + if err := provider.CreateWebHook(mainRepoUrl, webHook); err != nil { logdog.Errorf("create webhook failed: %v", err) scmType := pipeline.Build.Stages.CodeCheckout.MainRepo.Type if (scmType == api.Gitlab && strings.Contains(err.Error(), "403")) || (scmType == api.Github && strings.Contains(err.Error(), "404")) { - return nil, httperror.ErrorCreateWebhookPermissionDenied.Error(pipeline.Name) + return httperror.ErrorCreateWebhookPermissionDenied.Error(pipeline.Name) } - return nil, err + return err } pipeline.AutoTrigger.SCMTrigger.Webhook = webHook.Url } } - // Remove the webhook if there is error. - defer func() { - if err != nil && gitSource != nil && webHook != nil { - if err = provider.DeleteWebHook(gitSource.Url, webHook.Url); err != nil { - logdog.Errorf("Fail to delete the webhook %s", pipeline.Name) - } - } - }() - - createdPipeline, err := m.dataStore.CreatePipeline(pipeline) - if err != nil { - return nil, err - } - - return createdPipeline, nil + return nil } // GetPipeline gets the pipeline by name in one project. @@ -312,15 +327,15 @@ func (m *pipelineManager) UpdatePipeline(projectName string, pipelineName string return nil, err } + gitSource, err := api.GetGitSource(pipeline.Build.Stages.CodeCheckout.MainRepo) + if err != nil { + return nil, err + } + // Remove the old webhook if exists. if pipeline.AutoTrigger != nil && pipeline.AutoTrigger.SCMTrigger != nil { scmTrigger := pipeline.AutoTrigger.SCMTrigger if scmTrigger.Webhook != "" { - gitSource, err := api.GetGitSource(pipeline.Build.Stages.CodeCheckout.MainRepo) - if err != nil { - return nil, err - } - if err := provider.DeleteWebHook(gitSource.Url, scmTrigger.Webhook); err != nil { return nil, err } @@ -328,28 +343,10 @@ func (m *pipelineManager) UpdatePipeline(projectName string, pipelineName string } // Create the new webhook if necessary. - if newPipeline.AutoTrigger != nil && newPipeline.AutoTrigger.SCMTrigger != nil { - scmTrigger := newPipeline.AutoTrigger.SCMTrigger - gitSource, err := api.GetGitSource(newPipeline.Build.Stages.CodeCheckout.MainRepo) - if err != nil { - return nil, err - } - - webHook := &scm.WebHook{ - Url: generateWebhookURL(scmConfig.Type, pipeline.ID), - Events: collectSCMEvents(scmTrigger), - } - if err := provider.CreateWebHook(gitSource.Url, webHook); err != nil { - logdog.Errorf("create webhook failed: %v", err) - scmType := pipeline.Build.Stages.CodeCheckout.MainRepo.Type - if (scmType == api.Gitlab && strings.Contains(err.Error(), "403")) || - (scmType == api.Github && strings.Contains(err.Error(), "404")) { - return nil, httperror.ErrorCreateWebhookPermissionDenied.Error(pipeline.Name) - } - return nil, err - } - - newPipeline.AutoTrigger.SCMTrigger.Webhook = webHook.Url + err = createWebhook(newPipeline, provider, scmConfig.Type, gitSource.Url, pipeline.ID) + if err != nil { + logdog.Errorf("create webhook failed: %v", err) + return nil, err } pipeline.AutoTrigger = newPipeline.AutoTrigger