fix: vcs-status-name hardcoded in PullIsMergeable function (#2312)

* feat: add userConfig.VCSStatusName to applyCommandRunner context

* fix: use vcsstatusname from context

* chore: normalize tests

* chore: update interface mock
This commit is contained in:
Michel Z. Santello
2022-06-16 13:26:20 -03:00
committed by GitHub
parent 288e91ab1a
commit df0d58d29b
19 changed files with 42 additions and 33 deletions

View File

@@ -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,
)

View File

@@ -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,

View File

@@ -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{

View File

@@ -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}

View File

@@ -414,7 +414,7 @@ func TestAzureDevopsClient_PullIsMergeable(t *testing.T) {
},
}, models.PullRequest{
Num: 1,
})
}, "atlantis-test")
Ok(t, err)
Equals(t, c.expMergeable, actMergeable)
})

View File

@@ -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

View File

@@ -337,7 +337,7 @@ func TestClient_PullIsMergeable(t *testing.T) {
},
}, models.PullRequest{
Num: 1,
})
}, "atlantis-test")
Ok(t, err)
Equals(t, c.ExpMergeable, actMergeable)
})

View File

@@ -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

View File

@@ -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.

View File

@@ -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 {

View File

@@ -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)
})

View File

@@ -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" {

View File

@@ -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)
})

View File

@@ -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)

View File

@@ -170,7 +170,7 @@ func (mock *MockClient) GetTeamNamesForUser(repo models.Repo, user models.User)
return ret0, ret1
}
func (mock *MockClient) PullIsMergeable(_param0 models.Repo, _param1 models.PullRequest) (bool, error) {
func (mock *MockClient) PullIsMergeable(_param0 models.Repo, _param1 models.PullRequest, vcsstatusname string) (bool, error) {
if mock == nil {
panic("mock must not be nil. Use myMock := NewMockClient().")
}

View File

@@ -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 {

View File

@@ -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 {

View File

@@ -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)
}

View File

@@ -631,6 +631,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
userConfig.ParallelPoolSize,
userConfig.SilenceNoProjects,
userConfig.SilenceVCSStatusNoProjects,
userConfig.VCSStatusName,
pullReqStatusFetcher,
)