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

How can you require an apply before merge? #1316

Closed
kenske opened this issue Dec 15, 2020 · 13 comments
Closed

How can you require an apply before merge? #1316

kenske opened this issue Dec 15, 2020 · 13 comments

Comments

@kenske
Copy link

kenske commented Dec 15, 2020

Our whole Atlantis workflow is working great, except for when we get new users, and they always forget to comment atlantis apply before they merge. How can we require users to apply before merging?

We tried requiring atlantis:apply as a github status check, but then that means atlantis won't run because the branch is not mergeable.

Edit: removing mergeable in apply_requirements is not an options because we run the risk of someone trying to apply their changes on a branch that's not up to date with master.

@rawlbot
Copy link

rawlbot commented Dec 15, 2020

What you need to do is leverage both the Atlantis Apply Requirements and the Github checks (like you are doing).
On the Atlantis side, update the server level Atlantis.yaml to only require Approvals.

apply_requirements: [approved]

This, in combination with the github check you've implemented already should unblock you @kenske.

The workflow should be look like this:

  • PR is submitted and planned
  • Peer reviews code and approves
  • Code Submitter is unable to merge due to status check requirement (Atlantis apply check you already have in place)
  • Code Submitter comments atlantis apply
  • Atlantis can run since change is approved and will merge/close out the PR

@kenske
Copy link
Author

kenske commented Dec 16, 2020

@rawlbot thanks for the suggestion, but I don't want to remove the mergeable condition because we run the risk of someone trying to apply their changes on a branch that's not up to date with master. Is there another way to achieve this?

@ingwarsw
Copy link

ingwarsw commented Dec 16, 2020

@kenske We have it like that..

  • On atlantis:
    • Both approved and mergable is needed
    • Set to automerge
  • On GH:
    • We have CODEOWNERS to allow only our team to approve (otherwise anyone can approve)
    • We require atlantis:plan check pass (sorry originally I wrote apply and apply cannot be set)
    • We have only atlantis user (In aur case GH app) allowed to merge code..

@grimm26
Copy link
Contributor

grimm26 commented Jan 15, 2021

@ingwarsw sounds great except that if we require the atlantis:apply check to pass for the repository, then that screws us for our top level directory that has no terraform files in it (just our top level terragrunt.hcl and stuff).

@grimm26
Copy link
Contributor

grimm26 commented Jan 26, 2021

@ingwarsw If you have mergable set, why do you need approved set?

@grimm26
Copy link
Contributor

grimm26 commented Jan 28, 2021

if atlantis:apply is required for merge, I can't have mergable set because it won't let me apply until I've applied :)

@kenske
Copy link
Author

kenske commented Jan 28, 2021

@grimm26 I'm running into the same issue. I'm considering setting up some sort of PR bot with its own status check. The bot would check for a plan and then require atlantis apply before setting its own check to pass.

@ingwarsw
Copy link

@grimm26 @kenske I fixed my comment.. we are checking atlantis:plan not atlantis:apply .. sorry my mistake..
And yes mergable would be enough on atlantis..

Other than that it works ok and is secure..
CODEOWNERS is most important stuff.. otherwise anyone with any comment access would be able to merge..

@grimm26
Copy link
Contributor

grimm26 commented Jan 29, 2021

I have a setup working now with requiring atlantis/plan and atlantis/apply on github with automerge enabled on atlantis but only setting apply_requirements: [approved], not mergeable. CODEOWNERS is in play, obviously. Seems to be working fine. We have more admins that are allowed to merge, but this keeps them from slapping a green merge button after approvals and plan are done.

@dgteixeira
Copy link

@kenske , you've looked into undiverged in atlantis config?
I'll drop the link: atlantis undiverged

On our end, we force approved and undiverged and have status checks on atlantis plan and on atlantis apply.

Until today I've only found one issue:

  • If a PR + approval is super fast and you have some long-running status checks, you might atlantis apply before the status checks end and the auto-merge in atlantis will fail, because PR is not mergeable, yet.

@grimm26
Copy link
Contributor

grimm26 commented Nov 11, 2021

0.17.5 kinda fixes this, but approved only cares about one approval. We have some terraform PRs that require approvals from multiple teams. atlantis only cares if one approves.

@jacekn
Copy link

jacekn commented Mar 30, 2022

The mergable method has a significant limitation. It only ensures PRs can't be merged if there are no admins of the repo. Any admins will have implicit push permission so will be able to merge PRs as soon as CODEOWNERS review them.

I think the only possible solution is to allow people to choose workflow where atlantis applies changes on merge rather than requiring atlantis apply

@nitrocode
Copy link
Member

Add atlantis/apply as a required check

Set https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants