diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 6db49d49a0..7975a168b6 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -29,6 +29,7 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks" "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" "github.com/runatlantis/atlantis/server/events/webhooks" "github.com/runatlantis/atlantis/server/events/yaml" @@ -811,7 +812,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() diff --git a/server/controllers/github_app_controller.go b/server/controllers/github_app_controller.go index d8808cd835..df8b24de30 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 d5be3b0910..f41ec0a2b8 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,13 +39,12 @@ 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()) +func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, cmdName models.CommandName) error { + src := d.TitleBuilder.Build(cmdName.String()) var descripWords string switch status { case models.PendingCommitStatus: @@ -55,15 +54,15 @@ func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull model case models.SuccessCommitStatus: descripWords = "succeeded." } - descrip := fmt.Sprintf("%s %s", strings.Title(command.String()), descripWords) + descrip := fmt.Sprintf("%s %s", strings.Title(cmdName.String()), descripWords) 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: @@ -80,7 +79,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, + }) var descripWords string switch status { case models.PendingCommitStatus: diff --git a/server/events/commit_status_updater_test.go b/server/events/commit_status_updater_test.go index 582daf2586..aa2c658c06 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 117a2d3fcb..d8c310f343 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -33,15 +33,18 @@ import ( // maxCommentLength is the maximum number of chars allowed in a single comment // by GitHub. -const maxCommentLength = 65536 +const ( + maxCommentLength = 65536 +) // 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. @@ -59,7 +62,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") @@ -99,11 +102,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 } @@ -280,8 +284,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 } @@ -312,6 +348,38 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe return pull, err } +func (g *GithubClient) getRepoStatuses(repo models.Repo, pull models.PullRequest) ([]*github.RepoStatus, error) { + // Get Combined statuses + + nextPage := 0 + + var result []*github.RepoStatus + + for { + opts := github.ListOptions{ + // explicit default + // https://developer.github.com/v3/repos/statuses/#list-commit-statuses-for-a-reference + PerPage: 100, + } + if nextPage != 0 { + opts.Page = nextPage + } + + combinedStatus, response, err := g.client.Repositories.GetCombinedStatus(g.ctx, repo.Owner, repo.Name, pull.HeadCommit, &opts) + result = append(result, combinedStatus.Statuses...) + + if err != nil { + return nil, err + } + if response.NextPage == 0 { + break + } + nextPage = response.NextPage + } + + return result, nil +} + // UpdateStatus updates the status badge on the pull request. // See https://github.com/blog/1227-commit-status-api. func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error { diff --git a/server/events/vcs/github_client_internal_test.go b/server/events/vcs/github_client_internal_test.go index ebe039d319..70b79912ea 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://github.com/gitapi/", 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 bb4f55703e..f31de0d6a9 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "net/http/httptest" "net/url" @@ -62,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()() @@ -117,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()() @@ -211,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()() @@ -300,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()() @@ -366,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()() @@ -452,7 +453,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()() @@ -486,10 +487,6 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { "unknown", false, }, - { - "blocked", - false, - }, { "behind", false, @@ -543,7 +540,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()() @@ -566,6 +563,115 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { } } +func TestGithubClient_PullisMergeable_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, + ) + + combinedStatusJSON := `{ + "state": "success", + "statuses": [%s] + }` + statusJSON := `{ + "url": "https://github.com/gitapi/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_MergePullHandlesError(t *testing.T) { cases := []struct { code int @@ -625,7 +731,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()() @@ -748,7 +854,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()() @@ -776,7 +882,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) @@ -831,7 +937,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} @@ -889,7 +995,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{ diff --git a/server/events/vcs/github_credentials_test.go b/server/events/vcs/github_credentials_test.go index a85d8d2d95..a6951c1db5 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 0000000000..929d3930e1 --- /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 0000000000..144e37480b --- /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 9afab1c7f4..e14158ebff 100644 --- a/server/server.go +++ b/server/server.go @@ -179,7 +179,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } var err error - githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger) + githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger, userConfig.VCSStatusName) if err != nil { return nil, err } @@ -272,7 +272,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) @@ -653,6 +653,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GithubSetupComplete: githubAppEnabled, GithubHostname: userConfig.GithubHostname, GithubOrg: userConfig.GithubOrg, + GithubStatusName: userConfig.VCSStatusName, } return &Server{