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
caicloud · Nov 20, 2018 · 315acbf · 315acbf
1 parent 00b8ed4
commit 315acbf
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 66 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,44 +327,26 @@ 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
 			}
 		}
 	}
 
 	// 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