--gh-allow-mergeable-bypass-apply skips status checks

jffrose commented 1 year ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Overview of the Issue

When testing out the -gh-allow-mergeable-bypass-apply option on the Atlantis server I noticed that this action allows Atlantis to skip additional mergeable checks that are not part of a protected branch's status checks.

For example, a Protected branch can prevent merges on Require conversation resolution before merging. If this box is checked for a branch and the following workflow happens then atlantis apply still passes.

Reproduction Steps

enable Require conversation resolution before merging on protected branch
Run Atlantis with -gh-allow-mergeable-bypass-apply
Require apply_requirement:[mergeable]
Create PR
Create unresolved comment on PR
Run atlantis apply
Observe that apply succeeds

If I don't use the -gh-allow-mergeable-bypass-apply flag then the same PR's atlantis apply fails.

Logs

``` {"level":"debug","ts":"2022-10-27T14:48:33.436-0400","caller":"server/middleware.go:45","msg":"POST /events – from [::1]:65255","json":{}} {"level":"debug","ts":"2022-10-27T14:48:33.436-0400","caller":"events/events_controller.go:98","msg":"handling GitHub post","json":{}} {"level":"debug","ts":"2022-10-27T14:48:33.456-0400","caller":"events/events_controller.go:163","msg":"request valid","json":{"gh-request-id":"X-Github-Delivery=f4948200-5627-11ed-9925-aae3d0ca0550"}} {"level":"info","ts":"2022-10-27T14:48:33.457-0400","caller":"events/events_controller.go:533","msg":"parsed comment as command=\"apply\" verbose=false dir=\"\" workspace=\"\" project=\"\" flags=\"\"","json":{"gh-request-id":"X-Github-Delivery=f4948200-5627-11ed-9925-aae3d0ca0550"}} {"level":"debug","ts":"2022-10-27T14:48:33.457-0400","caller":"events/events_controller.go:563","msg":"executing command","json":{"gh-request-id":"X-Github-Delivery=f4948200-5627-11ed-9925-aae3d0ca0550"}} {"level":"debug","ts":"2022-10-27T14:48:33.457-0400","caller":"server/middleware.go:72","msg":"POST /events – respond HTTP 200","json":{}} {"level":"debug","ts":"2022-10-27T14:48:33.764-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.get_pull_request.execution_time","value":0.306741833,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:34.127-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.get_pull_request.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:34.127-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github_event.comment_created.success_200","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:34.181-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.update_status.execution_time","value":0.416329709,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:34.181-0400","caller":"vcs/github_client.go:254","msg":"GET /repos/chronosphereio/atlantis_test/pulls/5/reviews","json":{}} {"level":"debug","ts":"2022-10-27T14:48:34.397-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.pull_is_approved.execution_time","value":0.2156325,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:34.694-0400","caller":"vcs/github_client.go:395","msg":"AllowMergeableBypassApply feature flag is enabled - attempting to bypass apply from mergeable requirements","json":{}} {"name":"atlantis.github.update_status.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:35.127-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.pull_is_approved.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:35.215-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.pull_is_mergeable.execution_time","value":0.818263292,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:35.231-0400","caller":"events/project_command_builder.go:598","msg":"Merging config for project at dir: \"terraform\" workspace: \"default\"","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.231-0400","caller":"valid/global_cfg.go:247","msg":"MergeProjectCfg started","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.231-0400","caller":"valid/global_cfg.go:488","msg":"setting apply_requirements: [] from default server config","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.231-0400","caller":"valid/global_cfg.go:488","msg":"setting workflow: \"default\" from default server config","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.231-0400","caller":"valid/global_cfg.go:488","msg":"setting allowed_overrides: [apply_requirements] from repos[1], id: /.*/","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"valid/global_cfg.go:488","msg":"setting allow_custom_workflows: false from default server config","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"valid/global_cfg.go:488","msg":"setting delete_source_branch_on_merge: false from default server config","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"valid/global_cfg.go:255","msg":"overriding server-defined apply_requirements with repo settings: [mergeable]","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"valid/global_cfg.go:295","msg":"MergeProjectCfg completed","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"valid/global_cfg.go:298","msg":"final settings: apply_requirements: [mergeable], workflow: default","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"events/project_command_context_builder.go:95","msg":"Building project command context for apply","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"info","ts":"2022-10-27T14:48:35.232-0400","caller":"events/project_command_context_builder.go:294","msg":"cannot determine which version to use from terraform configuration, detected 0 possibilities.","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.232-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.cmd.comment.apply.builder.execution_time","value":0.016570459,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:35.550-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.update_status.execution_time","value":0.317586084,"tags":{},"type":"timer"}} {"level":"info","ts":"2022-10-27T14:48:35.550-0400","caller":"runtime/apply_step_runner.go:39","msg":"starting apply","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:35.551-0400","caller":"models/shell_command_runner.go:93","msg":"starting \"/opt/homebrew/bin/terraform apply -input=false \\\"/Users/jeffrose/.atlantis/repos/chronosphereio/atlantis_test/5/default/terraform/default.tfplan\\\"\" in \"/Users/jeffrose/.atlantis/repos/chronosphereio/atlantis_test/5/default/terraform\"","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"info","ts":"2022-10-27T14:48:35.911-0400","caller":"models/shell_command_runner.go:156","msg":"successfully ran \"/opt/homebrew/bin/terraform apply -input=false \\\"/Users/jeffrose/.atlantis/repos/chronosphereio/atlantis_test/5/default/terraform/default.tfplan\\\"\" in \"/Users/jeffrose/.atlantis/repos/chronosphereio/atlantis_test/5/default/terraform\"","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"info","ts":"2022-10-27T14:48:35.912-0400","caller":"runtime/apply_step_runner.go:58","msg":"apply successful, deleting planfile","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:36.126-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.pull_is_mergeable.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:36.126-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.cmd.comment.apply.builder.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:36.126-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.update_status.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:36.126-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.projects","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:36.242-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.update_status.execution_time","value":0.32952325,"tags":{},"type":"timer"}} {"level":"info","ts":"2022-10-27T14:48:36.242-0400","caller":"events/instrumented_project_command_runner.go:53","msg":"apply success. output available at: https://github.com/chronosphereio/atlantis_test/pull/5","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:36.242-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.cmd.comment.apply.execution_time","value":1.009759959,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:36.242-0400","caller":"vcs/github_client.go:175","msg":"POST /repos/chronosphereio/atlantis_test/issues/5/comments","json":{}} {"level":"debug","ts":"2022-10-27T14:48:37.048-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.create_comment.execution_time","value":0.805921834,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:37.048-0400","caller":"events/db_updater.go:25","msg":"updating DB with pull results","json":{"repo":"chronosphereio/atlantis_test","pull":"5"}} {"level":"debug","ts":"2022-10-27T14:48:37.126-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.create_comment.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:37.127-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.update_status.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:37.127-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.cmd.comment.apply.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:37.365-0400","caller":"server/middleware.go:45","msg":"POST /events – from [::1]:65211","json":{}} {"level":"debug","ts":"2022-10-27T14:48:37.366-0400","caller":"events/events_controller.go:98","msg":"handling GitHub post","json":{}} {"level":"debug","ts":"2022-10-27T14:48:37.391-0400","caller":"events/events_controller.go:163","msg":"request valid","json":{"gh-request-id":"X-Github-Delivery=f6ffb5a0-5627-11ed-8a4b-823051ce86cb"}} {"level":"debug","ts":"2022-10-27T14:48:37.391-0400","caller":"server/middleware.go:72","msg":"POST /events – respond HTTP 200","json":{}} {"level":"debug","ts":"2022-10-27T14:48:37.425-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.github.update_status.execution_time","value":0.333658875,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:37.425-0400","caller":"metrics/debug.go:52","msg":"timer","json":{"name":"atlantis.cmd.comment.apply.execution_time","value":3.967310833,"tags":{},"type":"timer"}} {"level":"debug","ts":"2022-10-27T14:48:38.125-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github.update_status.execution_success","value":1,"tags":{},"type":"counter"}} {"level":"debug","ts":"2022-10-27T14:48:38.126-0400","caller":"metrics/debug.go:42","msg":"counter","json":{"name":"atlantis.github_event.comment_created.success_200","value":1,"tags":{},"type":"counter"}} ```

