From 0d94a3c788a6f483dbf379e1ff84c80a8b838f36 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Fri, 29 Dec 2023 18:15:17 +0000 Subject: [PATCH] fix: Atlantis Does Not Consider the Plan Directory When Hiding Previous Plan Comments (#4012) * Fix Hide Previous Plan Comments * Update GitLab client tests * Update GitLab client test * Update github client test * Add nolint: errcheck to test * format github_client.go --- .../controllers/events/events_controller.go | 10 +- server/events/event_parser.go | 18 ++ server/events/pull_updater.go | 3 +- server/events/vcs/azuredevops_client.go | 2 +- server/events/vcs/bitbucketcloud/client.go | 2 +- server/events/vcs/bitbucketserver/client.go | 2 +- server/events/vcs/client.go | 4 +- server/events/vcs/github_client.go | 9 +- server/events/vcs/github_client_test.go | 171 +++++++++++------- server/events/vcs/gitlab_client.go | 7 +- server/events/vcs/gitlab_client_test.go | 164 ++++++++++------- server/events/vcs/instrumented_client.go | 4 +- server/events/vcs/mocks/mock_client.go | 20 +- .../events/vcs/not_configured_vcs_client.go | 2 +- server/events/vcs/proxy.go | 4 +- 15 files changed, 261 insertions(+), 161 deletions(-) diff --git a/server/controllers/events/events_controller.go b/server/controllers/events/events_controller.go index af8b85bbc8..e1e5479328 100644 --- a/server/controllers/events/events_controller.go +++ b/server/controllers/events/events_controller.go @@ -586,9 +586,13 @@ func (e *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, b body: "Commenting back on pull request", } } - - logger.Info("Running comment command '%v' on repo '%v', pull request: %v for user '%v'.", - parseResult.Command.Name, baseRepo.FullName, pullNum, user.Username) + if parseResult.Command.RepoRelDir != "" { + logger.Info("Running comment command '%v' on dir '%v' on repo '%v', pull request: %v for user '%v'.", + parseResult.Command.Name, parseResult.Command.RepoRelDir, baseRepo.FullName, pullNum, user.Username) + } else { + logger.Info("Running comment command '%v' on repo '%v', pull request: %v for user '%v'.", + parseResult.Command.Name, baseRepo.FullName, pullNum, user.Username) + } if !e.TestingMode { // Respond with success and then actually execute the command asynchronously. // We use a goroutine so that this function returns and the connection is diff --git a/server/events/event_parser.go b/server/events/event_parser.go index b77249ad62..c00704c47c 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -39,6 +39,9 @@ var lastBitbucketSha, _ = lru.New[string, string](300) // PullCommand is a command to run on a pull request. type PullCommand interface { + // Dir is the path relative to the repo root to run the command in. + // Will never end in "/". If empty then the comment specified no directory. + Dir() string // CommandName is the name of the command we're running. CommandName() command.Name // SubCommandName is the subcommand name of the command we're running. @@ -63,6 +66,11 @@ func (c PolicyCheckCommand) SubCommandName() string { return "" } +// Dir is empty +func (c PolicyCheckCommand) Dir() string { + return "" +} + // IsVerbose is false for policy_check commands. func (c PolicyCheckCommand) IsVerbose() bool { return false @@ -87,6 +95,11 @@ func (c AutoplanCommand) SubCommandName() string { return "" } +// Dir is empty +func (c AutoplanCommand) Dir() string { + return "" +} + // IsVerbose is false for autoplan commands. func (c AutoplanCommand) IsVerbose() bool { return false @@ -133,6 +146,11 @@ func (c CommentCommand) IsForSpecificProject() bool { return c.RepoRelDir != "" || c.Workspace != "" || c.ProjectName != "" } +// Dir returns the dir of this command. +func (c CommentCommand) Dir() string { + return c.RepoRelDir +} + // CommandName returns the name of this command. func (c CommentCommand) CommandName() command.Name { return c.Name diff --git a/server/events/pull_updater.go b/server/events/pull_updater.go index b6129cda82..d8fcfe34f9 100644 --- a/server/events/pull_updater.go +++ b/server/events/pull_updater.go @@ -23,7 +23,8 @@ func (c *PullUpdater) updatePull(ctx *command.Context, cmd PullCommand, res comm // clutter in a pull/merge request. This will not delete the comment, since the // comment trail may be useful in auditing or backtracing problems. if c.HidePrevPlanComments { - if err := c.VCSClient.HidePrevCommandComments(ctx.Pull.BaseRepo, ctx.Pull.Num, cmd.CommandName().TitleString()); err != nil { + ctx.Log.Debug("Hiding previous plan comments for command: '%v', directory: '%v'", cmd.CommandName().TitleString(), cmd.Dir()) + if err := c.VCSClient.HidePrevCommandComments(ctx.Pull.BaseRepo, ctx.Pull.Num, cmd.CommandName().TitleString(), cmd.Dir()); err != nil { ctx.Log.Err("unable to hide old comments: %s", err) } } diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 429380d3f8..c89d490005 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -134,7 +134,7 @@ func (g *AzureDevopsClient) ReactToComment(repo models.Repo, pullNum int, commen return nil } -func (g *AzureDevopsClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { //nolint: revive +func (g *AzureDevopsClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { //nolint: revive return nil } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 13462a7a51..a9b38b4d09 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -106,7 +106,7 @@ func (b *Client) ReactToComment(_ models.Repo, _ int, _ int64, _ string) error { return nil } -func (b *Client) HidePrevCommandComments(_ models.Repo, _ int, _ string) error { +func (b *Client) HidePrevCommandComments(_ models.Repo, _ int, _ string, _ string) error { return nil } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 9012c35e58..b6c75f3197 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -149,7 +149,7 @@ func (b *Client) ReactToComment(_ models.Repo, _ int, _ int64, _ string) error { return nil } -func (b *Client) HidePrevCommandComments(_ models.Repo, _ int, _ string) error { +func (b *Client) HidePrevCommandComments(_ models.Repo, _ int, _ string, _ string) error { return nil } diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index dd2e489f6f..9bcf972f6a 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -17,7 +17,7 @@ import ( "github.com/runatlantis/atlantis/server/events/models" ) -//go:generate pegomock generate --package mocks -o mocks/mock_client.go Client +//go:generate pegomock generate --package mocks -o mocks/mock_client.go github.com/runatlantis/atlantis/server/events/vcs Client // Client is used to make API calls to a VCS host like GitHub or GitLab. type Client interface { @@ -27,7 +27,7 @@ type Client interface { CreateComment(repo models.Repo, pullNum int, comment string, command string) error ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error - HidePrevCommandComments(repo models.Repo, pullNum int, command string) error + HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) // UpdateStatus updates the commit status to state for pull. src is the diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 5a5a9d6b20..4bed3e64e4 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -211,7 +211,7 @@ func (g *GithubClient) ReactToComment(repo models.Repo, _ int, commentID int64, return err } -func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { var allComments []*github.IssueComment nextPage := 0 for { @@ -252,6 +252,12 @@ func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, co if !strings.Contains(firstLine, strings.ToLower(command)) { continue } + + // If dir was specified, skip processing comments that don't contain the dir in the first line + if dir != "" && !strings.Contains(firstLine, strings.ToLower(dir)) { + continue + } + var m struct { MinimizeComment struct { MinimizedComment struct { @@ -265,6 +271,7 @@ func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, co Classifier: githubv4.ReportedContentClassifiersOutdated, SubjectID: comment.GetNodeID(), } + g.logger.Debug("Hiding comment %s", comment.GetNodeID()) if err := g.v4Client.Mutate(g.ctx, &m, input, nil); err != nil { return errors.Wrapf(err, "minimize comment %s", comment.GetNodeID()) } diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 4f2f8fd3e7..acb7d07b10 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -230,6 +230,7 @@ func TestGithubClient_PaginatesComments(t *testing.T) { }, 123, command.Plan.TitleString(), + "", ) Ok(t, err) Equals(t, 2, len(gotMinimizeCalls)) @@ -240,86 +241,120 @@ func TestGithubClient_PaginatesComments(t *testing.T) { } func TestGithubClient_HideOldComments(t *testing.T) { - // Only comment 6 should be minimized, because it's by the same Atlantis bot user - // and it has "plan" in the first line of the comment body. - issueResp := `[ + atlantisUser := "AtlantisUser" + pullRequestNum := 123 + issueResp := strings.ReplaceAll(`[ {"node_id": "1", "body": "asd\nplan\nasd", "user": {"login": "someone-else"}}, {"node_id": "2", "body": "asd plan\nasd", "user": {"login": "someone-else"}}, {"node_id": "3", "body": "asdasdasd\nasdasdasd", "user": {"login": "someone-else"}}, - {"node_id": "4", "body": "asdasdasd\nasdasdasd", "user": {"login": "user"}}, - {"node_id": "5", "body": "asd\nplan\nasd", "user": {"login": "user"}}, - {"node_id": "6", "body": "asd plan\nasd", "user": {"login": "user"}}, - {"node_id": "7", "body": "asdasdasd", "user": {"login": "user"}}, - {"node_id": "8", "body": "asd plan\nasd", "user": {"login": "user"}}, - {"node_id": "9", "body": "Continued Plan from previous comment\nasd", "user": {"login": "user"}} -]` + {"node_id": "4", "body": "asdasdasd\nasdasdasd", "user": {"login": "AtlantisUser"}}, + {"node_id": "5", "body": "asd\nplan\nasd", "user": {"login": "AtlantisUser"}}, + {"node_id": "6", "body": "Ran Plan for 2 projects:", "user": {"login": "AtlantisUser"}}, + {"node_id": "7", "body": "Ran Apply for 2 projects:", "user": {"login": "AtlantisUser"}}, + {"node_id": "8", "body": "Ran Plan for dir: 'stack1' workspace: 'default'", "user": {"login": "AtlantisUser"}}, + {"node_id": "9", "body": "Ran Plan for dir: 'stack2' workspace: 'default'", "user": {"login": "AtlantisUser"}}, + {"node_id": "10", "body": "Continued Plan from previous comment\nasd", "user": {"login": "AtlantisUser"}} +]`, "'", "`") minimizeResp := "{}" type graphQLCall struct { Variables struct { Input githubv4.MinimizeCommentInput `json:"input"` } `json:"variables"` } - gotMinimizeCalls := make([]graphQLCall, 0, 1) - testServer := httptest.NewTLSServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.Method + " " + r.RequestURI { - // This gets the pull request's comments. - case "GET /api/v3/repos/owner/repo/issues/123/comments?direction=asc&sort=created": - w.Write([]byte(issueResp)) // nolint: errcheck - return - case "POST /api/graphql": - defer r.Body.Close() // nolint: errcheck - body, err := io.ReadAll(r.Body) - if err != nil { - t.Errorf("read body error: %v", err) - http.Error(w, "server error", http.StatusInternalServerError) - return - } - call := graphQLCall{} - err = json.Unmarshal(body, &call) - if err != nil { - t.Errorf("parse body error: %v", err) - http.Error(w, "server error", http.StatusInternalServerError) - return - } - gotMinimizeCalls = append(gotMinimizeCalls, call) - w.Write([]byte(minimizeResp)) // nolint: errcheck - return - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - return - } - }), - ) - testServerURL, err := url.Parse(testServer.URL) - Ok(t, err) + cases := []struct { + dir string + processedComments int + processedCommentIds []string + }{ + { + // With no dir specified, comments 6, 8, 9 and 10 should be minimized. + "", + 4, + []string{"6", "8", "9", "10"}, + }, + { + // With a dir of "stack1", comment 8 should be minimized. + "stack1", + 1, + []string{"8"}, + }, + { + // With a dir of "stack2", comment 9 should be minimized. + "stack2", + 1, + []string{"9"}, + }, + } - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) - Ok(t, err) - defer disableSSLVerification()() + for _, c := range cases { + t.Run(c.dir, func(t *testing.T) { + gotMinimizeCalls := make([]graphQLCall, 0, 1) + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.RequestURI { + // This gets the pull request's comments. + case fmt.Sprintf("GET /api/v3/repos/owner/repo/issues/%v/comments?direction=asc&sort=created", pullRequestNum): + w.Write([]byte(issueResp)) // nolint: errcheck + return + case "POST /api/graphql": + defer r.Body.Close() // nolint: errcheck + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read body error: %v", err) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + call := graphQLCall{} + err = json.Unmarshal(body, &call) + if err != nil { + t.Errorf("parse body error: %v", err) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + gotMinimizeCalls = append(gotMinimizeCalls, call) + w.Write([]byte(minimizeResp)) // nolint: errcheck + return + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + }), + ) - err = client.HidePrevCommandComments( - models.Repo{ - FullName: "owner/repo", - Owner: "owner", - Name: "repo", - CloneURL: "", - SanitizedCloneURL: "", - VCSHost: models.VCSHost{ - Hostname: "github.com", - Type: models.Github, - }, - }, - 123, - command.Plan.TitleString(), - ) - Ok(t, err) - Equals(t, 3, len(gotMinimizeCalls)) - Equals(t, "6", gotMinimizeCalls[0].Variables.Input.SubjectID) - Equals(t, "9", gotMinimizeCalls[2].Variables.Input.SubjectID) - Equals(t, githubv4.ReportedContentClassifiersOutdated, gotMinimizeCalls[0].Variables.Input.Classifier) + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{atlantisUser, "pass"}, vcs.GithubConfig{}, + logging.NewNoopLogger(t)) + Ok(t, err) + defer disableSSLVerification()() + + err = client.HidePrevCommandComments( + models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Hostname: "github.com", + Type: models.Github, + }, + }, + pullRequestNum, + command.Plan.TitleString(), + c.dir, + ) + Ok(t, err) + Equals(t, c.processedComments, len(gotMinimizeCalls)) + for i := 0; i < c.processedComments; i++ { + Equals(t, c.processedCommentIds[i], gotMinimizeCalls[i].Variables.Input.SubjectID) + Equals(t, githubv4.ReportedContentClassifiersOutdated, gotMinimizeCalls[i].Variables.Input.Classifier) + } + }) + } } func TestGithubClient_UpdateStatus(t *testing.T) { diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 60ef6bd1f2..d9e2b4d33c 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -200,7 +200,7 @@ func (g *GitlabClient) ReactToComment(repo models.Repo, pullNum int, commentID i return err } -func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { var allComments []*gitlab.Note nextPage := 0 @@ -250,6 +250,11 @@ func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, co continue } + // If dir was specified, skip processing comments that don't contain the dir in the first line + if dir != "" && !strings.Contains(firstLine, strings.ToLower(dir)) { + continue + } + g.logger.Debug("Updating merge request note: Repo: '%s', MR: '%d', comment ID: '%d'", repo.FullName, pullNum, comment.ID) supersededComment := summaryHeader + lineFeed + comment.Body + lineFeed + summaryFooter + lineFeed diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index cbe2f4b5bc..99cf6b426e 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -566,7 +566,7 @@ func TestGitlabClient_HideOldComments(t *testing.T) { planCommentIDs := [2]string{"3", "5"} systemCommentIDs := [1]string{"4"} summaryCommentIDs := [1]string{"2"} - planComments := [3]string{"plan comment 1", "plan comment 2", "plan comment 3"} + planComments := [3]string{"Ran Plan for 2 projects:", "Ran Plan for dir: `stack1` workspace: `default`", "Ran Plan for 2 projects:"} summaryHeader := fmt.Sprintf("
Superseded Atlantis %s", command.Plan.TitleString()) summaryFooter := "
" @@ -585,60 +585,6 @@ func TestGitlabClient_HideOldComments(t *testing.T) { planCommentIDs[1], planComments[1], authorID, authorUserName, authorEmail, pullNum) + "]" - gitlabClientUnderTest = true - defer func() { gitlabClientUnderTest = false }() - gotNotePutCalls := make([]notePutCallDetails, 0, 1) - testServer := httptest.NewServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.Method { - case "GET": - switch r.RequestURI { - case "/api/v4/user": - w.WriteHeader(http.StatusOK) - w.Header().Set("Content-Type", "application/json") - response := fmt.Sprintf(`{"id": %d,"username": "%s", "email": "%s"}`, authorID, authorUserName, authorEmail) - w.Write([]byte(response)) // nolint: errcheck - case fmt.Sprintf("/api/v4/projects/runatlantis%%2Fatlantis/merge_requests/%d/notes?order_by=created_at&sort=asc", pullNum): - w.WriteHeader(http.StatusOK) - response := issueResp - w.Write([]byte(response)) // nolint: errcheck - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - } - case "PUT": - switch { - case strings.HasPrefix(r.RequestURI, fmt.Sprintf("/api/v4/projects/runatlantis%%2Fatlantis/merge_requests/%d/notes/", pullNum)): - w.WriteHeader(http.StatusOK) - var body jsonBody - err := json.NewDecoder(r.Body).Decode(&body) - Ok(t, err) - notePutCallDetail := notePutCallDetails{ - noteID: path.Base(r.RequestURI), - comment: strings.Split(body.Body, "\n"), - } - gotNotePutCalls = append(gotNotePutCalls, notePutCallDetail) - response := "{}" - w.Write([]byte(response)) // nolint: errcheck - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - } - default: - t.Errorf("got unexpected method at %q", r.Method) - http.Error(w, "not found", http.StatusNotFound) - } - }), - ) - - internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) - Ok(t, err) - client := &GitlabClient{ - Client: internalClient, - Version: nil, - logger: logging.NewNoopLogger(t), - } - repo := models.Repo{ FullName: "runatlantis/atlantis", Owner: "runatlantis", @@ -649,21 +595,101 @@ func TestGitlabClient_HideOldComments(t *testing.T) { }, } - err = client.HidePrevCommandComments(repo, pullNum, command.Plan.TitleString()) - Ok(t, err) + cases := []struct { + dir string + processedComments int + processedCommentIds []string + processedPlanComment []string + }{ + { + "", + 2, + []string{planCommentIDs[0], planCommentIDs[1]}, + []string{planComments[0], planComments[1]}, + }, + { + "stack1", + 1, + []string{planCommentIDs[1]}, + []string{planComments[1]}, + }, + { + "stack2", + 0, + []string{}, + []string{}, + }, + } - // Check the correct number of plan comments have been processed - Equals(t, len(planCommentIDs), len(gotNotePutCalls)) - // Check the first plan comment has been currectly summarised - Equals(t, planCommentIDs[0], gotNotePutCalls[0].noteID) - Equals(t, summaryHeader, gotNotePutCalls[0].comment[0]) - Equals(t, planComments[0], gotNotePutCalls[0].comment[1]) - Equals(t, summaryFooter, gotNotePutCalls[0].comment[2]) - // Check the second plan comment has been currectly summarised - Equals(t, planCommentIDs[1], gotNotePutCalls[1].noteID) - Equals(t, summaryHeader, gotNotePutCalls[1].comment[0]) - Equals(t, planComments[1], gotNotePutCalls[1].comment[1]) - Equals(t, summaryFooter, gotNotePutCalls[1].comment[2]) + for _, c := range cases { + t.Run(c.dir, func(t *testing.T) { + gitlabClientUnderTest = true + defer func() { gitlabClientUnderTest = false }() + gotNotePutCalls := make([]notePutCallDetails, 0, 1) + testServer := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "GET": + switch r.RequestURI { + case "/api/v4/user": + w.WriteHeader(http.StatusOK) + w.Header().Set("Content-Type", "application/json") + response := fmt.Sprintf(`{"id": %d,"username": "%s", "email": "%s"}`, authorID, authorUserName, authorEmail) + w.Write([]byte(response)) // nolint: errcheck + case fmt.Sprintf("/api/v4/projects/runatlantis%%2Fatlantis/merge_requests/%d/notes?order_by=created_at&sort=asc", pullNum): + w.WriteHeader(http.StatusOK) + response := issueResp + w.Write([]byte(response)) // nolint: errcheck + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + } + case "PUT": + switch { + case strings.HasPrefix(r.RequestURI, fmt.Sprintf("/api/v4/projects/runatlantis%%2Fatlantis/merge_requests/%d/notes/", pullNum)): + w.WriteHeader(http.StatusOK) + var body jsonBody + json.NewDecoder(r.Body).Decode(&body) // nolint: errcheck + notePutCallDetail := notePutCallDetails{ + noteID: path.Base(r.RequestURI), + comment: strings.Split(body.Body, "\n"), + } + gotNotePutCalls = append(gotNotePutCalls, notePutCallDetail) + response := "{}" + w.Write([]byte(response)) // nolint: errcheck + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + } + default: + t.Errorf("got unexpected method at %q", r.Method) + http.Error(w, "not found", http.StatusNotFound) + } + }), + ) + + internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) + Ok(t, err) + client := &GitlabClient{ + Client: internalClient, + Version: nil, + logger: logging.NewNoopLogger(t), + } + + err = client.HidePrevCommandComments(repo, pullNum, command.Plan.TitleString(), c.dir) + Ok(t, err) + + // Check the correct number of plan comments have been processed + Equals(t, c.processedComments, len(gotNotePutCalls)) + // Check the correct comments have been processed + for i := 0; i < c.processedComments; i++ { + Equals(t, c.processedCommentIds[i], gotNotePutCalls[i].noteID) + Equals(t, summaryHeader, gotNotePutCalls[i].comment[0]) + Equals(t, c.processedPlanComment[i], gotNotePutCalls[i].comment[1]) + Equals(t, summaryFooter, gotNotePutCalls[i].comment[2]) + } + }) + } } func TestGithubClient_GetPullLabels(t *testing.T) { diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go index 69978b9468..512d36f8a0 100644 --- a/server/events/vcs/instrumented_client.go +++ b/server/events/vcs/instrumented_client.go @@ -147,7 +147,7 @@ func (c *InstrumentedClient) ReactToComment(repo models.Repo, pullNum int, comme return nil } -func (c *InstrumentedClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (c *InstrumentedClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { scope := c.StatsScope.SubScope("hide_prev_plan_comments") scope = SetGitScopeTags(scope, repo.FullName, pullNum) logger := c.Logger.WithHistory(fmtLogSrc(repo, pullNum)...) @@ -158,7 +158,7 @@ func (c *InstrumentedClient) HidePrevCommandComments(repo models.Repo, pullNum i executionSuccess := scope.Counter(metrics.ExecutionSuccessMetric) executionError := scope.Counter(metrics.ExecutionErrorMetric) - if err := c.Client.HidePrevCommandComments(repo, pullNum, command); err != nil { + if err := c.Client.HidePrevCommandComments(repo, pullNum, command, dir); err != nil { executionError.Inc(1) logger.Err("Unable to hide previous %s comments, error: %s", command, err.Error()) return err diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 7583e22fac..5b0bb3051a 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -154,11 +154,11 @@ func (mock *MockClient) GetTeamNamesForUser(repo models.Repo, user models.User) return ret0, ret1 } -func (mock *MockClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (mock *MockClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } - params := []pegomock.Param{repo, pullNum, command} + params := []pegomock.Param{repo, pullNum, command, dir} result := pegomock.GetGenericMockFrom(mock).Invoke("HidePrevCommandComments", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -548,8 +548,8 @@ func (c *MockClient_GetTeamNamesForUser_OngoingVerification) GetAllCapturedArgum return } -func (verifier *VerifierMockClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) *MockClient_HidePrevCommandComments_OngoingVerification { - params := []pegomock.Param{repo, pullNum, command} +func (verifier *VerifierMockClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) *MockClient_HidePrevCommandComments_OngoingVerification { + params := []pegomock.Param{repo, pullNum, command, dir} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "HidePrevCommandComments", params, verifier.timeout) return &MockClient_HidePrevCommandComments_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -559,12 +559,12 @@ type MockClient_HidePrevCommandComments_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockClient_HidePrevCommandComments_OngoingVerification) GetCapturedArguments() (models.Repo, int, string) { - repo, pullNum, command := c.GetAllCapturedArguments() - return repo[len(repo)-1], pullNum[len(pullNum)-1], command[len(command)-1] +func (c *MockClient_HidePrevCommandComments_OngoingVerification) GetCapturedArguments() (models.Repo, int, string, string) { + repo, pullNum, command, dir := c.GetAllCapturedArguments() + return repo[len(repo)-1], pullNum[len(pullNum)-1], command[len(command)-1], dir[len(dir)-1] } -func (c *MockClient_HidePrevCommandComments_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []int, _param2 []string) { +func (c *MockClient_HidePrevCommandComments_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []int, _param2 []string, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.Repo, len(c.methodInvocations)) @@ -579,6 +579,10 @@ func (c *MockClient_HidePrevCommandComments_OngoingVerification) GetAllCapturedA for u, param := range params[2] { _param2[u] = param.(string) } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) + } } return } diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index c916195f3b..b7eed9e900 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -32,7 +32,7 @@ func (a *NotConfiguredVCSClient) GetModifiedFiles(_ models.Repo, _ models.PullRe func (a *NotConfiguredVCSClient) CreateComment(_ models.Repo, _ int, _ string, _ string) error { return a.err() } -func (a *NotConfiguredVCSClient) HidePrevCommandComments(_ models.Repo, _ int, _ string) error { +func (a *NotConfiguredVCSClient) HidePrevCommandComments(_ models.Repo, _ int, _ string, _ string) error { return nil } func (a *NotConfiguredVCSClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { // nolint: revive diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 25637bcd0f..768b0b6255 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -60,8 +60,8 @@ func (d *ClientProxy) CreateComment(repo models.Repo, pullNum int, comment strin return d.clients[repo.VCSHost.Type].CreateComment(repo, pullNum, comment, command) } -func (d *ClientProxy) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { - return d.clients[repo.VCSHost.Type].HidePrevCommandComments(repo, pullNum, command) +func (d *ClientProxy) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { + return d.clients[repo.VCSHost.Type].HidePrevCommandComments(repo, pullNum, command, dir) } func (d *ClientProxy) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error {