Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add other-vcs-status-names-to-ignore #4978

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
9 changes: 9 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,10 @@ 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 other Atlantis servers to ignore for pull request statuses.",
defaultValue: DefaultIgnoreVCSStatusNames,
},
VCSStatusName: {
description: "Name used to identify Atlantis for pull request statuses.",
defaultValue: DefaultVCSStatusName,
Expand Down Expand Up @@ -918,6 +924,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
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
16 changes: 11 additions & 5 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"encoding/base64"
"fmt"
"net/http"

"slices"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -748,7 +750,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 @@ -771,8 +773,8 @@ func (g *GithubClient) IsMergeableMinusApply(logger logging.SimpleLogging, repo
// Go through all checks and workflows required by branch protection or rulesets
// Make sure that they can all be found in the statusCheckRollup and that they all pass
for _, requiredCheck := range requiredChecks {
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 @@ -790,8 +792,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 @@ -813,7 +819,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
4 changes: 2 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 Down Expand Up @@ -873,7 +873,7 @@ func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T)
},
}, models.PullRequest{
Num: 1,
}, vcsStatusName)
}, vcsStatusName, []string{})
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
2 changes: 1 addition & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
userConfig.QuietPolicyChecks,
)

pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient, userConfig.VCSStatusName)
pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient, userConfig.VCSStatusName, strings.Split(userConfig.IgnoreVCSStatusNames, ","))
planCommandRunner := events.NewPlanCommandRunner(
userConfig.SilenceVCSStatusNoPlans,
userConfig.SilenceVCSStatusNoProjects,
Expand Down
1 change: 1 addition & 0 deletions server/user_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type UserConfig struct {
LogLevel string `mapstructure:"log-level"`
MarkdownTemplateOverridesDir string `mapstructure:"markdown-template-overrides-dir"`
MaxCommentsPerCommand int `mapstructure:"max-comments-per-command"`
IgnoreVCSStatusNames string `mapstructure:"ignore-vcs-status-names"`
ParallelPoolSize int `mapstructure:"parallel-pool-size"`
ParallelPlan bool `mapstructure:"parallel-plan"`
ParallelApply bool `mapstructure:"parallel-apply"`
Expand Down
Loading