Environment details

Atlantis version:

❯ atlantis version
atlantis 0.20.1

Environment var:

❯ env | grep MERGE
ATLANTIS_GH_ALLOW_MERGEABLE_BYPASS_APPLY=true

command:

❯ atlantis server \
--atlantis-url="$URL" \
--gh-user="$USERNAME" \
--gh-token="$TOKEN" \
--gh-webhook-secret="$SECRET" \
--repo-allowlist="$REPO_ALLOWLIST" \
--repo-config=./repos.yaml \
--skip-clone-no-changes \
--log-level=debug

repos.yaml

repos:
- id: /.*/
  allowed_overrides: [apply_requirements]

atlantis.yaml

version: 3
projects:
  - dir: terraform
    apply_requirements: [mergeable]

Github Protected Branch set on Require conversation resolution before merging

Additional Context

Github API spec link go-github GetCombinedStatus link atlantis GetCombinedStatusMinusApply link

nitrocode commented 1 year ago

@jffrose thanks for adding this issue. There must have been something missed in this PR https://github.com/runatlantis/atlantis/pull/2436

cc: @rayterrill could you take a look?

Somehow atlantis is skipping PullIsMergeable if ATLANTIS_GH_ALLOW_MERGEABLE_BYPASS_APPLY=true ?

