From f8b293ada054e413e3fc0b39eefa5173f46aad62 Mon Sep 17 00:00:00 2001 From: Marcus Ramberg Date: Tue, 25 Apr 2023 23:22:51 +0200 Subject: [PATCH] feat: Github reaction emojis on PR comments (#2706) * feat: Basic implementation of github reactions on PRs Adds eyes whenever it detects an `atlantis` command. * feat: Make the emoji reaction configurable * tests: Add a mocked test for EmojiReaction being called in github * ci: Disable revive linter for stubs --- cmd/server.go | 11 +++- .../controllers/events/events_controller.go | 27 +++++++--- .../events/events_controller_test.go | 21 ++++++++ server/core/config/raw/repo_cfg.go | 10 ++++ server/core/config/valid/repo_cfg.go | 1 + server/events/vcs/azuredevops_client.go | 4 ++ server/events/vcs/bitbucketcloud/client.go | 6 +++ server/events/vcs/bitbucketserver/client.go | 4 ++ server/events/vcs/client.go | 2 + server/events/vcs/github_client.go | 9 +++- server/events/vcs/gitlab_client.go | 4 ++ server/events/vcs/instrumented_client.go | 28 +++++++++-- server/events/vcs/mocks/mock_client.go | 50 +++++++++++++++++++ .../events/vcs/not_configured_vcs_client.go | 3 ++ server/events/vcs/proxy.go | 4 ++ server/server.go | 2 + server/user_config.go | 1 + 17 files changed, 173 insertions(+), 14 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 4c974ce2b..f8f92c5cb 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -67,6 +67,7 @@ const ( DisableMarkdownFoldingFlag = "disable-markdown-folding" DisableRepoLockingFlag = "disable-repo-locking" DiscardApprovalOnPlanFlag = "discard-approval-on-plan" + EmojiReaction = "emoji-reaction" EnablePolicyChecksFlag = "enable-policy-checks" EnableRegExpCmdFlag = "enable-regexp-cmd" EnableDiffMarkdownFormat = "enable-diff-markdown-format" @@ -144,6 +145,7 @@ const ( DefaultCheckoutDepth = 0 DefaultBitbucketBaseURL = bitbucketcloud.BaseURL DefaultDataDir = "~/.atlantis" + DefaultEmojiReaction = "eyes" DefaultExecutableName = "atlantis" DefaultMarkdownTemplateOverridesDir = "~/.markdown_templates" DefaultGHHostname = "github.com" @@ -244,6 +246,10 @@ var stringFlags = map[string]stringFlag{ description: "Path to directory to store Atlantis data.", defaultValue: DefaultDataDir, }, + EmojiReaction: { + description: "Emoji Reaction to use to react to comments", + defaultValue: DefaultEmojiReaction, + }, ExecutableName: { description: "Comment command executable name.", defaultValue: DefaultExecutableName, @@ -650,7 +656,7 @@ func (s *ServerCmd) Init() *cobra.Command { c.SetUsageTemplate(usageTmpl(stringFlags, intFlags, boolFlags)) // If a user passes in an invalid flag, tell them what the flag was. - c.SetFlagErrorFunc(func(c *cobra.Command, err error) error { + c.SetFlagErrorFunc(func(_ *cobra.Command, err error) error { s.printErr(err) return err }) @@ -792,6 +798,9 @@ func (s *ServerCmd) setDefaults(c *server.UserConfig) { if c.BitbucketBaseURL == "" { c.BitbucketBaseURL = DefaultBitbucketBaseURL } + if c.EmojiReaction == "" { + c.EmojiReaction = DefaultEmojiReaction + } if c.ExecutableName == "" { c.ExecutableName = DefaultExecutableName } diff --git a/server/controllers/events/events_controller.go b/server/controllers/events/events_controller.go index d45e4e76d..77fa3bd42 100644 --- a/server/controllers/events/events_controller.go +++ b/server/controllers/events/events_controller.go @@ -49,13 +49,15 @@ const azuredevopsTestURL = "https://fabrikam.visualstudio.com/DefaultCollection/ // VCSEventsController handles all webhook requests which signify 'events' in the // VCS host, ex. GitHub. type VCSEventsController struct { - CommandRunner events.CommandRunner - PullCleaner events.PullCleaner - Logger logging.SimpleLogging - Scope tally.Scope - Parser events.EventParsing - CommentParser events.CommentParsing - ApplyDisabled bool + CommandRunner events.CommandRunner + PullCleaner events.PullCleaner + Logger logging.SimpleLogging + Scope tally.Scope + Parser events.EventParsing + CommentParser events.CommentParsing + ApplyDisabled bool + EmojiReaction string + ExecutableName string // GithubWebhookSecret is the secret added to this webhook via the GitHub // UI that identifies this call as coming from GitHub. If empty, no // request validation is done. @@ -309,9 +311,18 @@ func (e *VCSEventsController) HandleGithubCommentEvent(event *github.IssueCommen } } + body := event.GetComment().GetBody() + + if strings.HasPrefix(body, e.ExecutableName+" ") { + err = e.VCSClient.ReactToComment(baseRepo, *event.Comment.ID, e.EmojiReaction) + if err != nil { + logger.Warn("Failed to react to comment: %s", err) + } + } + // We pass in nil for maybeHeadRepo because the head repo data isn't // available in the GithubIssueComment event. - return e.handleCommentEvent(logger, baseRepo, nil, nil, user, pullNum, event.Comment.GetBody(), models.Github) + return e.handleCommentEvent(logger, baseRepo, nil, nil, user, pullNum, body, models.Github) } // HandleBitbucketCloudCommentEvent handles comment events from Bitbucket. diff --git a/server/controllers/events/events_controller_test.go b/server/controllers/events/events_controller_test.go index 87e479029..ee1d455f5 100644 --- a/server/controllers/events/events_controller_test.go +++ b/server/controllers/events/events_controller_test.go @@ -381,6 +381,25 @@ func TestPost_GithubCommentSuccess(t *testing.T) { cr.VerifyWasCalledOnce().RunCommentCommand(baseRepo, nil, nil, user, 1, &cmd) } +func TestPost_GithubCommentReaction(t *testing.T) { + t.Log("when the event is a github comment with a valid command we call the command handler") + e, v, _, _, p, _, _, vcsClient, cp := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(githubHeader, "issue_comment") + event := `{"action": "created", "comment": {"body": "atlantis help", "id": 1}}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + baseRepo := models.Repo{} + user := models.User{} + cmd := events.CommentCommand{} + When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil) + When(cp.Parse("", models.Github)).ThenReturn(events.CommentParseResult{Command: &cmd}) + w := httptest.NewRecorder() + e.Post(w, req) + ResponseContains(t, w, http.StatusOK, "Processing...") + + vcsClient.VerifyWasCalledOnce().ReactToComment(baseRepo, 1, "eyes") +} + func TestPost_GithubPullRequestInvalid(t *testing.T) { t.Log("when the event is a github pull request with invalid data we return a 400") e, v, _, _, p, _, _, _, _ := setup(t) @@ -922,6 +941,8 @@ func setup(t *testing.T) (events_controllers.VCSEventsController, *mocks.MockGit logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "null") e := events_controllers.VCSEventsController{ + ExecutableName: "atlantis", + EmojiReaction: "eyes", TestingMode: true, Logger: logger, Scope: scope, diff --git a/server/core/config/raw/repo_cfg.go b/server/core/config/raw/repo_cfg.go index ef91fc96e..5c1b46391 100644 --- a/server/core/config/raw/repo_cfg.go +++ b/server/core/config/raw/repo_cfg.go @@ -22,6 +22,9 @@ const DefaultParallelPolicyCheck = false // DefaultDeleteSourceBranchOnMerge being false is the default setting whether or not to remove a source branch on merge const DefaultDeleteSourceBranchOnMerge = false +// DefaultEmojiReaction is the default emoji reaction for repos +const DefaultEmojiReaction = "" + // RepoCfg is the raw schema for repo-level atlantis.yaml config. type RepoCfg struct { Version *int `yaml:"version,omitempty"` @@ -32,6 +35,7 @@ type RepoCfg struct { ParallelApply *bool `yaml:"parallel_apply,omitempty"` ParallelPlan *bool `yaml:"parallel_plan,omitempty"` DeleteSourceBranchOnMerge *bool `yaml:"delete_source_branch_on_merge,omitempty"` + EmojiReaction *string `yaml:"emoji_reaction,omitempty"` AllowedRegexpPrefixes []string `yaml:"allowed_regexp_prefixes,omitempty"` } @@ -79,6 +83,11 @@ func (r RepoCfg) ToValid() valid.RepoCfg { parallelPlan = *r.ParallelPlan } + emojiReaction := DefaultEmojiReaction + if r.EmojiReaction != nil { + emojiReaction = *r.EmojiReaction + } + return valid.RepoCfg{ Version: *r.Version, Projects: validProjects, @@ -89,5 +98,6 @@ func (r RepoCfg) ToValid() valid.RepoCfg { ParallelPolicyCheck: parallelPlan, DeleteSourceBranchOnMerge: r.DeleteSourceBranchOnMerge, AllowedRegexpPrefixes: r.AllowedRegexpPrefixes, + EmojiReaction: emojiReaction, } } diff --git a/server/core/config/valid/repo_cfg.go b/server/core/config/valid/repo_cfg.go index 4c75da2b5..06391b4c5 100644 --- a/server/core/config/valid/repo_cfg.go +++ b/server/core/config/valid/repo_cfg.go @@ -24,6 +24,7 @@ type RepoCfg struct { ParallelPolicyCheck bool DeleteSourceBranchOnMerge *bool RepoLocking *bool + EmojiReaction string AllowedRegexpPrefixes []string } diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index e12cd078b..5a5fa6248 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -130,6 +130,10 @@ func (g *AzureDevopsClient) CreateComment(repo models.Repo, pullNum int, comment return nil } +func (g *AzureDevopsClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { //nolint: revive + return nil +} + func (g *AzureDevopsClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { return nil } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 74c7512c8..977c68905 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -100,6 +100,12 @@ func (b *Client) CreateComment(repo models.Repo, pullNum int, comment string, co return err } +// UpdateComment updates the body of a comment on the merge request. +func (b *Client) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint revive + // TODO: Bitbucket support for reactions + return nil +} + func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { return nil } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index b7fb63c1f..dabbad52b 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -145,6 +145,10 @@ func (b *Client) CreateComment(repo models.Repo, pullNum int, comment string, co return nil } +func (b *Client) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint: revive + return nil +} + func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { return nil } diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index e08d4583a..d2b30bfea 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -25,6 +25,8 @@ type Client interface { // relative to the repo root, e.g. parent/child/file.txt. GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) CreateComment(repo models.Repo, pullNum int, comment string, command string) error + + ReactToComment(repo models.Repo, commentID int64, reaction string) error HidePrevCommandComments(repo models.Repo, pullNum int, command string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index b9fac0c85..1523628b7 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -198,6 +198,13 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri return nil } +// ReactToComment adds a reaction to a comment. +func (g *GithubClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { + g.logger.Debug("POST /repos/%v/%v/issues/comments/%d/reactions", repo.Owner, repo.Name, commentID) + _, _, err := g.client.Reactions.CreateIssueCommentReaction(g.ctx, repo.Owner, repo.Name, commentID, reaction) + return err +} + func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { var allComments []*github.IssueComment nextPage := 0 @@ -395,7 +402,7 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu return false, errors.Wrap(err, "getting combined status") } - //iterate over statuses - return false if we find one that isnt "apply" and doesnt have state = "success" + //iterate over statuses - return false if we find one that isn't "apply" and doesn't have state = "success" for _, r := range status.Statuses { if strings.HasPrefix(*r.Context, fmt.Sprintf("%s/%s", vcstatusname, command.Apply.String())) { continue diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 6c597187b..c24e0ad35 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -180,6 +180,10 @@ func (g *GitlabClient) CreateComment(repo models.Repo, pullNum int, comment stri return nil } +func (g *GitlabClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint: revive + return nil +} + func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { return nil } diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go index 436af3682..b23550151 100644 --- a/server/events/vcs/instrumented_client.go +++ b/server/events/vcs/instrumented_client.go @@ -105,8 +105,8 @@ func (c *InstrumentedClient) GetModifiedFiles(repo models.Repo, pull models.Pull } return files, err - } + func (c *InstrumentedClient) CreateComment(repo models.Repo, pullNum int, comment string, command string) error { scope := c.StatsScope.SubScope("create_comment") scope = SetGitScopeTags(scope, repo.FullName, pullNum) @@ -127,6 +127,26 @@ func (c *InstrumentedClient) CreateComment(repo models.Repo, pullNum int, commen executionSuccess.Inc(1) return nil } + +func (c *InstrumentedClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { + scope := c.StatsScope.SubScope("react_to_comment") + + executionTime := scope.Timer(metrics.ExecutionTimeMetric).Start() + defer executionTime.Stop() + + executionSuccess := scope.Counter(metrics.ExecutionSuccessMetric) + executionError := scope.Counter(metrics.ExecutionErrorMetric) + + if err := c.Client.ReactToComment(repo, commentID, reaction); err != nil { + executionError.Inc(1) + c.Logger.Err("Unable to react to comment, error: %s", err.Error()) + return err + } + + executionSuccess.Inc(1) + return nil +} + func (c *InstrumentedClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { scope := c.StatsScope.SubScope("hide_prev_plan_comments") scope = SetGitScopeTags(scope, repo.FullName, pullNum) @@ -148,6 +168,7 @@ func (c *InstrumentedClient) HidePrevCommandComments(repo models.Repo, pullNum i return nil } + func (c *InstrumentedClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) { scope := c.StatsScope.SubScope("pull_is_approved") scope = SetGitScopeTags(scope, repo.FullName, pull.Num) @@ -169,8 +190,8 @@ func (c *InstrumentedClient) PullIsApproved(repo models.Repo, pull models.PullRe } return approved, err - } + func (c *InstrumentedClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) { scope := c.StatsScope.SubScope("pull_is_mergeable") scope = SetGitScopeTags(scope, repo.FullName, pull.Num) @@ -213,8 +234,8 @@ func (c *InstrumentedClient) UpdateStatus(repo models.Repo, pull models.PullRequ executionSuccess.Inc(1) return nil - } + func (c *InstrumentedClient) MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error { scope := c.StatsScope.SubScope("merge_pull") scope = SetGitScopeTags(scope, pull.BaseRepo.FullName, pull.Num) @@ -233,7 +254,6 @@ func (c *InstrumentedClient) MergePull(pull models.PullRequest, pullOptions mode executionSuccess.Inc(1) return nil - } // taken from other parts of the code, would be great to have this in a shared spot diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 85c3b7053..5a3cb9a43 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -222,6 +222,21 @@ func (mock *MockClient) PullIsMergeable(_param0 models.Repo, _param1 models.Pull return ret0, ret1 } +func (mock *MockClient) ReactToComment(_param0 models.Repo, _param1 int64, _param2 string) error { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockClient().") + } + params := []pegomock.Param{_param0, _param1, _param2} + result := pegomock.GetGenericMockFrom(mock).Invoke("ReactToComment", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(error) + } + } + return ret0 +} + func (mock *MockClient) SupportsSingleFileDownload(_param0 models.Repo) bool { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") @@ -642,6 +657,41 @@ func (c *MockClient_PullIsMergeable_OngoingVerification) GetAllCapturedArguments return } +func (verifier *VerifierMockClient) ReactToComment(_param0 models.Repo, _param1 int64, _param2 string) *MockClient_ReactToComment_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ReactToComment", params, verifier.timeout) + return &MockClient_ReactToComment_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockClient_ReactToComment_OngoingVerification struct { + mock *MockClient + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockClient_ReactToComment_OngoingVerification) GetCapturedArguments() (models.Repo, int64, string) { + _param0, _param1, _param2 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] +} + +func (c *MockClient_ReactToComment_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []int64, _param2 []string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]int64, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(int64) + } + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(string) + } + } + return +} + func (verifier *VerifierMockClient) SupportsSingleFileDownload(_param0 models.Repo) *MockClient_SupportsSingleFileDownload_OngoingVerification { params := []pegomock.Param{_param0} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SupportsSingleFileDownload", params, verifier.timeout) diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index e59d1d5f9..2594d9efe 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -35,6 +35,9 @@ func (a *NotConfiguredVCSClient) CreateComment(repo models.Repo, pullNum int, co func (a *NotConfiguredVCSClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { return nil } +func (a *NotConfiguredVCSClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint: revive + return nil +} func (a *NotConfiguredVCSClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) { return models.ApprovalStatus{}, a.err() } diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 0a17123bf..a0afd43bc 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -64,6 +64,10 @@ func (d *ClientProxy) HidePrevCommandComments(repo models.Repo, pullNum int, com return d.clients[repo.VCSHost.Type].HidePrevCommandComments(repo, pullNum, command) } +func (d *ClientProxy) ReactToComment(repo models.Repo, commentID int64, reaction string) error { + return d.clients[repo.VCSHost.Type].ReactToComment(repo, commentID, reaction) +} + func (d *ClientProxy) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) { return d.clients[repo.VCSHost.Type].PullIsApproved(repo, pull) } diff --git a/server/server.go b/server/server.go index 1cf323fff..fe7672d3f 100644 --- a/server/server.go +++ b/server/server.go @@ -861,6 +861,8 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GitlabWebhookSecret: []byte(userConfig.GitlabWebhookSecret), RepoAllowlistChecker: repoAllowlist, SilenceAllowlistErrors: userConfig.SilenceAllowlistErrors, + EmojiReaction: userConfig.EmojiReaction, + ExecutableName: userConfig.ExecutableName, SupportedVCSHosts: supportedVCSHosts, VCSClient: vcsClient, BitbucketWebhookSecret: []byte(userConfig.BitbucketWebhookSecret), diff --git a/server/user_config.go b/server/user_config.go index 8f7b1deed..c37e321f7 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -37,6 +37,7 @@ type UserConfig struct { DisableMarkdownFolding bool `mapstructure:"disable-markdown-folding"` DisableRepoLocking bool `mapstructure:"disable-repo-locking"` DiscardApprovalOnPlanFlag bool `mapstructure:"discard-approval-on-plan"` + EmojiReaction string `mapstructure:"emoji-reaction"` EnablePolicyChecksFlag bool `mapstructure:"enable-policy-checks"` EnableRegExpCmd bool `mapstructure:"enable-regexp-cmd"` EnableDiffMarkdownFormat bool `mapstructure:"enable-diff-markdown-format"`