Skip to content

Commit

Permalink
feat: add ignore-vcs-status-names (github only) (#4978)
Browse files Browse the repository at this point in the history
Signed-off-by: bakayolo <benjamin.apprederisse@gmail.com>
  • Loading branch information
bakayolo authored Nov 4, 2024
1 parent 8285a0f commit 841f7b4
Show file tree
Hide file tree
Showing 25 changed files with 239 additions and 33 deletions.
11 changes: 11 additions & 0 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ const (
UseTFPluginCache = "use-tf-plugin-cache"
VarFileAllowlistFlag = "var-file-allowlist"
VCSStatusName = "vcs-status-name"
IgnoreVCSStatusNames = "ignore-vcs-status-names"
TFEHostnameFlag = "tfe-hostname"
TFELocalExecutionModeFlag = "tfe-local-execution-mode"
TFETokenFlag = "tfe-token"
Expand Down Expand Up @@ -175,6 +176,7 @@ const (
DefaultGitlabHostname = "gitlab.com"
DefaultLockingDBType = "boltdb"
DefaultLogLevel = "info"
DefaultIgnoreVCSStatusNames = ""
DefaultMaxCommentsPerCommand = 100
DefaultParallelPoolSize = 15
DefaultStatsNamespace = "atlantis"
Expand Down Expand Up @@ -439,6 +441,12 @@ var stringFlags = map[string]stringFlag{
description: "Comma-separated list of additional paths where variable definition files can be read from." +
" If this argument is not provided, it defaults to Atlantis' data directory, determined by the --data-dir argument.",
},
IgnoreVCSStatusNames: {
description: "Comma separated list of VCS status names from other atlantis services." +
" When `gh-allow-mergeable-bypass-apply` is true, will ignore status checks (e.g. `status1/plan`, `status1/apply`, `status2/plan`, `status2/apply`) from other Atlantis services when checking if the PR is mergeable." +
" Currently only implemented for GitHub.",
defaultValue: DefaultIgnoreVCSStatusNames,
},
VCSStatusName: {
description: "Name used to identify Atlantis for pull request statuses.",
defaultValue: DefaultVCSStatusName,
Expand Down Expand Up @@ -918,6 +926,9 @@ func (s *ServerCmd) setDefaults(c *server.UserConfig, v *viper.Viper) {
if c.VCSStatusName == "" {
c.VCSStatusName = DefaultVCSStatusName
}
if c.IgnoreVCSStatusNames == "" {
c.IgnoreVCSStatusNames = DefaultIgnoreVCSStatusNames
}
if c.TFEHostname == "" {
c.TFEHostname = DefaultTFEHostname
}
Expand Down
1 change: 1 addition & 0 deletions cmd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ var testFlags = map[string]interface{}{
UseTFPluginCache: true,
VarFileAllowlistFlag: "/path",
VCSStatusName: "my-status",
IgnoreVCSStatusNames: "",
WebBasicAuthFlag: false,
WebPasswordFlag: "atlantis",
WebUsernameFlag: "atlantis",
Expand Down
14 changes: 14 additions & 0 deletions runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,20 @@ This is useful when you have many projects and want to keep the pull request cle
Used for example with CDKTF pre-workflow hooks that dynamically generate
Terraform files.

### `--ignore-vcs-status-names`

```bash
atlantis server --ignore-vcs-status-names="status1,status2"
# or
ATLANTIS_IGNORE_VCS_STATUS_NAMES=status1,status2
```

Comma separated list of VCS status names from other atlantis services.
When `gh-allow-mergeable-bypass-apply` is true, will ignore status checks
(e.g. `status1/plan`, `status1/apply`, `status2/plan`, `status2/apply`)
from other Atlantis services when checking if the PR is mergeable.
Currently only implemented for GitHub.

### `--locking-db-type`

```bash
Expand Down
4 changes: 2 additions & 2 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ func TestGitHubWorkflowWithPolicyCheck(t *testing.T) {
// Setup test dependencies.
w := httptest.NewRecorder()
When(vcsClient.PullIsMergeable(
Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq("atlantis-test"))).ThenReturn(true, nil)
Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq("atlantis-test"), Eq([]string{}))).ThenReturn(true, nil)
When(vcsClient.PullIsApproved(
Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(models.ApprovalStatus{
IsApproved: true,
Expand Down Expand Up @@ -1505,7 +1505,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
userConfig.QuietPolicyChecks,
)

e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient, "atlantis-test")
e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient, "atlantis-test", []string{})

planCommandRunner := events.NewPlanCommandRunner(
false,
Expand Down
2 changes: 1 addition & 1 deletion server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ func TestApplyMergeablityWhenPolicyCheckFails(t *testing.T) {
},
})

When(ch.VCSClient.PullIsMergeable(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull), Eq("atlantis-test"))).ThenReturn(true, nil)
When(ch.VCSClient.PullIsMergeable(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull), Eq("atlantis-test"), Eq([]string{}))).ThenReturn(true, nil)

When(projectCommandBuilder.BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())).Then(func(args []Param) ReturnValues {
return ReturnValues{
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (g *AzureDevopsClient) DiscardReviews(repo models.Repo, pull models.PullReq
}

// PullIsMergeable returns true if the merge request can be merged.
func (g *AzureDevopsClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) { //nolint: revive
func (g *AzureDevopsClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, _ string, _ []string) (bool, error) { //nolint: revive
owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)

opts := azuredevops.PullRequestGetOptions{IncludeWorkItemRefs: true}
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/azuredevops_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func TestAzureDevopsClient_PullIsMergeable(t *testing.T) {
},
}, models.PullRequest{
Num: 1,
}, "atlantis-test")
}, "atlantis-test", []string{})
Ok(t, err)
Equals(t, c.expMergeable, actMergeable)
})
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketcloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (b *Client) PullIsApproved(logger logging.SimpleLogging, repo models.Repo,
}

// PullIsMergeable returns true if the merge request has no conflicts and can be merged.
func (b *Client) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, _ string) (bool, error) {
func (b *Client) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, _ string, _ []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
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketcloud/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func TestClient_PullIsMergeable(t *testing.T) {
},
}, models.PullRequest{
Num: 1,
}, "atlantis-test")
}, "atlantis-test", []string{})
Ok(t, err)
Equals(t, c.ExpMergeable, actMergeable)
})
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (b *Client) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
}

// PullIsMergeable returns true if the merge request has no conflicts and can be merged.
func (b *Client) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, _ string) (bool, error) {
func (b *Client) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, _ string, _ []string) (bool, error) {
projectKey, err := b.GetProjectKey(repo.Name, repo.SanitizedCloneURL)
if err != nil {
return false, err
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Client interface {
ReactToComment(logger logging.SimpleLogging, repo models.Repo, pullNum int, commentID int64, reaction string) error
HidePrevCommandComments(logger logging.SimpleLogging, repo models.Repo, pullNum int, command string, dir string) error
PullIsApproved(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error)
PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error)
PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, ignoreVCSStatusNames []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.
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/gitea/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (c *GiteaClient) PullIsApproved(logger logging.SimpleLogging, repo models.R
}

// PullIsMergeable returns true if the pull request is mergeable
func (c *GiteaClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
func (c *GiteaClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, _ string, _ []string) (bool, error) {
logger.Debug("Checking if Gitea pull request %d is mergeable", pull.Num)

pullRequest, _, err := c.giteaClient.GetPullRequest(repo.Owner, repo.Name, int64(pull.Num))
Expand Down
14 changes: 9 additions & 5 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ func (g *GithubClient) ExpectedWorkflowPassed(expectedWorkflow WorkflowFileRefer
}

// IsMergeableMinusApply checks review decision (which takes into account CODEOWNERS) and required checks for PR (excluding the atlantis apply check).
func (g *GithubClient) IsMergeableMinusApply(logger logging.SimpleLogging, repo models.Repo, pull *github.PullRequest, vcsstatusname string) (bool, error) {
func (g *GithubClient) IsMergeableMinusApply(logger logging.SimpleLogging, repo models.Repo, pull *github.PullRequest, vcsstatusname string, ignoreVCSStatusNames []string) (bool, error) {
if pull.Number == nil {
return false, errors.New("pull request number is nil")
}
Expand All @@ -778,8 +778,8 @@ func (g *GithubClient) IsMergeableMinusApply(logger logging.SimpleLogging, repo
// Ignore atlantis apply check(s)
continue
}
if !ExpectedCheckPassed(requiredCheck, checkRuns, statusContexts, vcsstatusname) {
logger.Debug("%s: Expected Required Check: %s", notMergeablePrefix, requiredCheck)
if !slices.Contains(ignoreVCSStatusNames, GetVCSStatusNameFromRequiredCheck(requiredCheck)) && !ExpectedCheckPassed(requiredCheck, checkRuns, statusContexts, vcsstatusname) {
logger.Debug("%s: Expected Required Check: %s VCS Status Name: %s Ignore VCS Status Names: %s", notMergeablePrefix, requiredCheck, vcsstatusname, ignoreVCSStatusNames)
return false, nil
}
}
Expand All @@ -797,8 +797,12 @@ func (g *GithubClient) IsMergeableMinusApply(logger logging.SimpleLogging, repo
return true, nil
}

func GetVCSStatusNameFromRequiredCheck(requiredCheck githubv4.String) string {
return strings.Split(string(requiredCheck), "/")[0]
}

// PullIsMergeable returns true if the pull request is mergeable.
func (g *GithubClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
func (g *GithubClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, ignoreVCSStatusNames []string) (bool, error) {
logger.Debug("Checking if GitHub pull request %d is mergeable", pull.Num)
githubPR, err := g.GetPullRequest(logger, repo, pull.Num)
if err != nil {
Expand All @@ -820,7 +824,7 @@ func (g *GithubClient) PullIsMergeable(logger logging.SimpleLogging, repo models
case "blocked":
if g.config.AllowMergeableBypassApply {
logger.Debug("AllowMergeableBypassApply feature flag is enabled - attempting to bypass apply from mergeable requirements")
isMergeableMinusApply, err := g.IsMergeableMinusApply(logger, repo, githubPR, vcsstatusname)
isMergeableMinusApply, err := g.IsMergeableMinusApply(logger, repo, githubPR, vcsstatusname, ignoreVCSStatusNames)
if err != nil {
return false, errors.Wrap(err, "getting pull request status")
}
Expand Down
17 changes: 15 additions & 2 deletions server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
},
}, models.PullRequest{
Num: 1,
}, vcsStatusName)
}, vcsStatusName, []string{})
Ok(t, err)
Equals(t, c.expMergeable, actMergeable)
})
Expand All @@ -619,6 +619,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T) {
logger := logging.NewNoopLogger(t)
vcsStatusName := "atlantis"
ignoreVCSStatusNames := []string{"other-atlantis"}
cases := []struct {
state string
statusCheckRollupFilePath string
Expand Down Expand Up @@ -715,6 +716,12 @@ func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T)
`"APPROVED"`,
false,
},
{
"blocked",
"ruleset-check-pending-other-atlantis.json",
`"APPROVED"`,
true,
},
{
"blocked",
"ruleset-check-skipped.json",
Expand Down Expand Up @@ -763,6 +770,12 @@ func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T)
`"APPROVED"`,
false,
},
{
"blocked",
"ruleset-check-failed-other-atlantis.json",
`"APPROVED"`,
true,
},
{
"blocked",
"ruleset-check-passed.json",
Expand Down Expand Up @@ -879,7 +892,7 @@ func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T)
},
}, models.PullRequest{
Num: 1,
}, vcsStatusName)
}, vcsStatusName, ignoreVCSStatusNames)
Ok(t, err)
Equals(t, c.expMergeable, actMergeable)
})
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func (g *GitlabClient) PullIsApproved(logger logging.SimpleLogging, repo models.
// See:
// - https://gitlab.com/gitlab-org/gitlab-ee/issues/3169
// - https://gitlab.com/gitlab-org/gitlab-ce/issues/42344
func (g *GitlabClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
func (g *GitlabClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, _ []string) (bool, error) {
logger.Debug("Checking if GitLab merge request %d is mergeable", pull.Num)
mr, resp, err := g.Client.MergeRequests.GetMergeRequest(repo.FullName, pull.Num, nil)
if resp != nil {
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/gitlab_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) {
Num: c.mrID,
BaseRepo: repo,
HeadCommit: "67cb91d3f6198189f433c045154a885784ba6977",
}, vcsStatusName)
}, vcsStatusName, []string{})

Ok(t, err)
Equals(t, c.expState, mergeable)
Expand Down
4 changes: 2 additions & 2 deletions server/events/vcs/instrumented_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (c *InstrumentedClient) PullIsApproved(logger logging.SimpleLogging, repo m
return approved, err
}

func (c *InstrumentedClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
func (c *InstrumentedClient) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, ignoreVCSStatusNames []string) (bool, error) {
scope := c.StatsScope.SubScope("pull_is_mergeable")
scope = SetGitScopeTags(scope, repo.FullName, pull.Num)

Expand All @@ -193,7 +193,7 @@ func (c *InstrumentedClient) PullIsMergeable(logger logging.SimpleLogging, repo
executionSuccess := scope.Counter(metrics.ExecutionSuccessMetric)
executionError := scope.Counter(metrics.ExecutionErrorMetric)

mergeable, err := c.Client.PullIsMergeable(logger, repo, pull, vcsstatusname)
mergeable, err := c.Client.PullIsMergeable(logger, repo, pull, vcsstatusname, ignoreVCSStatusNames)

if err != nil {
executionError.Inc(1)
Expand Down
4 changes: 2 additions & 2 deletions server/events/vcs/mocks/mock_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/events/vcs/not_configured_vcs_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (a *NotConfiguredVCSClient) PullIsApproved(_ logging.SimpleLogging, _ model
func (a *NotConfiguredVCSClient) DiscardReviews(_ models.Repo, _ models.PullRequest) error {
return nil
}
func (a *NotConfiguredVCSClient) PullIsMergeable(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest, _ string) (bool, error) {
func (a *NotConfiguredVCSClient) PullIsMergeable(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest, _ string, _ []string) (bool, error) {
return false, a.err()
}
func (a *NotConfiguredVCSClient) UpdateStatus(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest, _ models.CommitStatus, _ string, _ string, _ string) error {
Expand Down
4 changes: 2 additions & 2 deletions server/events/vcs/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ func (d *ClientProxy) DiscardReviews(repo models.Repo, pull models.PullRequest)
return d.clients[repo.VCSHost.Type].DiscardReviews(repo, pull)
}

func (d *ClientProxy) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
return d.clients[repo.VCSHost.Type].PullIsMergeable(logger, repo, pull, vcsstatusname)
func (d *ClientProxy) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, vcsstatusname string, ignoreVCSStatusNames []string) (bool, error) {
return d.clients[repo.VCSHost.Type].PullIsMergeable(logger, repo, pull, vcsstatusname, ignoreVCSStatusNames)
}

func (d *ClientProxy) UpdateStatus(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {
Expand Down
14 changes: 8 additions & 6 deletions server/events/vcs/pull_status_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ type PullReqStatusFetcher interface {
}

type pullReqStatusFetcher struct {
client Client
vcsStatusName string
client Client
vcsStatusName string
ignoreVCSStatusNames []string
}

func NewPullReqStatusFetcher(client Client, vcsStatusName string) PullReqStatusFetcher {
func NewPullReqStatusFetcher(client Client, vcsStatusName string, ignoreVCSStatusNames []string) PullReqStatusFetcher {
return &pullReqStatusFetcher{
client: client,
vcsStatusName: vcsStatusName,
client: client,
vcsStatusName: vcsStatusName,
ignoreVCSStatusNames: ignoreVCSStatusNames,
}
}

Expand All @@ -30,7 +32,7 @@ func (f *pullReqStatusFetcher) FetchPullStatus(logger logging.SimpleLogging, pul
return pullStatus, errors.Wrapf(err, "fetching pull approval status for repo: %s, and pull number: %d", pull.BaseRepo.FullName, pull.Num)
}

mergeable, err := f.client.PullIsMergeable(logger, pull.BaseRepo, pull, f.vcsStatusName)
mergeable, err := f.client.PullIsMergeable(logger, pull.BaseRepo, pull, f.vcsStatusName, f.ignoreVCSStatusNames)
if err != nil {
return pullStatus, errors.Wrapf(err, "fetching mergeability status for repo: %s, and pull number: %d", pull.BaseRepo.FullName, pull.Num)
}
Expand Down
Loading

0 comments on commit 841f7b4

Please sign in to comment.