diff --git a/scm/driver/gitlab/repo.go b/scm/driver/gitlab/repo.go index ed3152a25..520d7bf93 100644 --- a/scm/driver/gitlab/repo.go +++ b/scm/driver/gitlab/repo.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "net/url" + "sort" "strconv" "strings" "time" @@ -46,19 +47,22 @@ type access struct { } type hook struct { - ID int `json:"id"` - URL string `json:"url"` - ProjectID int `json:"project_id"` - PushEvents bool `json:"push_events"` - IssuesEvents bool `json:"issues_events"` - MergeRequestsEvents bool `json:"merge_requests_events"` - TagPushEvents bool `json:"tag_push_events"` - NoteEvents bool `json:"note_events"` - JobEvents bool `json:"job_events"` - PipelineEvents bool `json:"pipeline_events"` - WikiPageEvents bool `json:"wiki_page_events"` - EnableSslVerification bool `json:"enable_ssl_verification"` - CreatedAt time.Time `json:"created_at"` + ID int `json:"id"` + URL string `json:"url"` + ProjectID int `json:"project_id"` + PushEvents bool `json:"push_events"` + IssuesEvents bool `json:"issues_events"` + ConfidentialIssuesEvents bool `json:"confidential_issues_events"` + MergeRequestsEvents bool `json:"merge_requests_events"` + TagPushEvents bool `json:"tag_push_events"` + NoteEvents bool `json:"note_events"` + ConfidentialNoteEvents bool `json:"confidential_note_events"` + JobEvents bool `json:"job_events"` + PipelineEvents bool `json:"pipeline_events"` + WikiPageEvents bool `json:"wiki_page_events"` + DeploymentEvents bool `json:"deployment_events"` + EnableSslVerification bool `json:"enable_ssl_verification"` + CreatedAt time.Time `json:"created_at"` } type repositoryService struct { @@ -110,30 +114,17 @@ 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) + // Earlier versions of gitlab returned an error if the token was an empty string, so we only set + // the token value if non-empty. if input.Secret != "" { params.Set("token", input.Secret) } - if input.SkipVerify { - params.Set("enable_ssl_verification", "false") + params.Set("enable_ssl_verification", strconv.FormatBool(!input.SkipVerify)) + for k, v := range convertFromHookEvents(input.Events) { + params.Set(k, strconv.FormatBool(v)) } - if input.Events.Branch { - // no-op - } - if input.Events.Issue { - params.Set("issues_events", "true") - } - if input.Events.IssueComment || - input.Events.PullRequestComment { - params.Set("note_events", "true") - } - if input.Events.PullRequest { - params.Set("merge_requests_events", "true") - } - if input.Events.Push || input.Events.Branch { - params.Set("push_events", "true") - } - if input.Events.Tag { - params.Set("tag_push_events", "true") + for _, k := range input.NativeEvents { + params.Set(k, "true") } path := fmt.Sprintf("api/v4/projects/%s/hooks?%s", encode(repo), params.Encode()) @@ -153,8 +144,22 @@ func (s *repositoryService) CreateStatus(ctx context.Context, repo, ref string, return convertStatus(out), res, err } -func (s *repositoryService) UpdateHook(ctx context.Context, repo string, id string, input *scm.HookInput) (*scm.Hook, *scm.Response, error) { - return nil, nil, scm.ErrNotSupported +func (s *repositoryService) UpdateHook(ctx context.Context, repo, id string, input *scm.HookInput) (*scm.Hook, *scm.Response, error) { + params := url.Values{} + params.Set("url", input.Target) + params.Set("token", input.Secret) + params.Set("enable_ssl_verification", strconv.FormatBool(!input.SkipVerify)) + for k, v := range convertFromHookEvents(input.Events) { + params.Set(k, strconv.FormatBool(v)) + } + for _, k := range input.NativeEvents { + params.Set(k, "true") + } + + path := fmt.Sprintf("api/v4/projects/%s/hooks/%s?%s", encode(repo), id, params.Encode()) + out := new(hook) + res, err := s.client.do(ctx, "PUT", path, nil, out) + return convertHook(out), res, err } func (s *repositoryService) DeleteHook(ctx context.Context, repo string, id string) (*scm.Response, error) { @@ -219,6 +224,45 @@ func convertHook(from *hook) *scm.Hook { } } +func convertFromHookEvents(from scm.HookEvents) map[string]bool { + return map[string]bool{ + "push_events": from.Push || from.Branch, + "issues_events": from.Issue, + "merge_requests_events": from.PullRequest, + "confidential_issues_events": false, + "tag_push_events": from.Tag, + "note_events": from.IssueComment || from.PullRequestComment || from.ReviewComment, + "confidential_note_events": false, + "job_events": false, + "pipeline_events": false, + "wiki_page_events": false, + "deployment_events": from.Deployment, + } +} + +func convertEvents(from *hook) []string { + var events []string + for k, v := range map[string]bool{ + "push_events": from.PushEvents, + "issues_events": from.IssuesEvents, + "merge_requests_events": from.MergeRequestsEvents, + "confidential_issues_events": from.ConfidentialIssuesEvents, + "tag_push_events": from.TagPushEvents, + "note_events": from.NoteEvents, + "confidential_note_events": from.ConfidentialNoteEvents, + "job_events": from.JobEvents, + "pipeline_events": from.PipelineEvents, + "wiki_page_events": from.WikiPageEvents, + "deployment_events": from.DeploymentEvents, + } { + if v { + events = append(events, k) + } + } + sort.Strings(events) + return events +} + type status struct { Name string `json:"name"` Desc null.String `json:"description"` @@ -247,26 +291,6 @@ func convertStatus(from *status) *scm.Status { } } -func convertEvents(from *hook) []string { - var events []string - if from.IssuesEvents { - events = append(events, "issues") - } - if from.TagPushEvents { - events = append(events, "tag") - } - if from.PushEvents { - events = append(events, "push") - } - if from.NoteEvents { - events = append(events, "comment") - } - if from.MergeRequestsEvents { - events = append(events, "merge") - } - return events -} - func convertState(from string) scm.State { switch from { case "canceled": diff --git a/scm/driver/gitlab/repo_test.go b/scm/driver/gitlab/repo_test.go index 48e21db6b..396448a67 100644 --- a/scm/driver/gitlab/repo_test.go +++ b/scm/driver/gitlab/repo_test.go @@ -7,7 +7,9 @@ package gitlab import ( "context" "encoding/json" + "fmt" "io/ioutil" + "regexp" "testing" "github.com/drone/go-scm/scm" @@ -301,47 +303,63 @@ func TestRepositoryHookList(t *testing.T) { t.Run("Page", testPage(res)) } -func TestRepositoryHookDelete(t *testing.T) { +func TestRepositoryHookCreate(t *testing.T) { defer gock.Off() gock.New("https://gitlab.com"). - Delete("/api/v4/projects/diaspora/diaspora/hooks/1"). - Reply(204). + Post("/api/v4/projects/diaspora/diaspora/hooks"). + MatchParam("token", "topsecret"). + MatchParam("url", "https://ci.example.com/hook"). + Reply(201). Type("application/json"). - SetHeaders(mockHeaders) + SetHeaders(mockHeaders). + File("testdata/hook.json") + + in := &scm.HookInput{ + Name: "drone", + Target: "https://ci.example.com/hook", + Secret: "topsecret", + SkipVerify: false, + } client := NewDefault() - res, err := client.Repositories.DeleteHook(context.Background(), "diaspora/diaspora", "1") + got, res, err := client.Repositories.CreateHook(context.Background(), "diaspora/diaspora", in) if err != nil { t.Error(err) return } - if got, want := res.Status, 204; got != want { - t.Errorf("Want response status %d, got %d", want, got) + want := new(scm.Hook) + raw, _ := ioutil.ReadFile("testdata/hook.json.golden") + json.Unmarshal(raw, want) + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("Unexpected Results") + t.Log(diff) } t.Run("Request", testRequest(res)) t.Run("Rate", testRate(res)) } -func TestRepositoryHookCreate(t *testing.T) { +func TestRepositoryHookCreate_SkipVerification(t *testing.T) { defer gock.Off() gock.New("https://gitlab.com"). Post("/api/v4/projects/diaspora/diaspora/hooks"). + MatchParam("enable_ssl_verification", "false"). MatchParam("token", "topsecret"). MatchParam("url", "https://ci.example.com/hook"). Reply(201). Type("application/json"). SetHeaders(mockHeaders). - File("testdata/hook.json") + File("testdata/hook_skip_verification.json") in := &scm.HookInput{ Name: "drone", Target: "https://ci.example.com/hook", Secret: "topsecret", - SkipVerify: false, + SkipVerify: true, } client := NewDefault() @@ -352,7 +370,7 @@ func TestRepositoryHookCreate(t *testing.T) { } want := new(scm.Hook) - raw, _ := ioutil.ReadFile("testdata/hook.json.golden") + raw, _ := ioutil.ReadFile("testdata/hook_skip_verification.json.golden") json.Unmarshal(raw, want) if diff := cmp.Diff(got, want); diff != "" { @@ -364,35 +382,34 @@ func TestRepositoryHookCreate(t *testing.T) { t.Run("Rate", testRate(res)) } -func TestRepositoryHookCreate_SkipVerification(t *testing.T) { +func TestRepositoryHookUpdate(t *testing.T) { defer gock.Off() gock.New("https://gitlab.com"). - Post("/api/v4/projects/diaspora/diaspora/hooks"). - MatchParam("enable_ssl_verification", "false"). + Put("/api/v4/projects/diaspora/diaspora/hooks/1"). MatchParam("token", "topsecret"). MatchParam("url", "https://ci.example.com/hook"). - Reply(201). + Reply(200). Type("application/json"). SetHeaders(mockHeaders). - File("testdata/hook_skip_verification.json") + File("testdata/hook.json") in := &scm.HookInput{ Name: "drone", Target: "https://ci.example.com/hook", Secret: "topsecret", - SkipVerify: true, + SkipVerify: false, } client := NewDefault() - got, res, err := client.Repositories.CreateHook(context.Background(), "diaspora/diaspora", in) + got, res, err := client.Repositories.UpdateHook(context.Background(), "diaspora/diaspora", "1", in) if err != nil { t.Error(err) return } want := new(scm.Hook) - raw, _ := ioutil.ReadFile("testdata/hook_skip_verification.json.golden") + raw, _ := ioutil.ReadFile("testdata/hook.json.golden") json.Unmarshal(raw, want) if diff := cmp.Diff(got, want); diff != "" { @@ -404,6 +421,91 @@ func TestRepositoryHookCreate_SkipVerification(t *testing.T) { t.Run("Rate", testRate(res)) } +func TestRepositoryHookDelete(t *testing.T) { + defer gock.Off() + + gock.New("https://gitlab.com"). + Delete("/api/v4/projects/diaspora/diaspora/hooks/1"). + Reply(204). + Type("application/json"). + SetHeaders(mockHeaders) + + client := NewDefault() + res, err := client.Repositories.DeleteHook(context.Background(), "diaspora/diaspora", "1") + if err != nil { + t.Error(err) + return + } + + if got, want := res.Status, 204; got != want { + t.Errorf("Want response status %d, got %d", want, got) + } + + t.Run("Request", testRequest(res)) + t.Run("Rate", testRate(res)) +} + +func TestConvertFromHookEvents(t *testing.T) { + str := func(v scm.HookEvents) string { + s := fmt.Sprintf("%+v", v) + return regexp.MustCompile(" *[A-Za-z]+:false *").ReplaceAllString(s, "") + } + for _, test := range []struct { + src scm.HookEvents + dst string + }{{ + src: scm.HookEvents{Push: true}, + dst: "push_events", + }, { + src: scm.HookEvents{Branch: true}, + dst: "push_events", + }, { + src: scm.HookEvents{Issue: true}, + dst: "issues_events", + }, { + src: scm.HookEvents{PullRequest: true}, + dst: "merge_requests_events", + }, { + src: scm.HookEvents{Tag: true}, + dst: "tag_push_events", + }, { + src: scm.HookEvents{IssueComment: true}, + dst: "note_events", + }, { + src: scm.HookEvents{PullRequestComment: true}, + dst: "note_events", + }, { + src: scm.HookEvents{ReviewComment: true}, + dst: "note_events", + }, { + src: scm.HookEvents{Deployment: true}, + dst: "deployment_events", + }} { + events := convertFromHookEvents(test.src) + for _, k := range []string{ + "push_events", + "issues_events", + "merge_requests_events", + "confidential_issues_events", + "tag_push_events", + "note_events", + "confidential_note_events", + "job_events", + "pipeline_events", + "wiki_page_events", + "deployment_events", + } { + if v, ok := events[k]; !ok || v != (k == test.dst) { + got, want := fmt.Sprintf("%s=%t", k, v), fmt.Sprintf("%s=%t", k, k == test.dst) + if !ok { + got = "none" + } + t.Errorf("Want %s converted to %s, got %s", str(test.src), want, got) + } + } + } +} + func TestConvertState(t *testing.T) { tests := []struct { src string diff --git a/scm/driver/gitlab/testdata/hook.json.golden b/scm/driver/gitlab/testdata/hook.json.golden index d11f4b7c6..e320db0d5 100644 --- a/scm/driver/gitlab/testdata/hook.json.golden +++ b/scm/driver/gitlab/testdata/hook.json.golden @@ -3,11 +3,14 @@ "Name": "", "Target": "http://example.com/hook", "Events": [ - "issues", - "tag", - "push", - "comment", - "merge" + "issues_events", + "job_events", + "merge_requests_events", + "note_events", + "pipeline_events", + "push_events", + "tag_push_events", + "wiki_page_events" ], "Active": true, "SkipVerify": false diff --git a/scm/driver/gitlab/testdata/hook_skip_verification.json.golden b/scm/driver/gitlab/testdata/hook_skip_verification.json.golden index 746342595..b4ad9ca7c 100644 --- a/scm/driver/gitlab/testdata/hook_skip_verification.json.golden +++ b/scm/driver/gitlab/testdata/hook_skip_verification.json.golden @@ -3,11 +3,14 @@ "Name": "", "Target": "http://example.com/hook", "Events": [ - "issues", - "tag", - "push", - "comment", - "merge" + "issues_events", + "job_events", + "merge_requests_events", + "note_events", + "pipeline_events", + "push_events", + "tag_push_events", + "wiki_page_events" ], "Active": true, "SkipVerify": true diff --git a/scm/driver/gitlab/testdata/hooks.json.golden b/scm/driver/gitlab/testdata/hooks.json.golden index a90d275c3..741696194 100644 --- a/scm/driver/gitlab/testdata/hooks.json.golden +++ b/scm/driver/gitlab/testdata/hooks.json.golden @@ -4,11 +4,14 @@ "Name": "", "Target": "http://example.com/hook", "Events": [ - "issues", - "tag", - "push", - "comment", - "merge" + "issues_events", + "job_events", + "merge_requests_events", + "note_events", + "pipeline_events", + "push_events", + "tag_push_events", + "wiki_page_events" ], "Active": true, "SkipVerify": false