From 89d33a07378701f95dba6e0c8d33f2c3a0773ca2 Mon Sep 17 00:00:00 2001 From: Ray Terrill Date: Thu, 18 Aug 2022 09:46:50 -0700 Subject: [PATCH] Make required atlantis/apply status check work with mergeable (#2436) * Make required apply work with mergeable * Fix lint hint * Migrate mergeable approval check to graphql, update tests * Adding support for check suites * Adding feature flag protection for new functionality * Fix linting falsepos * Adjusted logic to handle required check runs, added doc --- cmd/server.go | 103 ++++++------ runatlantis.io/docs/server-configuration.md | 6 + server/controllers/github_app_controller.go | 3 +- .../fixtures/github-commit-status-full.json | 148 ++++++++++++++++++ server/events/vcs/github_client.go | 136 +++++++++++++++- .../events/vcs/github_client_internal_test.go | 4 +- server/events/vcs/github_client_test.go | 51 ++++-- server/events/vcs/github_config.go | 6 + server/events/vcs/github_credentials_test.go | 6 +- server/server.go | 8 +- server/user_config.go | 97 ++++++------ 11 files changed, 449 insertions(+), 119 deletions(-) create mode 100644 server/events/vcs/fixtures/github-commit-status-full.json create mode 100644 server/events/vcs/github_config.go diff --git a/cmd/server.go b/cmd/server.go index 2e26de3055..ac855774ed 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -37,55 +37,56 @@ import ( // 3. Add your flag's description etc. to the stringFlags, intFlags, or boolFlags slices. const ( // Flag names. - ADWebhookPasswordFlag = "azuredevops-webhook-password" // nolint: gosec - ADWebhookUserFlag = "azuredevops-webhook-user" - ADTokenFlag = "azuredevops-token" // nolint: gosec - ADUserFlag = "azuredevops-user" - ADHostnameFlag = "azuredevops-hostname" - AllowForkPRsFlag = "allow-fork-prs" - AllowRepoConfigFlag = "allow-repo-config" - AtlantisURLFlag = "atlantis-url" - AutomergeFlag = "automerge" - AutoplanFileListFlag = "autoplan-file-list" - BitbucketBaseURLFlag = "bitbucket-base-url" - BitbucketTokenFlag = "bitbucket-token" - BitbucketUserFlag = "bitbucket-user" - BitbucketWebhookSecretFlag = "bitbucket-webhook-secret" - ConfigFlag = "config" - CheckoutStrategyFlag = "checkout-strategy" - DataDirFlag = "data-dir" - DefaultTFVersionFlag = "default-tf-version" - DisableApplyAllFlag = "disable-apply-all" - DisableApplyFlag = "disable-apply" - DisableAutoplanFlag = "disable-autoplan" - DisableMarkdownFoldingFlag = "disable-markdown-folding" - DisableRepoLockingFlag = "disable-repo-locking" - EnablePolicyChecksFlag = "enable-policy-checks" - EnableRegExpCmdFlag = "enable-regexp-cmd" - EnableDiffMarkdownFormat = "enable-diff-markdown-format" - GHHostnameFlag = "gh-hostname" - GHTeamAllowlistFlag = "gh-team-allowlist" - GHTokenFlag = "gh-token" - GHUserFlag = "gh-user" - GHAppIDFlag = "gh-app-id" - GHAppKeyFlag = "gh-app-key" - GHAppKeyFileFlag = "gh-app-key-file" - GHAppSlugFlag = "gh-app-slug" - GHOrganizationFlag = "gh-org" - GHWebhookSecretFlag = "gh-webhook-secret" // nolint: gosec - GitlabHostnameFlag = "gitlab-hostname" - GitlabTokenFlag = "gitlab-token" - GitlabUserFlag = "gitlab-user" - GitlabWebhookSecretFlag = "gitlab-webhook-secret" // nolint: gosec - APISecretFlag = "api-secret" - HidePrevPlanComments = "hide-prev-plan-comments" - LogLevelFlag = "log-level" - ParallelPoolSize = "parallel-pool-size" - StatsNamespace = "stats-namespace" - AllowDraftPRs = "allow-draft-prs" - PortFlag = "port" - RepoConfigFlag = "repo-config" - RepoConfigJSONFlag = "repo-config-json" + ADWebhookPasswordFlag = "azuredevops-webhook-password" // nolint: gosec + ADWebhookUserFlag = "azuredevops-webhook-user" + ADTokenFlag = "azuredevops-token" // nolint: gosec + ADUserFlag = "azuredevops-user" + ADHostnameFlag = "azuredevops-hostname" + AllowForkPRsFlag = "allow-fork-prs" + AllowRepoConfigFlag = "allow-repo-config" + AtlantisURLFlag = "atlantis-url" + AutomergeFlag = "automerge" + AutoplanFileListFlag = "autoplan-file-list" + BitbucketBaseURLFlag = "bitbucket-base-url" + BitbucketTokenFlag = "bitbucket-token" + BitbucketUserFlag = "bitbucket-user" + BitbucketWebhookSecretFlag = "bitbucket-webhook-secret" + ConfigFlag = "config" + CheckoutStrategyFlag = "checkout-strategy" + DataDirFlag = "data-dir" + DefaultTFVersionFlag = "default-tf-version" + DisableApplyAllFlag = "disable-apply-all" + DisableApplyFlag = "disable-apply" + DisableAutoplanFlag = "disable-autoplan" + DisableMarkdownFoldingFlag = "disable-markdown-folding" + DisableRepoLockingFlag = "disable-repo-locking" + EnablePolicyChecksFlag = "enable-policy-checks" + EnableRegExpCmdFlag = "enable-regexp-cmd" + EnableDiffMarkdownFormat = "enable-diff-markdown-format" + GHHostnameFlag = "gh-hostname" + GHTeamAllowlistFlag = "gh-team-allowlist" + GHTokenFlag = "gh-token" + GHUserFlag = "gh-user" + GHAppIDFlag = "gh-app-id" + GHAppKeyFlag = "gh-app-key" + GHAppKeyFileFlag = "gh-app-key-file" + GHAppSlugFlag = "gh-app-slug" + GHOrganizationFlag = "gh-org" + GHWebhookSecretFlag = "gh-webhook-secret" // nolint: gosec + GHAllowMergeableBypassApply = "gh-allow-mergeable-bypass-apply" // nolint: gosec + GitlabHostnameFlag = "gitlab-hostname" + GitlabTokenFlag = "gitlab-token" + GitlabUserFlag = "gitlab-user" + GitlabWebhookSecretFlag = "gitlab-webhook-secret" // nolint: gosec + APISecretFlag = "api-secret" + HidePrevPlanComments = "hide-prev-plan-comments" + LogLevelFlag = "log-level" + ParallelPoolSize = "parallel-pool-size" + StatsNamespace = "stats-namespace" + AllowDraftPRs = "allow-draft-prs" + PortFlag = "port" + RepoConfigFlag = "repo-config" + RepoConfigJSONFlag = "repo-config-json" // RepoWhitelistFlag is deprecated for RepoAllowlistFlag. RepoWhitelistFlag = "repo-whitelist" RepoAllowlistFlag = "repo-allowlist" @@ -374,6 +375,10 @@ var boolFlags = map[string]boolFlag{ description: "Enable Atlantis to format Terraform plan output into a markdown-diff friendly format for color-coding purposes.", defaultValue: false, }, + GHAllowMergeableBypassApply: { + description: "Feature flag to enable functionality to allow mergeable check to ignore apply required check", + defaultValue: false, + }, AllowDraftPRs: { description: "Enable autoplan for Github Draft Pull Requests", defaultValue: false, diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 67c8af9b52..d3a92a5071 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -369,6 +369,12 @@ Values are chosen in this order: ``` Comma-separated list of GitHub team name (not a slug) and permission pairs. By default, any team can plan and apply. +- ### `--gh-allow-mergeable-bypass-apply` + ```bash + atlantis server --gh-allow-mergeable-bypass-apply + ``` + Feature flag to enable ability to use `mergeable` mode with required apply status check. + ### `--gitlab-hostname` ```bash atlantis server --gitlab-hostname="my.gitlab.enterprise.com" diff --git a/server/controllers/github_app_controller.go b/server/controllers/github_app_controller.go index 7221f2008b..14753a38f6 100644 --- a/server/controllers/github_app_controller.go +++ b/server/controllers/github_app_controller.go @@ -55,7 +55,8 @@ 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) + config := vcs.GithubConfig{} + client, err := vcs.NewGithubClient(g.GithubHostname, creds, config, 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/vcs/fixtures/github-commit-status-full.json b/server/events/vcs/fixtures/github-commit-status-full.json new file mode 100644 index 0000000000..a042101c40 --- /dev/null +++ b/server/events/vcs/fixtures/github-commit-status-full.json @@ -0,0 +1,148 @@ +{ + "state": "blocked", + "statuses": [ + { + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "id": 16230299674, + "node_id": "SC_kwDOFRFvL88AAAADx2a4Gg", + "state": "success", + "description": "Plan succeeded.", + "target_url": "https://localhost/jobs/octocat/Hello-World/1/project1", + "context": "atlantis/plan: project1", + "created_at": "2022-02-10T15:26:01Z", + "updated_at": "2022-02-10T15:26:01Z" + }, + { + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "id": 16230303174, + "node_id": "SC_kwDOFRFvL88AAAADx2bFxg", + "state": "success", + "description": "Plan succeeded.", + "target_url": "https://localhost/jobs/octocat/Hello-World/1/project2", + "context": "atlantis/plan: project2", + "created_at": "2022-02-10T15:26:12Z", + "updated_at": "2022-02-10T15:26:12Z" + }, + { + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "id": 16230303679, + "node_id": "SC_kwDOFRFvL88AAAADx2bHvw", + "state": "success", + "description": "2/2 projects planned successfully.", + "target_url": "", + "context": "atlantis/plan", + "created_at": "2022-02-10T15:26:13Z", + "updated_at": "2022-02-10T15:26:13Z" + }, + { + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "id": 16230307923, + "node_id": "SC_kwDOFRFvL88AAAADx2bYUw", + "state": "failure", + "description": "Apply failed.", + "target_url": "https://localhost/jobs/octocat/Hello-World/1/project1", + "context": "atlantis/apply: project1", + "created_at": "2022-02-10T15:26:27Z", + "updated_at": "2022-02-10T15:26:27Z" + }, + { + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "id": 16230308153, + "node_id": "SC_kwDOFRFvL88AAAADx2bZOQ", + "state": "failure", + "description": "Apply failed.", + "target_url": "https://localhost/jobs/octocat/Hello-World/1/project2", + "context": "atlantis/apply: project2", + "created_at": "2022-02-10T15:26:27Z", + "updated_at": "2022-02-10T15:26:27Z" + }, + { + "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "id": 16230308528, + "node_id": "SC_kwDOFRFvL88AAAADx2basA", + "state": "failure", + "description": "0/2 projects applied successfully.", + "target_url": "", + "context": "atlantis/apply", + "created_at": "2022-02-10T15:26:28Z", + "updated_at": "2022-02-10T15:26:28Z" + } + ], + "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", + "total_count": 0, + "repository": { + "id": 1296269, + "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", + "name": "Hello-World", + "full_name": "octocat/Hello-World", + "private": false, + "owner": { + "login": "octocat", + "id": 583231, + "node_id": "MDQ6VXNlcjU4MzIzMQ==", + "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "html_url": "https://github.com/octocat/Hello-World", + "description": "My first repository on GitHub!", + "fork": false, + "url": "https://api.github.com/repos/octocat/Hello-World", + "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks", + "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams", + "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks", + "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}", + "events_url": "https://api.github.com/repos/octocat/Hello-World/events", + "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}", + "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}", + "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags", + "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}", + "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages", + "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers", + "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors", + "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers", + "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription", + "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}", + "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges", + "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads", + "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}", + "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}", + "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}", + "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}", + "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}", + "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments" + }, + "commit_url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e", + "url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/status" + } \ No newline at end of file diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 8b396ad557..2645b56429 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-github/v31/github" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/config" + "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs/common" "github.com/runatlantis/atlantis/server/logging" @@ -44,6 +45,7 @@ type GithubClient struct { v4QueryClient *githubv4.Client ctx context.Context logger logging.SimpleLogging + config GithubConfig } // GithubAppTemporarySecrets holds app credentials obtained from github after creation. @@ -61,7 +63,7 @@ type GithubAppTemporarySecrets struct { } // NewGithubClient returns a valid GitHub client. -func NewGithubClient(hostname string, credentials GithubCredentials, logger logging.SimpleLogging) (*GithubClient, error) { +func NewGithubClient(hostname string, credentials GithubCredentials, config GithubConfig, logger logging.SimpleLogging) (*GithubClient, error) { transport, err := credentials.Client() if err != nil { return nil, errors.Wrap(err, "error initializing github authentication transport") @@ -117,6 +119,7 @@ func NewGithubClient(hostname string, credentials GithubCredentials, logger logg v4QueryClient: v4QueryClient, ctx: context.Background(), logger: logger, + config: config, }, nil } @@ -296,12 +299,118 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest) return approvalStatus, nil } +// isRequiredCheck is a helper function to determine if a check is required or not +func isRequiredCheck(check string, required []string) bool { + //in go1.18 can prob replace this with slices.Contains + for _, r := range required { + if r == check { + return true + } + } + + return false +} + +// GetCombinedStatusMinusApply checks Statuses for PR, excluding atlantis apply. Returns true if all other statuses are not in failure. +func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *github.PullRequest, vcstatusname string) (bool, error) { + //check combined status api + status, _, err := g.client.Repositories.GetCombinedStatus(g.ctx, repo.Owner, repo.Name, *pull.Head.Ref, nil) + if err != nil { + return false, errors.Wrap(err, "getting combined status") + } + + //iterate over statuses - return false if we find one that isnt "apply" and doesnt have state = "success" + for _, r := range status.Statuses { + if strings.HasPrefix(*r.Context, fmt.Sprintf("%s/%s", vcstatusname, command.Apply.String())) { + continue + } + if *r.State != "success" { + return false, nil + } + } + + //get required status checks + required, _, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner, repo.Name, *pull.Base.Ref) + if err != nil { + return false, errors.Wrap(err, "getting required status checks") + } + + //check check suite/check run api + checksuites, _, err := g.client.Checks.ListCheckSuitesForRef(context.Background(), repo.Owner, repo.Name, *pull.Head.Ref, nil) + if err != nil { + return false, errors.Wrap(err, "getting check suites for ref") + } + + //iterate over check completed check suites - return false if we find one that doesnt have conclusion = "success" + for _, c := range checksuites.CheckSuites { + if *c.Status == "completed" { + fmt.Printf("Looking at suite %v\n", *c.ID) + //iterate over the runs inside the suite + suite, _, err := g.client.Checks.ListCheckRunsCheckSuite(context.Background(), repo.Owner, repo.Name, *c.ID, nil) + if err != nil { + return false, errors.Wrap(err, "getting check runs for check suite") + } + + for _, r := range suite.CheckRuns { + fmt.Printf("Looking at check run %s\n", *r.Name) + //check to see if the check is required + if isRequiredCheck(*r.Name, required.RequiredStatusChecks.Contexts) { + fmt.Println("Check is required") + if *c.Conclusion == "success" { + fmt.Println("Check is successful") + continue + } else { + fmt.Println("Check is failed") + return false, nil + } + } else { + //ignore checks that arent required + fmt.Println("Check is not required") + continue + } + + } + } + } + + return true, nil +} + +// GetPullReviewDecision gets the pull review decision, which takes into account CODEOWNERS +func (g *GithubClient) GetPullReviewDecision(repo models.Repo, pull models.PullRequest) (approvalStatus bool, err error) { + var query struct { + Repository struct { + PullRequest struct { + ReviewDecision string + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.Owner), + "name": githubv4.String(repo.Name), + "number": githubv4.Int(pull.Num), + } + + err = g.v4QueryClient.Query(g.ctx, &query, variables) + if err != nil { + return approvalStatus, errors.Wrap(err, "getting reviewDecision") + } + + if query.Repository.PullRequest.ReviewDecision == "APPROVED" { + return true, nil + } + + return false, nil +} + // PullIsMergeable returns true if the pull request is mergeable. 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") } + state := githubPR.GetMergeableState() // We map our mergeable check to when the GitHub merge button is clickable. // This corresponds to the following states: @@ -313,6 +422,31 @@ 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" { + //mergeable bypass apply code hidden by feature flag + if g.config.AllowMergeableBypassApply { + g.logger.Debug("AllowMergeableBypassApply feature flag is enabled - attempting to bypass apply from mergeable requirements") + if state == "blocked" { + //check status excluding atlantis apply + status, err := g.GetCombinedStatusMinusApply(repo, githubPR, vcsstatusname) + if err != nil { + return false, errors.Wrap(err, "getting pull request status") + } + + fmt.Printf("Status was %v\n", status) + + //check to see if pr is approved using reviewDecision + approved, err := g.GetPullReviewDecision(repo, pull) + if err != nil { + return false, errors.Wrap(err, "getting pull request reviewDecision") + } + + //if all other status checks EXCEPT atlantis/apply are successful, and the PR is approved based on reviewDecision, let it proceed + if status && approved { + return true, nil + } + } + } + return false, nil } return true, nil diff --git a/server/events/vcs/github_client_internal_test.go b/server/events/vcs/github_client_internal_test.go index ebe039d319..8809b0a44a 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)) + client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, 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)) + client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, 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 c705750a47..2e3c8b4e38 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -63,7 +63,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) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logger) Ok(t, err) defer disableSSLVerification()() @@ -118,7 +118,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -212,7 +212,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -301,7 +301,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -367,7 +367,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -453,7 +453,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -523,6 +523,22 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { Ok(t, err) prJSON := string(jsBytes) + // Status Check Response + jsBytes, err = os.ReadFile("fixtures/github-commit-status-full.json") + Ok(t, err) + commitJSON := string(jsBytes) + + //reviewDecision Response + reviewDecision := `{ + "data": { + "repository": { + "pullRequest": { + "reviewDecision": "REVIEW_REQUIRED" + } + } + } + }` + for _, c := range cases { t.Run(c.state, func(t *testing.T) { response := strings.Replace(prJSON, @@ -537,6 +553,13 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { case "/api/v3/repos/owner/repo/pulls/1": w.Write([]byte(response)) // nolint: errcheck return + case "/api/v3/repos/owner/repo/pulls/1/reviews?per_page=300": + w.Write([]byte("[]")) // nolint: errcheck + return + case "/api/v3/repos/owner/repo/commits/new-topic/status": + w.Write([]byte(commitJSON)) // nolint: errcheck + case "/api/graphql": + w.Write([]byte(reviewDecision)) // nolint: errcheck default: t.Errorf("got unexpected request at %q", r.RequestURI) http.Error(w, "not found", http.StatusNotFound) @@ -545,7 +568,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -627,7 +650,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -750,7 +773,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() @@ -778,7 +801,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)) + client, err := vcs.NewGithubClient("hostname", &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) pull := models.PullRequest{Num: 1} s, _ := client.MarkdownPullLink(pull) @@ -833,7 +856,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() pull := models.PullRequest{Num: 1} @@ -891,7 +914,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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() repo := models.Repo{ @@ -936,7 +959,7 @@ func TestGithubClient_Retry404Files(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)) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) defer disableSSLVerification()() repo := models.Repo{ @@ -989,7 +1012,7 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) { })) testServerURL, err := url.Parse(testServer.URL) Ok(t, err) - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logger) + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, logger) Ok(t, err) defer disableSSLVerification()() diff --git a/server/events/vcs/github_config.go b/server/events/vcs/github_config.go new file mode 100644 index 0000000000..ceeb3c720c --- /dev/null +++ b/server/events/vcs/github_config.go @@ -0,0 +1,6 @@ +package vcs + +// GithubConfig allows for custom github-specific functionality and behavior +type GithubConfig struct { + AllowMergeableBypassApply bool +} diff --git a/server/events/vcs/github_credentials_test.go b/server/events/vcs/github_credentials_test.go index a85d8d2d95..08b3de6171 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)) + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, vcs.GithubConfig{}, 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)) + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, vcs.GithubConfig{}, 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)) + _, err = vcs.NewGithubClient(testServer, appCreds, vcs.GithubConfig{}, logging.NewNoopLogger(t)) Ok(t, err) token, err := appCreds.GetToken() diff --git a/server/server.go b/server/server.go index f7ba8d19f7..ce8dbffe88 100644 --- a/server/server.go +++ b/server/server.go @@ -157,6 +157,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { var supportedVCSHosts []models.VCSHostType var githubClient vcs.IGithubClient var githubAppEnabled bool + var githubConfig vcs.GithubConfig var githubCredentials vcs.GithubCredentials var gitlabClient *vcs.GitlabClient var bitbucketCloudClient *bitbucketcloud.Client @@ -198,6 +199,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } if userConfig.GithubUser != "" || userConfig.GithubAppID != 0 { + if userConfig.GithubAllowMergeableBypassApply { + githubConfig = vcs.GithubConfig{ + AllowMergeableBypassApply: true, + } + } supportedVCSHosts = append(supportedVCSHosts, models.Github) if userConfig.GithubUser != "" { githubCredentials = &vcs.GithubUserCredentials{ @@ -227,7 +233,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } var err error - rawGithubClient, err := vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger) + rawGithubClient, err := vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, githubConfig, logger) if err != nil { return nil, err } diff --git a/server/user_config.go b/server/user_config.go index 84cf275498..af3e5e1c02 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -8,54 +8,55 @@ import ( // The mapstructure tags correspond to flags in cmd/server.go and are used when // the config is parsed from a YAML file. type UserConfig struct { - AllowForkPRs bool `mapstructure:"allow-fork-prs"` - AllowRepoConfig bool `mapstructure:"allow-repo-config"` - AtlantisURL string `mapstructure:"atlantis-url"` - Automerge bool `mapstructure:"automerge"` - AutoplanFileList string `mapstructure:"autoplan-file-list"` - AzureDevopsToken string `mapstructure:"azuredevops-token"` - AzureDevopsUser string `mapstructure:"azuredevops-user"` - AzureDevopsWebhookPassword string `mapstructure:"azuredevops-webhook-password"` - AzureDevopsWebhookUser string `mapstructure:"azuredevops-webhook-user"` - AzureDevOpsHostname string `mapstructure:"azuredevops-hostname"` - BitbucketBaseURL string `mapstructure:"bitbucket-base-url"` - BitbucketToken string `mapstructure:"bitbucket-token"` - BitbucketUser string `mapstructure:"bitbucket-user"` - BitbucketWebhookSecret string `mapstructure:"bitbucket-webhook-secret"` - CheckoutStrategy string `mapstructure:"checkout-strategy"` - DataDir string `mapstructure:"data-dir"` - DisableApplyAll bool `mapstructure:"disable-apply-all"` - DisableApply bool `mapstructure:"disable-apply"` - DisableAutoplan bool `mapstructure:"disable-autoplan"` - DisableMarkdownFolding bool `mapstructure:"disable-markdown-folding"` - DisableRepoLocking bool `mapstructure:"disable-repo-locking"` - EnablePolicyChecksFlag bool `mapstructure:"enable-policy-checks"` - EnableRegExpCmd bool `mapstructure:"enable-regexp-cmd"` - EnableDiffMarkdownFormat bool `mapstructure:"enable-diff-markdown-format"` - GithubHostname string `mapstructure:"gh-hostname"` - GithubToken string `mapstructure:"gh-token"` - GithubUser string `mapstructure:"gh-user"` - GithubWebhookSecret string `mapstructure:"gh-webhook-secret"` - GithubOrg string `mapstructure:"gh-org"` - GithubAppID int64 `mapstructure:"gh-app-id"` - GithubAppKey string `mapstructure:"gh-app-key"` - GithubAppKeyFile string `mapstructure:"gh-app-key-file"` - GithubAppSlug string `mapstructure:"gh-app-slug"` - GithubTeamAllowlist string `mapstructure:"gh-team-allowlist"` - GitlabHostname string `mapstructure:"gitlab-hostname"` - GitlabToken string `mapstructure:"gitlab-token"` - GitlabUser string `mapstructure:"gitlab-user"` - GitlabWebhookSecret string `mapstructure:"gitlab-webhook-secret"` - APISecret string `mapstructure:"api-secret"` - HidePrevPlanComments bool `mapstructure:"hide-prev-plan-comments"` - LogLevel string `mapstructure:"log-level"` - ParallelPoolSize int `mapstructure:"parallel-pool-size"` - StatsNamespace string `mapstructure:"stats-namespace"` - PlanDrafts bool `mapstructure:"allow-draft-prs"` - Port int `mapstructure:"port"` - RepoConfig string `mapstructure:"repo-config"` - RepoConfigJSON string `mapstructure:"repo-config-json"` - RepoAllowlist string `mapstructure:"repo-allowlist"` + AllowForkPRs bool `mapstructure:"allow-fork-prs"` + AllowRepoConfig bool `mapstructure:"allow-repo-config"` + AtlantisURL string `mapstructure:"atlantis-url"` + Automerge bool `mapstructure:"automerge"` + AutoplanFileList string `mapstructure:"autoplan-file-list"` + AzureDevopsToken string `mapstructure:"azuredevops-token"` + AzureDevopsUser string `mapstructure:"azuredevops-user"` + AzureDevopsWebhookPassword string `mapstructure:"azuredevops-webhook-password"` + AzureDevopsWebhookUser string `mapstructure:"azuredevops-webhook-user"` + AzureDevOpsHostname string `mapstructure:"azuredevops-hostname"` + BitbucketBaseURL string `mapstructure:"bitbucket-base-url"` + BitbucketToken string `mapstructure:"bitbucket-token"` + BitbucketUser string `mapstructure:"bitbucket-user"` + BitbucketWebhookSecret string `mapstructure:"bitbucket-webhook-secret"` + CheckoutStrategy string `mapstructure:"checkout-strategy"` + DataDir string `mapstructure:"data-dir"` + DisableApplyAll bool `mapstructure:"disable-apply-all"` + DisableApply bool `mapstructure:"disable-apply"` + DisableAutoplan bool `mapstructure:"disable-autoplan"` + DisableMarkdownFolding bool `mapstructure:"disable-markdown-folding"` + DisableRepoLocking bool `mapstructure:"disable-repo-locking"` + EnablePolicyChecksFlag bool `mapstructure:"enable-policy-checks"` + EnableRegExpCmd bool `mapstructure:"enable-regexp-cmd"` + EnableDiffMarkdownFormat bool `mapstructure:"enable-diff-markdown-format"` + GithubAllowMergeableBypassApply bool `mapstructure:"gh-allow-mergeable-bypass-apply"` + GithubHostname string `mapstructure:"gh-hostname"` + GithubToken string `mapstructure:"gh-token"` + GithubUser string `mapstructure:"gh-user"` + GithubWebhookSecret string `mapstructure:"gh-webhook-secret"` + GithubOrg string `mapstructure:"gh-org"` + GithubAppID int64 `mapstructure:"gh-app-id"` + GithubAppKey string `mapstructure:"gh-app-key"` + GithubAppKeyFile string `mapstructure:"gh-app-key-file"` + GithubAppSlug string `mapstructure:"gh-app-slug"` + GithubTeamAllowlist string `mapstructure:"gh-team-allowlist"` + GitlabHostname string `mapstructure:"gitlab-hostname"` + GitlabToken string `mapstructure:"gitlab-token"` + GitlabUser string `mapstructure:"gitlab-user"` + GitlabWebhookSecret string `mapstructure:"gitlab-webhook-secret"` + APISecret string `mapstructure:"api-secret"` + HidePrevPlanComments bool `mapstructure:"hide-prev-plan-comments"` + LogLevel string `mapstructure:"log-level"` + ParallelPoolSize int `mapstructure:"parallel-pool-size"` + StatsNamespace string `mapstructure:"stats-namespace"` + PlanDrafts bool `mapstructure:"allow-draft-prs"` + Port int `mapstructure:"port"` + RepoConfig string `mapstructure:"repo-config"` + RepoConfigJSON string `mapstructure:"repo-config-json"` + RepoAllowlist string `mapstructure:"repo-allowlist"` // RepoWhitelist is deprecated in favour of RepoAllowlist. RepoWhitelist string `mapstructure:"repo-whitelist"`