From e4917905c6362282428349fa7f8e36de6956dcfe Mon Sep 17 00:00:00 2001 From: Nish Krishnan Date: Thu, 18 Nov 2021 09:22:14 -0800 Subject: [PATCH] feat: filter out atlantis/apply from mergeability clause (#1856) (#156) Filter out atlantis apply status during mergeability check. --- .../events/events_controller_e2e_test.go | 2 +- server/controllers/github_app_controller.go | 3 +- server/events/commit_status_updater.go | 27 +-- server/events/commit_status_updater_test.go | 19 ++- server/events/vcs/github_client.go | 60 +++++-- .../events/vcs/github_client_internal_test.go | 4 +- server/events/vcs/github_client_test.go | 155 +++++++++++++++--- server/events/vcs/github_credentials_test.go | 6 +- server/events/vcs/status.go | 39 +++++ server/events/vcs/status_test.go | 32 ++++ server/server.go | 5 +- 11 files changed, 283 insertions(+), 69 deletions(-) create mode 100644 server/events/vcs/status.go create mode 100644 server/events/vcs/status_test.go diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 57c95048b..6a963ceeb 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -822,7 +822,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl // Mocks. e2eVCSClient := vcsmocks.NewMockClient() - e2eStatusUpdater := &events.DefaultCommitStatusUpdater{Client: e2eVCSClient} + e2eStatusUpdater := &events.DefaultCommitStatusUpdater{Client: e2eVCSClient, TitleBuilder: vcs.StatusTitleBuilder{TitlePrefix: "atlantis"}} e2eGithubGetter := mocks.NewMockGithubPullGetter() e2eGitlabGetter := mocks.NewMockGitlabMergeRequestGetter() projectCmdOutputHandler := handlermocks.NewMockProjectCommandOutputHandler() diff --git a/server/controllers/github_app_controller.go b/server/controllers/github_app_controller.go index d8808cd83..df8b24de3 100644 --- a/server/controllers/github_app_controller.go +++ b/server/controllers/github_app_controller.go @@ -18,6 +18,7 @@ type GithubAppController struct { GithubSetupComplete bool GithubHostname string GithubOrg string + GithubStatusName string } type githubWebhook struct { @@ -55,7 +56,7 @@ func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Reques g.Logger.Debug("Exchanging GitHub app code for app credentials") creds := &vcs.GithubAnonymousCredentials{} - client, err := vcs.NewGithubClient(g.GithubHostname, creds, g.Logger) + client, err := vcs.NewGithubClient(g.GithubHostname, creds, g.Logger, g.GithubStatusName) if err != nil { g.respond(w, logging.Error, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err) return diff --git a/server/events/commit_status_updater.go b/server/events/commit_status_updater.go index 636d6a230..312211a2f 100644 --- a/server/events/commit_status_updater.go +++ b/server/events/commit_status_updater.go @@ -28,10 +28,10 @@ import ( type CommitStatusUpdater interface { // UpdateCombined updates the combined status of the head commit of pull. // A combined status represents all the projects modified in the pull. - UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error + UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, cmdName models.CommandName) error // UpdateCombinedCount updates the combined status to reflect the // numSuccess out of numTotal. - UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName, numSuccess int, numTotal int) error + UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, cmdName models.CommandName, numSuccess int, numTotal int) error // UpdateProject sets the commit status for the project represented by // ctx. UpdateProject(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus, url string) error @@ -39,23 +39,21 @@ type CommitStatusUpdater interface { // DefaultCommitStatusUpdater implements CommitStatusUpdater. type DefaultCommitStatusUpdater struct { - Client vcs.Client - // StatusName is the name used to identify Atlantis when creating PR statuses. - StatusName string + Client vcs.Client + TitleBuilder vcs.StatusTitleBuilder } -func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error { - src := fmt.Sprintf("%s/%s", d.StatusName, command.String()) - - descrip := fmt.Sprintf("%s %s", strings.Title(command.String()), d.statusDescription(status)) +func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, cmdName models.CommandName) error { + src := d.TitleBuilder.Build(cmdName.String()) + descrip := fmt.Sprintf("%s %s", strings.Title(cmdName.String()), d.statusDescription(status)) return d.Client.UpdateStatus(repo, pull, status, src, descrip, "") } -func (d *DefaultCommitStatusUpdater) UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName, numSuccess int, numTotal int) error { - src := fmt.Sprintf("%s/%s", d.StatusName, command.String()) +func (d *DefaultCommitStatusUpdater) UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, cmdName models.CommandName, numSuccess int, numTotal int) error { + src := d.TitleBuilder.Build(cmdName.String()) cmdVerb := "unknown" - switch command { + switch cmdName { case models.PlanCommand: cmdVerb = "planned" case models.PolicyCheckCommand: @@ -72,7 +70,10 @@ func (d *DefaultCommitStatusUpdater) UpdateProject(ctx models.ProjectCommandCont if projectID == "" { projectID = fmt.Sprintf("%s/%s", ctx.RepoRelDir, ctx.Workspace) } - src := fmt.Sprintf("%s/%s: %s", d.StatusName, cmdName.String(), projectID) + src := d.TitleBuilder.Build(cmdName.String(), vcs.StatusTitleOptions{ + ProjectName: projectID, + }) + descrip := fmt.Sprintf("%s %s", strings.Title(cmdName.String()), d.statusDescription(status)) return d.Client.UpdateStatus(ctx.BaseRepo, ctx.Pull, status, src, descrip, url) } diff --git a/server/events/commit_status_updater_test.go b/server/events/commit_status_updater_test.go index 582daf258..aa2c658c0 100644 --- a/server/events/commit_status_updater_test.go +++ b/server/events/commit_status_updater_test.go @@ -20,6 +20,7 @@ import ( . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/events/vcs/mocks" . "github.com/runatlantis/atlantis/testing" ) @@ -66,7 +67,9 @@ func TestUpdateCombined(t *testing.T) { t.Run(c.expDescrip, func(t *testing.T) { RegisterMockTestingT(t) client := mocks.NewMockClient() - s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis"} + + titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis"} + s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} err := s.UpdateCombined(models.Repo{}, models.PullRequest{}, c.status, c.command) Ok(t, err) @@ -132,11 +135,12 @@ func TestUpdateCombinedCount(t *testing.T) { t.Run(c.expDescrip, func(t *testing.T) { RegisterMockTestingT(t) client := mocks.NewMockClient() - s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis-test"} + titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis-test"} + s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} err := s.UpdateCombinedCount(models.Repo{}, models.PullRequest{}, c.status, c.command, c.numSuccess, c.numTotal) Ok(t, err) - expSrc := fmt.Sprintf("%s/%s", s.StatusName, c.command) + expSrc := fmt.Sprintf("%s/%s", titleBuilder.TitlePrefix, c.command) client.VerifyWasCalledOnce().UpdateStatus(models.Repo{}, models.PullRequest{}, c.status, expSrc, c.expDescrip, "") }) } @@ -169,7 +173,8 @@ func TestDefaultCommitStatusUpdater_UpdateProjectSrc(t *testing.T) { for _, c := range cases { t.Run(c.expSrc, func(t *testing.T) { client := mocks.NewMockClient() - s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis"} + titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis"} + s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} err := s.UpdateProject(models.ProjectCommandContext{ ProjectName: c.projectName, RepoRelDir: c.repoRelDir, @@ -227,7 +232,8 @@ func TestDefaultCommitStatusUpdater_UpdateProject(t *testing.T) { for _, c := range cases { t.Run(c.expDescrip, func(t *testing.T) { client := mocks.NewMockClient() - s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis"} + titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis"} + s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} err := s.UpdateProject(models.ProjectCommandContext{ RepoRelDir: ".", Workspace: "default", @@ -245,7 +251,8 @@ func TestDefaultCommitStatusUpdater_UpdateProject(t *testing.T) { func TestDefaultCommitStatusUpdater_UpdateProjectCustomStatusName(t *testing.T) { RegisterMockTestingT(t) client := mocks.NewMockClient() - s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "custom"} + titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "custom"} + s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} err := s.UpdateProject(models.ProjectCommandContext{ RepoRelDir: ".", Workspace: "default", diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index d009bdf1b..8f3dac63d 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -39,7 +39,6 @@ const ( SubmitQueueReadinessStatusContext = "sq-ready-to-merge" OwnersStatusContext = "_owners-check" - AtlantisApplyStatusContext = "atlantis/apply" LockValue = "lock" ) @@ -54,11 +53,12 @@ func (p *PullRequestNotFound) Error() string { // GithubClient is used to perform GitHub actions. type GithubClient struct { - user string - client *github.Client - v4MutateClient *graphql.Client - ctx context.Context - logger logging.SimpleLogging + user string + client *github.Client + v4MutateClient *graphql.Client + ctx context.Context + logger logging.SimpleLogging + statusTitleMatcher StatusTitleMatcher } // GithubAppTemporarySecrets holds app credentials obtained from github after creation. @@ -76,7 +76,7 @@ type GithubAppTemporarySecrets struct { } // NewGithubClient returns a valid GitHub client. -func NewGithubClient(hostname string, credentials GithubCredentials, logger logging.SimpleLogging) (*GithubClient, error) { +func NewGithubClient(hostname string, credentials GithubCredentials, logger logging.SimpleLogging, commitStatusPrefix string) (*GithubClient, error) { transport, err := credentials.Client() if err != nil { return nil, errors.Wrap(err, "error initializing github authentication transport") @@ -116,11 +116,12 @@ func NewGithubClient(hostname string, credentials GithubCredentials, logger logg return nil, errors.Wrap(err, "getting user") } return &GithubClient{ - user: user, - client: client, - v4MutateClient: v4MutateClient, - ctx: context.Background(), - logger: logger, + user: user, + client: client, + v4MutateClient: v4MutateClient, + ctx: context.Background(), + logger: logger, + statusTitleMatcher: StatusTitleMatcher{TitlePrefix: commitStatusPrefix}, }, nil } @@ -311,8 +312,40 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest // hooks. Merging is allowed (green box). // See: https://github.com/octokit/octokit.net/issues/1763 if state != "clean" && state != "unstable" && state != "has_hooks" { + + if state != "blocked" { + return false, nil + } + + return g.getSupplementalMergeability(repo, pull) + } + return true, nil +} + +// Checks to make sure that all statuses are passing except the atlantis/apply. If we only rely on GetMergeableState, +// we can run into issues where if an apply failed, we can never apply again due to mergeability failures. +func (g *GithubClient) getSupplementalMergeability(repo models.Repo, pull models.PullRequest) (bool, error) { + statuses, err := g.GetRepoStatuses(repo, pull) + + if err != nil { + return false, errors.Wrapf(err, "fetching repo statuses for repo: %s, and pull number: %d", repo.FullName, pull.Num) + } + + for _, status := range statuses { + state := status.GetState() + + if g.statusTitleMatcher.MatchesCommand(status.GetContext(), "apply") || + state == "success" { + continue + + } + + // we either have a failure or a pending status check + // hence the PR is not mergeable return false, nil } + + // all our status checks are successful by our definition, return true, nil } @@ -396,8 +429,7 @@ func (g *GithubClient) getSubmitQueueMergeability(repo models.Repo, pull models. ownersCheckApplied = true } - if strings.HasPrefix(status.GetContext(), AtlantisApplyStatusContext) || - state == "success" || + if state == "success" || (state == "pending" && status.GetContext() == SubmitQueueReadinessStatusContext) { continue } diff --git a/server/events/vcs/github_client_internal_test.go b/server/events/vcs/github_client_internal_test.go index ebe039d31..70b79912e 100644 --- a/server/events/vcs/github_client_internal_test.go +++ b/server/events/vcs/github_client_internal_test.go @@ -22,14 +22,14 @@ import ( // If the hostname is github.com, should use normal BaseURL. func TestNewGithubClient_GithubCom(t *testing.T) { - client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) Equals(t, "https://api.github.com/", client.client.BaseURL.String()) } // If the hostname is a non-github hostname should use the right BaseURL. func TestNewGithubClient_NonGithub(t *testing.T) { - client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) Equals(t, "https://example.com/api/v3/", client.client.BaseURL.String()) // If possible in the future, test the GraphQL client's URL as well. But at the diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 72432cb19..bbcd8505b 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -63,7 +63,7 @@ func TestGithubClient_GetModifiedFiles(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logger) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logger, "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -118,7 +118,7 @@ func TestGithubClient_GetModifiedFilesMovedFile(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -212,7 +212,7 @@ func TestGithubClient_PaginatesComments(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -301,7 +301,7 @@ func TestGithubClient_HideOldComments(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -367,7 +367,7 @@ func TestGithubClient_UpdateStatus(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -454,7 +454,7 @@ func TestGithubClient_PullIsApproved(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -550,7 +550,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { })) testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -585,13 +585,121 @@ func TestGithubClient_PullisMergeable_BlockedStatus(t *testing.T) { 1, ) + combinedStatusJSON := `{ + "state": "success", + "statuses": [%s] + }` + statusJSON := `{ + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://github.com/images/error/other_user_happy.gif", + "id": 2, + "node_id": "MDY6U3RhdHVzMg==", + "state": "%s", + "description": "Testing has completed successfully", + "target_url": "https://ci.example.com/2000/output", + "context": "%s", + "created_at": "2012-08-20T01:19:13Z", + "updated_at": "2012-08-20T01:19:13Z" + }` + + cases := []struct { + description string + statuses []string + expMergeable bool + }{ + { + "apply-failure", + []string{ + fmt.Sprintf(statusJSON, "failure", "atlantis/apply"), + }, + true, + }, + { + "apply-project-failure", + []string{ + fmt.Sprintf(statusJSON, "failure", "atlantis/apply: terraform_cloud_workspace"), + }, + true, + }, + { + "sq-pending+owners-failure", + []string{ + fmt.Sprintf(statusJSON, "failure", "atlantis/plan"), + fmt.Sprintf(statusJSON, "failure", "atlantis/apply"), + }, + false, + }, + } + + for _, c := range cases { + + t.Run("blocked/"+c.description, func(t *testing.T) { + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/commits/2/status?per_page=100": + _, _ = w.Write([]byte( + fmt.Sprintf(combinedStatusJSON, strings.Join(c.statuses, ",")), + )) // nolint: errcheck + return + case "/api/v3/repos/owner/repo/pulls/1": + w.Write([]byte(pullResponse)) // nolint: errcheck + return + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + defer testServer.Close() + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") + Ok(t, err) + defer disableSSLVerification()() + + actMergeable, err := client.PullIsMergeable(models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, models.PullRequest{ + Num: 1, + HeadCommit: "2", + }) + Ok(t, err) + Equals(t, c.expMergeable, actMergeable) + }) + } + +} + +func TestGithubClient_PullisSQMergeable_BlockedStatus(t *testing.T) { + // Use a real GitHub json response and edit the mergeable_state field. + jsBytes, err := ioutil.ReadFile("fixtures/github-pull-request.json") + Ok(t, err) + json := string(jsBytes) + + pullResponse := strings.Replace(json, + `"mergeable_state": "clean"`, + fmt.Sprintf(`"mergeable_state": "%s"`, "blocked"), + 1, + ) + cases := []struct { description string statuses []*github.RepoStatus expMergeable bool }{ { - "sq-pending+owners-success+apply-failure", + "sq-pending+owners-success", []*github.RepoStatus{ { State: helper("pending"), @@ -601,14 +709,6 @@ func TestGithubClient_PullisMergeable_BlockedStatus(t *testing.T) { State: helper("success"), Context: helper("_owners-check"), }, - { - State: helper("failure"), - Context: helper("atlantis/apply"), - }, - { - State: helper("failure"), - Context: helper("atlantis/apply: terraform_cloud_workspace"), - }, }, true, }, @@ -658,7 +758,7 @@ func TestGithubClient_PullisMergeable_BlockedStatus(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -683,6 +783,7 @@ func TestGithubClient_PullisMergeable_BlockedStatus(t *testing.T) { } + func helper(str string) *string { temp := str return &temp @@ -747,7 +848,7 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -870,7 +971,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() @@ -898,7 +999,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { } func TestGithubClient_MarkdownPullLink(t *testing.T) { - client, err := vcs.NewGithubClient("hostname", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient("hostname", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) pull := models.PullRequest{Num: 1} s, _ := client.MarkdownPullLink(pull) @@ -953,7 +1054,7 @@ func TestGithubClient_SplitComments(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() pull := models.PullRequest{Num: 1} @@ -1011,7 +1112,7 @@ func TestGithubClient_Retry404(t *testing.T) { testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) defer disableSSLVerification()() repo := models.Repo{ @@ -1031,7 +1132,7 @@ func TestGithubClient_Retry404(t *testing.T) { } func TestGithubClient_PullIsLocked_Locked(t *testing.T) { - client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) statuses := []*github.RepoStatus{ @@ -1060,7 +1161,7 @@ func TestGithubClient_PullIsLocked_Locked(t *testing.T) { } func TestGithubClient_PullIsLocked_Unlocked(t *testing.T) { - client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) statuses := []*github.RepoStatus{ @@ -1089,7 +1190,7 @@ func TestGithubClient_PullIsLocked_Unlocked(t *testing.T) { } func TestGithubClient_PullIsLocked_SQContextNotFound(t *testing.T) { - client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) statuses := []*github.RepoStatus{ @@ -1118,7 +1219,7 @@ func TestGithubClient_PullIsLocked_SQContextNotFound(t *testing.T) { } func TestGithubClient_PullIsLocked_WaitingKeyNotFound(t *testing.T) { - client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) statuses := []*github.RepoStatus{ @@ -1147,7 +1248,7 @@ func TestGithubClient_PullIsLocked_WaitingKeyNotFound(t *testing.T) { } func TestGithubClient_PullIsLocked_SQ_No_Tags(t *testing.T) { - client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) + client, err := vcs.NewGithubClient("temp", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t), "atlantis") Ok(t, err) statuses := []*github.RepoStatus{ diff --git a/server/events/vcs/github_credentials_test.go b/server/events/vcs/github_credentials_test.go index a85d8d2d9..a6951c1db 100644 --- a/server/events/vcs/github_credentials_test.go +++ b/server/events/vcs/github_credentials_test.go @@ -15,7 +15,7 @@ func TestGithubClient_GetUser_AppSlug(t *testing.T) { Ok(t, err) anonCreds := &vcs.GithubAnonymousCredentials{} - anonClient, err := vcs.NewGithubClient(testServer, anonCreds, logging.NewNoopLogger(t)) + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, logging.NewNoopLogger(t), "atlantis") Ok(t, err) tempSecrets, err := anonClient.ExchangeCode("good-code") Ok(t, err) @@ -39,7 +39,7 @@ func TestGithubClient_AppAuthentication(t *testing.T) { Ok(t, err) anonCreds := &vcs.GithubAnonymousCredentials{} - anonClient, err := vcs.NewGithubClient(testServer, anonCreds, logging.NewNoopLogger(t)) + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, logging.NewNoopLogger(t), "atlantis") Ok(t, err) tempSecrets, err := anonClient.ExchangeCode("good-code") Ok(t, err) @@ -49,7 +49,7 @@ func TestGithubClient_AppAuthentication(t *testing.T) { Key: []byte(fixtures.GithubPrivateKey), Hostname: testServer, } - _, err = vcs.NewGithubClient(testServer, appCreds, logging.NewNoopLogger(t)) + _, err = vcs.NewGithubClient(testServer, appCreds, logging.NewNoopLogger(t), "atlantis") Ok(t, err) token, err := appCreds.GetToken() diff --git a/server/events/vcs/status.go b/server/events/vcs/status.go new file mode 100644 index 000000000..929d3930e --- /dev/null +++ b/server/events/vcs/status.go @@ -0,0 +1,39 @@ +package vcs + +import ( + "fmt" + "strings" +) + +type StatusTitleMatcher struct { + TitlePrefix string +} + +func (m StatusTitleMatcher) MatchesCommand(title string, command string) bool { + return strings.HasPrefix(title, fmt.Sprintf("%s/%s", m.TitlePrefix, command)) +} + +type StatusTitleBuilder struct { + TitlePrefix string +} + +type StatusTitleOptions struct { + ProjectName string +} + +func (b StatusTitleBuilder) Build(command string, options ...StatusTitleOptions) string { + src := fmt.Sprintf("%s/%s", b.TitlePrefix, command) + + var projectName string + for _, opt := range options { + if opt.ProjectName != "" { + projectName = opt.ProjectName + } + } + + if projectName != "" { + src = fmt.Sprintf("%s: %s", src, projectName) + } + + return src +} diff --git a/server/events/vcs/status_test.go b/server/events/vcs/status_test.go new file mode 100644 index 000000000..144e37480 --- /dev/null +++ b/server/events/vcs/status_test.go @@ -0,0 +1,32 @@ +package vcs_test + +import ( + "testing" + + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/stretchr/testify/assert" +) + +func TestMatches(t *testing.T) { + + t.Run("in sync with builder", func(t *testing.T) { + titlePrefix := "atlantis-test" + command := "apply" + builder := vcs.StatusTitleBuilder{TitlePrefix: titlePrefix} + matcher := vcs.StatusTitleMatcher{TitlePrefix: titlePrefix} + + title := builder.Build(command) + + assert.True(t, matcher.MatchesCommand(title, command)) + }) + + t.Run("incorrect command", func(t *testing.T) { + titlePrefix := "atlantis-test" + builder := vcs.StatusTitleBuilder{TitlePrefix: titlePrefix} + matcher := vcs.StatusTitleMatcher{TitlePrefix: titlePrefix} + + title := builder.Build("apply") + + assert.False(t, matcher.MatchesCommand(title, "plan")) + }) +} diff --git a/server/server.go b/server/server.go index 52f21b942..988c87292 100644 --- a/server/server.go +++ b/server/server.go @@ -201,7 +201,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } var err error - rawGithubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger) + rawGithubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger, userConfig.VCSStatusName) if err != nil { return nil, err } @@ -295,7 +295,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { return nil, errors.Wrap(err, "initializing webhooks") } vcsClient := vcs.NewClientProxy(githubClient, gitlabClient, bitbucketCloudClient, bitbucketServerClient, azuredevopsClient) - commitStatusUpdater := &events.DefaultCommitStatusUpdater{Client: vcsClient, StatusName: userConfig.VCSStatusName} + commitStatusUpdater := &events.DefaultCommitStatusUpdater{Client: vcsClient, TitleBuilder: vcs.StatusTitleBuilder{TitlePrefix: userConfig.VCSStatusName}} binDir, err := mkSubDir(userConfig.DataDir, BinDirName) @@ -782,6 +782,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GithubSetupComplete: githubAppEnabled, GithubHostname: userConfig.GithubHostname, GithubOrg: userConfig.GithubOrg, + GithubStatusName: userConfig.VCSStatusName, } scheduledExecutorService := scheduled.NewExecutorService(