fix: vcs-status-name hardcoded in PullIsMergeable function (runatlant…

…is#2312) * feat: add userConfig.VCSStatusName to applyCommandRunner context * fix: use vcsstatusname from context * chore: normalize tests * chore: update interface mock
krrrr38 · Dec 16, 2022 · 563d85d · 563d85d
1 parent c470ed0
commit 563d85d
Show file tree

Hide file tree

Showing 19 changed files with 42 additions and 33 deletions.
diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go
@@ -756,7 +756,7 @@ func TestGitHubWorkflowWithPolicyCheck(t *testing.T) {
 
 			// Setup test dependencies.
 			w := httptest.NewRecorder()
-			When(vcsClient.PullIsMergeable(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn(true, nil)
+			When(vcsClient.PullIsMergeable(AnyRepo(), matchers.AnyModelsPullRequest(), "atlantis-test")).ThenReturn(true, nil)
 			When(vcsClient.PullIsApproved(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn(models.ApprovalStatus{
 				IsApproved: true,
 			}, nil)
@@ -1045,6 +1045,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 		parallelPoolSize,
 		silenceNoProjects,
 		false,
+		"atlantis-test",
 		e2ePullReqStatusFetcher,
 	)
 

diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go
@@ -22,6 +22,7 @@ func NewApplyCommandRunner(
 	parallelPoolSize int,
 	SilenceNoProjects bool,
 	silenceVCSStatusNoProjects bool,
+	VCSStatusName string,
 	pullReqStatusFetcher vcs.PullReqStatusFetcher,
 ) *ApplyCommandRunner {
 	return &ApplyCommandRunner{
@@ -38,6 +39,7 @@ func NewApplyCommandRunner(
 		parallelPoolSize:           parallelPoolSize,
 		SilenceNoProjects:          SilenceNoProjects,
 		silenceVCSStatusNoProjects: silenceVCSStatusNoProjects,
+		VCSStatusName:              VCSStatusName,
 		pullReqStatusFetcher:       pullReqStatusFetcher,
 	}
 }
@@ -60,6 +62,7 @@ type ApplyCommandRunner struct {
 	SilenceNoProjects bool
 	// SilenceVCSStatusNoPlans is whether any plan should set commit status if no projects
 	// are found
+	VCSStatusName              string
 	silenceVCSStatusNoProjects bool
 }
 
@@ -103,7 +106,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
 	// required the Atlantis status checks to pass, then we've now changed
 	// the mergeability status of the pull request.
 	// This sets the approved, mergeable, and sqlocked status in the context.
-	ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(baseRepo, pull)
+	ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(baseRepo, pull, a.VCSStatusName)
 	if err != nil {
 		// On error we continue the request with mergeable assumed false.
 		// We want to continue because not all apply's will need this status,

diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go
@@ -107,6 +107,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 
 	parallelPoolSize := 1
 	SilenceNoProjects := false
+	StatusName := "atlantis-test"
 	policyCheckCommandRunner = events.NewPolicyCheckCommandRunner(
 		dbUpdater,
 		pullUpdater,
@@ -151,6 +152,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		parallelPoolSize,
 		SilenceNoProjects,
 		false,
+		StatusName,
 		pullReqStatusFetcher,
 	)
 
@@ -781,7 +783,7 @@ func TestApplyMergeablityWhenPolicyCheckFails(t *testing.T) {
 		},
 	})
 
-	When(ch.VCSClient.PullIsMergeable(fixtures.GithubRepo, modelPull)).ThenReturn(true, nil)
+	When(ch.VCSClient.PullIsMergeable(fixtures.GithubRepo, modelPull, "atlantis-test")).ThenReturn(true, nil)
 
 	When(projectCommandBuilder.BuildApplyCommands(matchers.AnyPtrToEventsCommandContext(), matchers.AnyPtrToEventsCommentCommand())).Then(func(args []Param) ReturnValues {
 		return ReturnValues{

diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go
@@ -167,7 +167,7 @@ func (g *AzureDevopsClient) PullIsApproved(repo models.Repo, pull models.PullReq
 }
 
 // PullIsMergeable returns true if the merge request can be merged.
-func (g *AzureDevopsClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (g *AzureDevopsClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)
 
 	opts := azuredevops.PullRequestGetOptions{IncludeWorkItemRefs: true}

diff --git a/server/events/vcs/azuredevops_client_test.go b/server/events/vcs/azuredevops_client_test.go
@@ -414,7 +414,7 @@ func TestAzureDevopsClient_PullIsMergeable(t *testing.T) {
 				},
 			}, models.PullRequest{
 				Num: 1,
-			})
+			}, "atlantis-test")
 			Ok(t, err)
 			Equals(t, c.expMergeable, actMergeable)
 		})

diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go
@@ -132,7 +132,7 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (appr
 }
 
 // PullIsMergeable returns true if the merge request has no conflicts and can be merged.
-func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	nextPageURL := fmt.Sprintf("%s/2.0/repositories/%s/pullrequests/%d/diffstat", b.BaseURL, repo.FullName, pull.Num)
 	// We'll only loop 1000 times as a safety measure.
 	maxLoops := 1000

diff --git a/server/events/vcs/bitbucketcloud/client_test.go b/server/events/vcs/bitbucketcloud/client_test.go
@@ -337,7 +337,7 @@ func TestClient_PullIsMergeable(t *testing.T) {
 				},
 			}, models.PullRequest{
 				Num: 1,
-			})
+			}, "atlantis-test")
 			Ok(t, err)
 			Equals(t, c.ExpMergeable, actMergeable)
 		})

diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go
@@ -193,7 +193,7 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (appr
 }
 
 // PullIsMergeable returns true if the merge request has no conflicts and can be merged.
-func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	projectKey, err := b.GetProjectKey(repo.Name, repo.SanitizedCloneURL)
 	if err != nil {
 		return false, err

diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go
@@ -27,7 +27,7 @@ type Client interface {
 	CreateComment(repo models.Repo, pullNum int, comment string, command 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) (bool, error)
+	PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error)
 	// UpdateStatus updates the commit status to state for pull. src is the
 	// source of this status. This should be relatively static across runs,
 	// ex. atlantis/plan or atlantis/apply.

diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go
@@ -298,7 +298,7 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
 }
 
 // PullIsMergeable returns true if the pull request is mergeable.
-func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	githubPR, err := g.GetPullRequest(repo, pull.Num)
 	if err != nil {
 		return false, errors.Wrap(err, "getting pull request")
@@ -339,8 +339,8 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
 		}
 		for _, status := range allStatuses {
 			for _, requiredCheck := range requiredChecks.Contexts {
-				// Ignore any commit statuses with 'altantis/apply' as prefix
-				if strings.HasPrefix(status.GetContext(), fmt.Sprintf("atlantis/%s", command.Apply.String())) {
+				// Ignore any commit statuses with 'atlantis/apply' as prefix
+				if strings.HasPrefix(status.GetContext(), fmt.Sprintf("%s/%s", vcsstatusname, command.Apply.String())) {
 					continue
 				}
 				if status.GetContext() == requiredCheck {

diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go
@@ -475,6 +475,7 @@ func TestGithubClient_PullIsApproved(t *testing.T) {
 }
 
 func TestGithubClient_PullIsMergeable(t *testing.T) {
+	vcsStatusName := "atlantis-test"
 	cases := []struct {
 		state               string
 		requiredCheckName   string
@@ -531,13 +532,13 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
 		},
 		{
 			"blocked",
-			"atlantis/apply",
+			fmt.Sprintf("%s/apply", vcsStatusName),
 			"failure",
 			true,
 		},
 		{
 			"blocked",
-			"atlantis/apply",
+			fmt.Sprintf("%s/apply", vcsStatusName),
 			"pending",
 			true,
 		},
@@ -611,7 +612,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
 				Num:        1,
 				HeadBranch: "headBranch",
 				BaseBranch: "baseBranch",
-			})
+			}, vcsStatusName)
 			Ok(t, err)
 			Equals(t, c.expMergeable, actMergeable)
 		})

diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go
@@ -193,7 +193,7 @@ func (g *GitlabClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
 // See:
 // - https://gitlab.com/gitlab-org/gitlab-ee/issues/3169
 // - https://gitlab.com/gitlab-org/gitlab-ce/issues/42344
-func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	mr, _, err := g.Client.MergeRequests.GetMergeRequest(repo.FullName, pull.Num, nil)
 	if err != nil {
 		return false, err
@@ -212,8 +212,8 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
 	}
 
 	for _, status := range statuses {
-		// Ignore any commit statuses with 'altantis/apply' as prefix
-		if strings.HasPrefix(status.Name, fmt.Sprintf("atlantis/%s", command.Apply.String())) {
+		// Ignore any commit statuses with 'atlantis/apply' as prefix
+		if strings.HasPrefix(status.Name, fmt.Sprintf("%s/%s", vcsstatusname, command.Apply.String())) {
 			continue
 		}
 		if !status.AllowFailure && project.OnlyAllowMergeIfPipelineSucceeds && status.Status != "success" {

diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go
@@ -250,33 +250,34 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
 
 func TestGitlabClient_PullIsMergeable(t *testing.T) {
 	gitlabClientUnderTest = true
+	vcsStatusName := "atlantis-test"
 	cases := []struct {
 		statusName string
 		status     models.CommitStatus
 		expState   bool
 	}{
 		{
-			"atlantis/apply: resource/default",
+			fmt.Sprintf("%s/apply: resource/default", vcsStatusName),
 			models.FailedCommitStatus,
 			true,
 		},
 		{
-			"atlantis/apply",
+			fmt.Sprintf("%s/apply", vcsStatusName),
 			models.FailedCommitStatus,
 			true,
 		},
 		{
-			"atlantis/plan: resource/default",
+			fmt.Sprintf("%s/plan: resource/default", vcsStatusName),
 			models.FailedCommitStatus,
 			false,
 		},
 		{
-			"atlantis/plan",
+			fmt.Sprintf("%s/plan", vcsStatusName),
 			models.PendingCommitStatus,
 			false,
 		},
 		{
-			"atlantis/plan",
+			fmt.Sprintf("%s/plan", vcsStatusName),
 			models.SuccessCommitStatus,
 			true,
 		},
@@ -325,7 +326,7 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) {
 				Num:        1,
 				BaseRepo:   repo,
 				HeadCommit: "sha",
-			})
+			}, vcsStatusName)
 			Ok(t, err)
 			Equals(t, c.expState, mergeable)
 		})

diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go
@@ -166,7 +166,7 @@ func (c *InstrumentedClient) PullIsApproved(repo models.Repo, pull models.PullRe
 	return approved, err
 
 }
-func (c *InstrumentedClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (c *InstrumentedClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	scope := c.StatsScope.SubScope("pull_is_mergeable")
 	logger := c.Logger.WithHistory(fmtLogSrc(repo, pull.Num)...)
 
@@ -176,7 +176,7 @@ func (c *InstrumentedClient) PullIsMergeable(repo models.Repo, pull models.PullR
 	executionSuccess := scope.Counter(metrics.ExecutionSuccessMetric)
 	executionError := scope.Counter(metrics.ExecutionErrorMetric)
 
-	mergeable, err := c.Client.PullIsMergeable(repo, pull)
+	mergeable, err := c.Client.PullIsMergeable(repo, pull, vcsstatusname)
 
 	if err != nil {
 		executionError.Inc(1)

diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go
diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go
@@ -38,7 +38,7 @@ func (a *NotConfiguredVCSClient) HidePrevCommandComments(repo models.Repo, pullN
 func (a *NotConfiguredVCSClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) {
 	return models.ApprovalStatus{}, a.err()
 }
-func (a *NotConfiguredVCSClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
+func (a *NotConfiguredVCSClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
 	return false, a.err()
 }
 func (a *NotConfiguredVCSClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {

diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go
@@ -68,8 +68,8 @@ func (d *ClientProxy) PullIsApproved(repo models.Repo, pull models.PullRequest)
 	return d.clients[repo.VCSHost.Type].PullIsApproved(repo, pull)
 }
 
-func (d *ClientProxy) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
-	return d.clients[repo.VCSHost.Type].PullIsMergeable(repo, pull)
+func (d *ClientProxy) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
+	return d.clients[repo.VCSHost.Type].PullIsMergeable(repo, pull, vcsstatusname)
 }
 
 func (d *ClientProxy) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {

diff --git a/server/events/vcs/pull_status_fetcher.go b/server/events/vcs/pull_status_fetcher.go
@@ -6,7 +6,7 @@ import (
 )
 
 type PullReqStatusFetcher interface {
-	FetchPullStatus(repo models.Repo, pull models.PullRequest) (models.PullReqStatus, error)
+	FetchPullStatus(repo models.Repo, pull models.PullRequest, vcsstatusname string) (models.PullReqStatus, error)
 }
 
 type pullReqStatusFetcher struct {
@@ -19,13 +19,13 @@ func NewPullReqStatusFetcher(client Client) PullReqStatusFetcher {
 	}
 }
 
-func (f *pullReqStatusFetcher) FetchPullStatus(repo models.Repo, pull models.PullRequest) (pullStatus models.PullReqStatus, err error) {
+func (f *pullReqStatusFetcher) FetchPullStatus(repo models.Repo, pull models.PullRequest, vcsstatusname string) (pullStatus models.PullReqStatus, err error) {
 	approvalStatus, err := f.client.PullIsApproved(repo, pull)
 	if err != nil {
 		return pullStatus, errors.Wrapf(err, "fetching pull approval status for repo: %s, and pull number: %d", repo.FullName, pull.Num)
 	}
 
-	mergeable, err := f.client.PullIsMergeable(repo, pull)
+	mergeable, err := f.client.PullIsMergeable(repo, pull, vcsstatusname)
 	if err != nil {
 		return pullStatus, errors.Wrapf(err, "fetching mergeability status for repo: %s, and pull number: %d", repo.FullName, pull.Num)
 	}

diff --git a/server/server.go b/server/server.go
@@ -631,6 +631,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		userConfig.ParallelPoolSize,
 		userConfig.SilenceNoProjects,
 		userConfig.SilenceVCSStatusNoProjects,
+		userConfig.VCSStatusName,
 		pullReqStatusFetcher,
 	)