Merge pull request #365 from LinuxSuRen/fix/gitlab-hook-verify

fix: the logic of if skip tls is incorrect
jenkins-x · Jan 25, 2023 · ab9cfaa · ab9cfaa
2 parents aff8b3c + 2c0d51f
commit ab9cfaa
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 45 deletions.
diff --git a/scm/driver/gitlab/repo.go b/scm/driver/gitlab/repo.go
@@ -347,37 +347,7 @@ func (s *repositoryService) ListStatus(ctx context.Context, repo, ref string, op
 }
 
 func (s *repositoryService) CreateHook(ctx context.Context, repo string, input *scm.HookInput) (*scm.Hook, *scm.Response, error) {
-	params := url.Values{}
-	params.Set("url", input.Target)
-	if input.Secret != "" {
-		params.Set("token", input.Secret)
-	}
-	params.Set("enable_ssl_verification", strconv.FormatBool(input.SkipVerify))
-	hasStarEvents := false
-	for _, event := range input.NativeEvents {
-		if event == "*" {
-			hasStarEvents = true
-		}
-	}
-	if input.Events.Issue || hasStarEvents {
-		params.Set("issues_events", "true")
-	}
-	if input.Events.IssueComment ||
-		input.Events.PullRequestComment || hasStarEvents {
-		params.Set("note_events", "true")
-	}
-	if input.Events.PullRequest || hasStarEvents {
-		params.Set("merge_requests_events", "true")
-	}
-	if input.Events.Push || input.Events.Branch || hasStarEvents {
-		params.Set("push_events", "true")
-	}
-	if input.Events.Tag || hasStarEvents {
-		params.Set("tag_push_events", "true")
-	}
-	if input.Events.Release || hasStarEvents {
-		params.Set("releases_events", "true")
-	}
+	params := convertHookInputToGenericParam(input)
 
 	path := fmt.Sprintf("api/v4/projects/%s/hooks?%s", encode(repo), params.Encode())
 	out := new(hook)
@@ -386,13 +356,22 @@ func (s *repositoryService) CreateHook(ctx context.Context, repo string, input *
 }
 
 func (s *repositoryService) UpdateHook(ctx context.Context, repo string, input *scm.HookInput) (*scm.Hook, *scm.Response, error) {
-	params := url.Values{}
+	params := convertHookInputToGenericParam(input)
 	hookID := input.Name
+
+	path := fmt.Sprintf("api/v4/projects/%s/hooks/%s?%s", encode(repo), hookID, params.Encode())
+	out := new(hook)
+	res, err := s.client.do(ctx, "PUT", path, nil, out)
+	return convertHook(out), res, err
+}
+
+func convertHookInputToGenericParam(input *scm.HookInput) url.Values {
+	params := url.Values{}
 	params.Set("url", input.Target)
 	if input.Secret != "" {
 		params.Set("token", input.Secret)
 	}
-	params.Set("enable_ssl_verification", strconv.FormatBool(input.SkipVerify))
+	params.Set("enable_ssl_verification", strconv.FormatBool(!input.SkipVerify))
 	hasStarEvents := false
 	for _, event := range input.NativeEvents {
 		if event == "*" {
@@ -425,11 +404,7 @@ func (s *repositoryService) UpdateHook(ctx context.Context, repo string, input *
 	} else {
 		params.Set("tag_push_events", "false")
 	}
-
-	path := fmt.Sprintf("api/v4/projects/%s/hooks/%s?%s", encode(repo), hookID, params.Encode())
-	out := new(hook)
-	res, err := s.client.do(ctx, "PUT", path, nil, out)
-	return convertHook(out), res, err
+	return params
 }
 
 func (s *repositoryService) CreateStatus(ctx context.Context, repo, ref string, input *scm.StatusInput) (*scm.Status, *scm.Response, error) {

diff --git a/scm/driver/gitlab/repo_test.go b/scm/driver/gitlab/repo_test.go
@@ -566,9 +566,16 @@ func TestRepositoryHookCreate(t *testing.T) {
 
 	gock.New("https://gitlab.com").
 		Post("/api/v4/projects/diaspora/diaspora/hooks").
-		MatchParam("enable_ssl_verification", "true").
-		MatchParam("token", "topsecret").
-		MatchParam("url", "https://ci.example.com/hook").
+		MatchParams(map[string]string{
+			"enable_ssl_verification": "true",
+			"issues_events":           "false",
+			"merge_requests_events":   "false",
+			"note_events":             "false",
+			"push_events":             "false",
+			"tag_push_events":         "false",
+			"token":                   "topsecret",
+			"url":                     "https://ci.example.com/hook",
+		}).
 		Reply(201).
 		Type("application/json").
 		SetHeaders(mockHeaders).
@@ -578,7 +585,7 @@ func TestRepositoryHookCreate(t *testing.T) {
 		Name:       "drone",
 		Target:     "https://ci.example.com/hook",
 		Secret:     "topsecret",
-		SkipVerify: true,
+		SkipVerify: false,
 	}
 
 	client := NewDefault()
@@ -621,7 +628,7 @@ func TestRepositoryHookCreateSkipVerify(t *testing.T) {
 		Name:       "drone",
 		Target:     "https://ci.example.com/hook",
 		Secret:     "topsecret",
-		SkipVerify: false,
+		SkipVerify: true,
 	}
 
 	client := NewDefault()
@@ -691,14 +698,24 @@ func TestRepositoryHookUpdateSkipVerify(t *testing.T) {
 
 	gock.New("https://gitlab.com").
 		Put("/api/v4/projects/diaspora/diaspora/hooks/1").
+		MatchParams(map[string]string{
+			"enable_ssl_verification": "false",
+			"issues_events":           "false",
+			"merge_requests_events":   "false",
+			"note_events":             "false",
+			"push_events":             "false",
+			"tag_push_events":         "false",
+			"url":                     "http://example.com/hook",
+		}).
 		Reply(200).
 		Type("application/json").
 		SetHeaders(mockHeaders).
 		File("testdata/hook_ssl_false.json")
 
 	in := &scm.HookInput{
-		Name:   "1",
-		Target: "http://example.com/hook",
+		Name:       "1",
+		Target:     "http://example.com/hook",
+		SkipVerify: true,
 	}
 
 	client := NewDefault()