From 64612019f44c64b2d4584a2c93f97735eab827b9 Mon Sep 17 00:00:00 2001 From: Nish Krishnan Date: Sat, 1 Jan 2022 15:11:39 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"feat:=20filter=20out=20atlantis/apply?= =?UTF-8?q?=20from=20mergeability=20clause=20(#18=E2=80=A6=20(#1968)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Revert "feat: filter out atlantis/apply from mergeability clause (#1856)" This reverts commit d01796b9d9bf696fd8b42047fcdbd5c3709f995f. * Add missing import. --- .../events/events_controller_e2e_test.go | 2 +- server/controllers/github_app_controller.go | 3 +- server/events/commit_status_updater.go | 40 ++--- server/events/commit_status_updater_test.go | 19 +-- server/events/vcs/github_client.go | 92 ++---------- .../events/vcs/github_client_internal_test.go | 4 +- server/events/vcs/github_client_test.go | 138 ++---------------- server/events/vcs/github_credentials_test.go | 6 +- server/events/vcs/status.go | 39 ----- server/events/vcs/status_test.go | 32 ---- server/server.go | 6 +- 11 files changed, 66 insertions(+), 315 deletions(-) delete mode 100644 server/events/vcs/status.go delete 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 3496d86642..c9b3274803 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -814,7 +814,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl // Mocks. e2eVCSClient := vcsmocks.NewMockClient() - e2eStatusUpdater := &events.DefaultCommitStatusUpdater{Client: e2eVCSClient, TitleBuilder: vcs.StatusTitleBuilder{TitlePrefix: "atlantis"}} + e2eStatusUpdater := &events.DefaultCommitStatusUpdater{Client: e2eVCSClient} 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 df8b24de30..d8808cd835 100644 --- a/server/controllers/github_app_controller.go +++ b/server/controllers/github_app_controller.go @@ -18,7 +18,6 @@ type GithubAppController struct { GithubSetupComplete bool GithubHostname string GithubOrg string - GithubStatusName string } type githubWebhook struct { @@ -56,7 +55,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, g.GithubStatusName) + client, err := vcs.NewGithubClient(g.GithubHostname, creds, g.Logger) 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 a851cdbadd..4e197f93cd 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, cmdName models.CommandName) error + UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command 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, cmdName models.CommandName, numSuccess int, numTotal int) error + UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command 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,21 +39,31 @@ type CommitStatusUpdater interface { // DefaultCommitStatusUpdater implements CommitStatusUpdater. type DefaultCommitStatusUpdater struct { - Client vcs.Client - TitleBuilder vcs.StatusTitleBuilder + Client vcs.Client + // StatusName is the name used to identify Atlantis when creating PR statuses. + StatusName string } func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error { - src := d.TitleBuilder.Build(command.String()) - descrip := fmt.Sprintf("%s %s", strings.Title(command.String()), d.statusDescription(status)) + src := fmt.Sprintf("%s/%s", d.StatusName, command.String()) + var descripWords string + switch status { + case models.PendingCommitStatus: + descripWords = "in progress..." + case models.FailedCommitStatus: + descripWords = "failed." + case models.SuccessCommitStatus: + descripWords = "succeeded." + } + descrip := fmt.Sprintf("%s %s", strings.Title(command.String()), descripWords) return d.Client.UpdateStatus(repo, pull, status, src, descrip, "") } -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()) +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()) cmdVerb := "unknown" - switch cmdName { + switch command { case models.PlanCommand: cmdVerb = "planned" case models.PolicyCheckCommand: @@ -70,14 +80,7 @@ func (d *DefaultCommitStatusUpdater) UpdateProject(ctx models.ProjectCommandCont if projectID == "" { projectID = fmt.Sprintf("%s/%s", ctx.RepoRelDir, ctx.Workspace) } - 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) -} - -func (d *DefaultCommitStatusUpdater) statusDescription(status models.CommitStatus) string { + src := fmt.Sprintf("%s/%s: %s", d.StatusName, cmdName.String(), projectID) var descripWords string switch status { case models.PendingCommitStatus: @@ -88,5 +91,6 @@ func (d *DefaultCommitStatusUpdater) statusDescription(status models.CommitStatu descripWords = "succeeded." } - return descripWords + descrip := fmt.Sprintf("%s %s", strings.Title(cmdName.String()), descripWords) + 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 aa2c658c06..582daf2586 100644 --- a/server/events/commit_status_updater_test.go +++ b/server/events/commit_status_updater_test.go @@ -20,7 +20,6 @@ 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" ) @@ -67,9 +66,7 @@ func TestUpdateCombined(t *testing.T) { t.Run(c.expDescrip, func(t *testing.T) { RegisterMockTestingT(t) client := mocks.NewMockClient() - - titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis"} - s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} + s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis"} err := s.UpdateCombined(models.Repo{}, models.PullRequest{}, c.status, c.command) Ok(t, err) @@ -135,12 +132,11 @@ func TestUpdateCombinedCount(t *testing.T) { t.Run(c.expDescrip, func(t *testing.T) { RegisterMockTestingT(t) client := mocks.NewMockClient() - titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis-test"} - s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} + s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis-test"} err := s.UpdateCombinedCount(models.Repo{}, models.PullRequest{}, c.status, c.command, c.numSuccess, c.numTotal) Ok(t, err) - expSrc := fmt.Sprintf("%s/%s", titleBuilder.TitlePrefix, c.command) + expSrc := fmt.Sprintf("%s/%s", s.StatusName, c.command) client.VerifyWasCalledOnce().UpdateStatus(models.Repo{}, models.PullRequest{}, c.status, expSrc, c.expDescrip, "") }) } @@ -173,8 +169,7 @@ func TestDefaultCommitStatusUpdater_UpdateProjectSrc(t *testing.T) { for _, c := range cases { t.Run(c.expSrc, func(t *testing.T) { client := mocks.NewMockClient() - titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis"} - s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} + s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis"} err := s.UpdateProject(models.ProjectCommandContext{ ProjectName: c.projectName, RepoRelDir: c.repoRelDir, @@ -232,8 +227,7 @@ func TestDefaultCommitStatusUpdater_UpdateProject(t *testing.T) { for _, c := range cases { t.Run(c.expDescrip, func(t *testing.T) { client := mocks.NewMockClient() - titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "atlantis"} - s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} + s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "atlantis"} err := s.UpdateProject(models.ProjectCommandContext{ RepoRelDir: ".", Workspace: "default", @@ -251,8 +245,7 @@ func TestDefaultCommitStatusUpdater_UpdateProject(t *testing.T) { func TestDefaultCommitStatusUpdater_UpdateProjectCustomStatusName(t *testing.T) { RegisterMockTestingT(t) client := mocks.NewMockClient() - titleBuilder := vcs.StatusTitleBuilder{TitlePrefix: "custom"} - s := events.DefaultCommitStatusUpdater{Client: client, TitleBuilder: titleBuilder} + s := events.DefaultCommitStatusUpdater{Client: client, StatusName: "custom"} 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 96f7877d6c..fc6e80207c 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -33,18 +33,15 @@ 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 - statusTitleMatcher StatusTitleMatcher + user string + client *github.Client + v4MutateClient *graphql.Client + ctx context.Context + logger logging.SimpleLogging } // GithubAppTemporarySecrets holds app credentials obtained from github after creation. @@ -62,7 +59,7 @@ type GithubAppTemporarySecrets struct { } // NewGithubClient returns a valid GitHub client. -func NewGithubClient(hostname string, credentials GithubCredentials, logger logging.SimpleLogging, commitStatusPrefix string) (*GithubClient, error) { +func NewGithubClient(hostname string, credentials GithubCredentials, logger logging.SimpleLogging) (*GithubClient, error) { transport, err := credentials.Client() if err != nil { return nil, errors.Wrap(err, "error initializing github authentication transport") @@ -102,12 +99,11 @@ 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, - statusTitleMatcher: StatusTitleMatcher{TitlePrefix: commitStatusPrefix}, + user: user, + client: client, + v4MutateClient: v4MutateClient, + ctx: context.Background(), + logger: logger, }, nil } @@ -284,40 +280,8 @@ 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 } @@ -348,38 +312,6 @@ 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 70b79912ea..ebe039d319 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), "atlantis") + client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) 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), "atlantis") + client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) 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 f31de0d6a9..bb4f55703e 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "net/http" "net/http/httptest" "net/url" @@ -63,7 +62,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, "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logger) Ok(t, err) defer disableSSLVerification()() @@ -118,7 +117,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -212,7 +211,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -301,7 +300,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -367,7 +366,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -453,7 +452,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -487,6 +486,10 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { "unknown", false, }, + { + "blocked", + false, + }, { "behind", false, @@ -540,7 +543,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -563,115 +566,6 @@ 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://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_MergePullHandlesError(t *testing.T) { cases := []struct { code int @@ -731,7 +625,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -854,7 +748,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -882,7 +776,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), "atlantis") + client, err := vcs.NewGithubClient("hostname", &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) pull := models.PullRequest{Num: 1} s, _ := client.MarkdownPullLink(pull) @@ -937,7 +831,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() pull := models.PullRequest{Num: 1} @@ -995,7 +889,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), "atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t)) 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 a6951c1db5..a85d8d2d95 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), "atlantis") + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, logging.NewNoopLogger(t)) 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), "atlantis") + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, logging.NewNoopLogger(t)) 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), "atlantis") + _, err = vcs.NewGithubClient(testServer, appCreds, logging.NewNoopLogger(t)) Ok(t, err) token, err := appCreds.GetToken() diff --git a/server/events/vcs/status.go b/server/events/vcs/status.go deleted file mode 100644 index 929d3930e1..0000000000 --- a/server/events/vcs/status.go +++ /dev/null @@ -1,39 +0,0 @@ -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 deleted file mode 100644 index 144e37480b..0000000000 --- a/server/events/vcs/status_test.go +++ /dev/null @@ -1,32 +0,0 @@ -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 c29af04774..5c4516c713 100644 --- a/server/server.go +++ b/server/server.go @@ -185,7 +185,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } var err error - githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger, userConfig.VCSStatusName) + githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger) if err != nil { return nil, err } @@ -278,7 +278,8 @@ 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, TitleBuilder: vcs.StatusTitleBuilder{TitlePrefix: userConfig.VCSStatusName}} + commitStatusUpdater := &events.DefaultCommitStatusUpdater{Client: vcsClient, StatusName: userConfig.VCSStatusName} + binDir, err := mkSubDir(userConfig.DataDir, BinDirName) if err != nil { @@ -709,7 +710,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GithubSetupComplete: githubAppEnabled, GithubHostname: userConfig.GithubHostname, GithubOrg: userConfig.GithubOrg, - GithubStatusName: userConfig.VCSStatusName, } return &Server{ AtlantisVersion: config.AtlantisVersion,