From 8fdba885448c102df30e658d803a13ee25ad8983 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Mon, 4 Dec 2023 17:51:37 +0000 Subject: [PATCH 1/6] Fix Hide Previous Plan Comments --- .../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 | 2 ++ server/events/vcs/gitlab_client.go | 7 ++++++- server/events/vcs/gitlab_client_test.go | 2 +- 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, 66 insertions(+), 25 deletions(-) diff --git a/server/controllers/events/events_controller.go b/server/controllers/events/events_controller.go index 10b8daf1f5..06b30f40a9 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 379cba7bb5..c2645cc571 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 5ff3f3ff38..1310b153a3 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 { +func (g *AzureDevopsClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { return nil } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index fa1751db19..58f95190ae 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -106,7 +106,7 @@ func (b *Client) ReactToComment(repo models.Repo, pullNum int, commentID int64, return nil } -func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir string) error { return nil } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 943fd3b6c8..401a2f746a 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -149,7 +149,7 @@ func (b *Client) ReactToComment(repo models.Repo, pullNum int, commentID int64, return nil } -func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir 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 ab075fba03..a4625b5034 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -205,7 +205,7 @@ func (g *GithubClient) ReactToComment(repo models.Repo, pullNum int, commentID i 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 { @@ -244,6 +244,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 { @@ -257,6 +263,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..286417ecd3 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)) @@ -314,6 +315,7 @@ func TestGithubClient_HideOldComments(t *testing.T) { }, 123, command.Plan.TitleString(), + "", ) Ok(t, err) Equals(t, 3, len(gotMinimizeCalls)) diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index b159e3d83b..03afb77d3b 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -194,7 +194,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 @@ -242,6 +242,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 112f35bcc5..19993315d8 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -591,7 +591,7 @@ func TestGitlabClient_HideOldComments(t *testing.T) { }, } - err = client.HidePrevCommandComments(repo, pullNum, command.Plan.TitleString()) + err = client.HidePrevCommandComments(repo, pullNum, command.Plan.TitleString(),"") Ok(t, err) // Check the correct number of plan comments have been processed diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go index e77d2b7980..6f564af994 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 4150e5ffeb..1b8b630c22 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 { @@ -555,8 +555,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} } @@ -566,12 +566,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)) @@ -586,6 +586,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 6ce6f2da56..7b0db234d4 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(repo models.Repo, pull models. func (a *NotConfiguredVCSClient) CreateComment(repo models.Repo, pullNum int, comment string, command string) error { return a.err() } -func (a *NotConfiguredVCSClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { +func (a *NotConfiguredVCSClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string, dir 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 { From 53600c89a7c1d16cb40988fbf6c22aaa1f655da3 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Thu, 7 Dec 2023 15:53:53 +0000 Subject: [PATCH 2/6] Update GitLab client tests --- server/events/vcs/gitlab_client_test.go | 163 ++++++++++++++---------- 1 file changed, 95 insertions(+), 68 deletions(-) diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 19993315d8..88c6fd30e7 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -509,7 +509,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 := "
" @@ -528,59 +528,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 - json.NewDecoder(r.Body).Decode(&body) - 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", @@ -591,21 +538,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) + 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) { From 3d8ac39b5fc36cb876a1e1e76b4ebf2f3c3cdef1 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Tue, 12 Dec 2023 23:39:54 +0000 Subject: [PATCH 3/6] Update GitLab client test --- server/events/vcs/gitlab_client_test.go | 54 ------------------------- 1 file changed, 54 deletions(-) diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 1cc03856fe..11955008cf 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -553,60 +553,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", From 1a3defe107aad4bb7a8cb229e27b1e611c13ae66 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Wed, 13 Dec 2023 12:03:56 +0000 Subject: [PATCH 4/6] Update github client test --- server/events/vcs/github_client_test.go | 171 ++++++++++++++---------- 1 file changed, 102 insertions(+), 69 deletions(-) diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 286417ecd3..acb7d07b10 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -241,87 +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) { From 3ca1be4545a87a02064bcdb839689410af32a0cc Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Wed, 13 Dec 2023 12:12:27 +0000 Subject: [PATCH 5/6] Add nolint: errcheck to test --- server/events/vcs/gitlab_client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 11955008cf..5e616f425d 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -617,7 +617,7 @@ func TestGitlabClient_HideOldComments(t *testing.T) { 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) + json.NewDecoder(r.Body).Decode(&body) // nolint: errcheck notePutCallDetail := notePutCallDetails{ noteID: path.Base(r.RequestURI), comment: strings.Split(body.Body, "\n"), From 8c9be0e87a7832cfcb9d5394d4f93e0603692820 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Wed, 13 Dec 2023 13:05:18 +0000 Subject: [PATCH 6/6] format github_client.go --- server/events/vcs/github_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 4511436d7e..1e1d51e200 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -249,7 +249,7 @@ func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, co if dir != "" && !strings.Contains(firstLine, strings.ToLower(dir)) { continue } - + var m struct { MinimizeComment struct { MinimizedComment struct {