Perhaps it's in this block that the PR needs to be checked if it's mergeable ?

https://github.com/runatlantis/atlantis/blob/5d7c9cfb8a1811ac9cee0e6fc20ef7d84b5d5208/server/events/vcs/github_client.go#L426-L448

Perhaps it's doing what was implemented ? Bypassing the mergeable requirements ? 🤔

rayterrill commented 1 year ago

I've moved on to a new gig where I don't have an easy place to test this at the moment. I would need to build up a test environment for atlantis.

Unfortunately, this is sort of the issue with this functionality - we're trying to back into a configuration that github doesn't actually support, with ample opportunity to miss something along the way because there are so many moving parts to how this works (and github has a number of apis that need to be consulted to see the actual "status" of a PR).

@jffrose what does the github GUI look like in the state where it would allow you to apply?

jffrose commented 1 year ago

Merging is blocked but then atlantis apply still passes

rayterrill commented 1 year ago

My guess is the issue is in here: https://github.com/runatlantis/atlantis/blob/9ab3fd24163d85b62493bfb0e6b7fbd173598a84/server/events/vcs/github_client.go#L288-L345. We're trying to back into whether or not the PR should be allowed to proceed, trying to look at everything EXCEPT the atlantis/apply check. Specifically I bet something needs to be added in here: https://github.com/runatlantis/atlantis/blob/9ab3fd24163d85b62493bfb0e6b7fbd173598a84/server/events/vcs/github_client.go#L319-L342 to check for this specific scenario (and other similar ones) - must be missing now.

I'll try to set up an Atlantis test case this wknd.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.'

tpolekhin commented 1 year ago

@nitrocode I am willing to look at this issue, because I think it breaks a desired workflow. Not sure how to attack it though, except for adding additional checks in the PullIsMergeable function for all the different "blockers" of the PR merge. If you have any ideas please share.

nitrocode commented 1 year ago

@tpolekhin that sounds exactly like what we'd need to do. Please propose a pr. We'd love to see this get into the repo :smile:

stasostrovskyi commented 1 year ago

Seems like Repository Rules feature from GitHub can solve this issue, see my comment

runatlantis / atlantis