atlantis
atlantis copied to clipboard
Feature request: allow apply on merge
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
Currently when a PR is merged without atlantis apply
we end up in a situation where reality doesn't match desired state described with terraform. There is also no way to apply changes post-merge without raising a dummy PR.
At the same time, in GitHub, it's sometimes not possible to prevent people form merging PRs. For example anyone with repo admin access will get implicit push access and thus will be able to merge approved PRs without having to apply first.
In practice this means that there is currently no way to prevent human errors (people merging approved PRs without running atlantis apply
first).
I'd like to propose optional functionality to allow atlantis to be configured to apply changes on PR merges. This would allow organizations who cannot remove push or admin access from repos to ensure changes are applied on merges without relying on humans to remember to atlantis apply
I realize that there is a possibility that apply fails and we'll end up being out of sync but this is a trade off that some organizations might be willing to accept.
Reproduction Steps
Open a PR and then merge it without running atlantis apply
first. This will result in code being out of sync with reality and there is no way to retroactively apply changes.
The only workaround I found is to open 2nd dummy PR and ensure it's applied before merging.
Logs
Environment details
I'm on atlantis v 0.18.2 but I believe older/newer versions will also be affected by this.
Additional Context
This feature was requested in 2017 but wasn't implemented back then: https://github.com/runatlantis/atlantis/issues/36
There is also discussion about the mergable limitations here: https://github.com/runatlantis/atlantis/issues/1316
Apply on merge would allow us to create workflow similar to that hashicorp describe in a docs. This makes me think that the functionality is non-controversial as hashicorp themselves describe it.
will setting atlantis/apply
as a required check in branch protection rules help in that case?
note: might be only available on latest version
will setting
atlantis/apply
as a required check in branch protection rules help in that case? note: might be only available on latest version
No it won't work. For atlantis to respect GitHub branch protection rules we need to use mergable
apply requirement. If we set up branch protection rule to require successful atlantis/apply
then the PR will not become mergable
until we apply and we can't apply until it's mergable...
Hey all, To add this feature can we do something like this (Please correct me if I am wrong)? :
- Add a support for a flag
apply-on-merge
in the atlantis server with which users can specify if they want to enable apply post merge. Addpull_request_event
type as merged toPullRequestEventType
inserver/events/models/models.go
. Refer go-github API code - Add condition to allow atlantis to apply when there's flag
apply-on-merge
is set totrue
and PR is merged. - If flag
apply-on-merge
is set to true, bypass the rule for checking the PR statectx.Pull.State != models.OpenPullState
in theserver/events/command_runner.go
. - Make sure that post run hooks (
server/events/post_workflow_hooks_command_runner.go
) uses flagapply-on-merge
to decide not to merge the PR (since its already merged) and continue with removing the atlantis locks. - If state of the PR is open and if
atlantis apply
comment event occurs, check the flagapply-on-merge
and then reject the apply since flagapply-on-merge
flag is set. - If user comments
atlantis apply
on the closed PR, return as usual.
There can be an alternate atlantis requirement for apply-on-merge
flag. So that users can specify that via config file.
Does this make sense? What else can we improve in this to achieve apply
post merge?
Thanks.
Would one of the maintainers be able to access above plan form @satyamz ? We're considering contributing to the project by adding this functionality but would like to know if the plan looks sound and whether a PR would be accepted.
I'm interested in this due to the GitLab API bug mentioned in #1174 that is preventing Atlantis' mergeable
apply requirement from functioning. Having apply-on-merge would let us make sure all the MR approvals are adhered to before the plan can be applied.
I'd like to take this issue one step further:
automatically plan on any change to the PR - failing on other parallel PRs is okay - but maybe make that optional for obvious reasons:
do NOT respond to atlantis apply
manual commands on unmerged files
and finally, apply on merge
this relies on the plan being okay and the git provider approvals/results on the PR being acceptable and merge possible - so no smarts needed there, just dump plan output on every push and do apply and dump output on merge.
This tool is almost perfect on the face of it but the above work flow is the only one that makes sense to me for any serious use.
I'd like to take this issue one step further:
automatically plan on any change to the PR - failing on other parallel PRs is okay - but maybe make that optional for obvious reasons:
I believe plans are regenerated on PR changes already, there is no need to atlantis plan
when new commits are added.
do NOT respond to
atlantis apply
manual commands on unmerged files
Yes absolutely, it's a good point that in this scenario atlantis apply
should not do anything.
and finally, apply on merge
this relies on the plan being okay and the git provider approvals/results on the PR being acceptable and merge possible - so no smarts needed there, just dump plan output on every push and do apply and dump output on merge.
Yes exactly, this is exactly the workflow I'd like to use as well!
This tool is almost perfect on the face of it but the above work flow is the only one that makes sense to me for any serious use.
It's good to see that there are more people who have the same requirements!
We would love something along these lines as feature, as this is something that happens on a regular basis with engineers new to our team, or for engineers from outside our organisation.
The ability to require the atlantis/apply
check to pass for users to merge, and for atlantis to consider a PR mergeable if all required checks except for that check pass, would likely be enough for us.
we would love to see this feature implemented for our team as well, many people forgot to comment atlantis apply
and straight merged the PR. And we can't set the atlantis/apply
status check due to the conflict with mergeable
Speaking as an interested observer, I'm curious if there's something about Atlantis' design that requires it to only run during the PR phase. I ask because I recently came across a notice that said a failure occurring post-merge "would need" to be fixed by a subsequent PR. Is this really mandatory?
If I have a GitHub Action that fails post-merge, I have the option to just re-run it. Granted, if the fault is something within the repository that should never have been merged I'll most likely fix it in a new PR. But if the issue lies outside the repo, if it's something like an expired token or an intermittent network outage, I just need a button.
Is there something fundamental to Atlantis that prevents this sort of capability?
is this still an issue with v0.19.8
?
is this still an issue with
v0.19.8
?
As far as I can tell yes this is still a problem. The tricky part about "apply on merge" functionality is that it's mostly GitHub limitation that we are dealing with. It is currently not possible to confirm whether GitHub merge requirements were met without actually allowing people to merge. And once people merge we have a drift between code and actual state.
Add atlantis/apply
as a required check
Set https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply flag
@nitrocode regardless of the potential workaround I still think this feature would be very useful. Based on thumbs up there are at least 44 others who agree.
We actually added this capability to forked atlantis, we should be in a position to submit a PR soon so that everyone in the community can benefit from it.
Could this issue be reopened?
this can be reopened but keep in mind that people tried to do this before in different ways and a lot of logic was added and broke other functionality since this part of the code is very tricky due to workarounds and limitations on VCSs.
PRs are welcome but please give it a try to the gh-allow-mergeable-bypass-apply that @nitrocode mentioned.
I don't understand what is wrong with the current solution or workaround of using https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply. Please explain why this method is not what you prefer @jacekn
@nitrocode Adding this flag still doesn't work, unless there's some other setting I'm missing. I've setup the atlantis apply check and adding the flag, but after getting an approval I get this message. Once I remove the atlantis/apply check, I'm able to apply normally.
Setting the flag
- name: ATLANTIS_GH_ALLOW_MERGEABLE_BYPASS_APPLY
value: "true"
This is what we have for our apply_requirements
apply_requirements: [approved, mergeable]
Apply Failed: Pull request must be approved by at least one person other than the author before running apply.
@Manan-Kothari
Apply Failed: Pull request must be approved by at least one person other than the author before running apply.
You have mergeable
set and you have PR requirements of 1 approval or more... so you need an approval on the PR before you can merge the PR via atlantis apply
.
I did have that which is why I'm asking
![image](https://user-images.githubusercontent.com/13862485/220150681-53098385-ebe6-4e71-860b-67fd374d43f1.png)
I don't understand what is wrong with the current solution or workaround of using https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply. Please explain why this method is not what you prefer @jacekn
Here are the reasons I can think of:
- If atlantis just ignores "atlantis/apply" status check then we would require users to know internals of atlantis and know that changes can be applied without this particular status check. Basically there would be no green "merge" button to indicate all requirements are met.
- It's not unusual for projects to act on new commits in the main/master branch. For example those can trigger builds or deploys to dev. With this workflow being used in an organization it wold be nice for atlantis to behave in the same way. It's a matter of choice of course. Driving things from GH comments is a valid method to interact with systems too.
- The bypass method does not allow admins to restrict who can land changes. With apply on merge admins can control, via GitHub repo perms, who can merge changes thus apply them. People who can apply can be subset of those who can open PRs
- The "apply on merge" workflow is what hashicorp describe, for example here. It would be nice to be able to replicate this workflow in atlantis without the need for terraform cloud. Given that the workflow is described by hashicorp I think this workflow should not be considered controversial.
@nitrocode quick question about the --gh-allow-mergeable-bypass-apply
flag as it's not clear from the docs. With the flag set will atlantis ensure that all non-status-check branch protection rules are passing? For example branches might require conversation resolution or signed commits. Will those be taken into account and will atlantis only allow changes to be applied if all rules are met?
@jacekn number 4 exactly - terraform cloud works quite nicely in this regard, though it'd be nice to have some alternative to it that isn't proprietary and pay per view.
This stuff is trivial to achieve in something like Jenkins. You need some sort of call back event or webhook on new updates to the default branch ref spec in the remote - eg direct push or PR merge of any kind.
The comment in the docs about lack of status checks in the branch protection rules seems bizarre at first - you can configure them to require any/all status checks and then when some kind of CI/CD registers one against it, suddenly that becomes required, for example.
PR created = plan Any update to default branch refspec = apply
How hard can it be? I don't understand how that option helps at all, @nitrocode / @jamengual ? Seems to be orthogonal to the request here.
we are not opposed to add a new functionality that can solve this problem.
there is many ways mentioned that try in one way or another to solve this issue but it looks like at the end of the day something like - - apply-on-merge flag is necessary for those admins that do not like to apply their changes before merging.
disclaimer: i did not yet install or use atlantis, rather waiting for this feature to be implemented before trying out.
question: could apply-on-merge be somehow achieved with existing atlantis version, if we would trigger some github action on "push to master" which would then do an api call to atlantis? in this case it could be possible to say that merge condition is "atlantis plan passed without errors, and codeowner gave approval to merge" (that would be taken care of completely at github side) and then after merge, push to master would happen triggering "something" via atlantis http api. I can imagine that main problem could be to somehow infer which exactly prepared plan needs to be run at atlantis side, and then it would just boild down to streaming atlantis api output as github action output for real-time progress report and merge result outcome (pass|errored)
yes, that is possible.
On Thu, Feb 23, 2023, 12:39 a.m. Ervin Weber @.***> wrote:
disclaimer: i did not yet install or use atlantis, rather waiting for this feature to be implemented before trying out.
question: could apply-on-merge be somehow achieved with existing atlantis version, if we would trigger some github action on "push to master" which would then do an api call to atlantis? in this case it could be possible to say that merge condition is "atlantis plan passed without errors, and codeowner gave approval to merge" (that would be taken care of completely at github side) and then after merge, push to master would happen triggering "something" via atlantis http api. I can imagine that main problem could be to somehow infer which exactly prepared plan needs to be run at atlantis side, and then it would just boild down to streaming atlantis api output as github action output for real-time progress report and merge result outcome (pass|errored)
— Reply to this email directly, view it on GitHub https://github.com/runatlantis/atlantis/issues/2172#issuecomment-1441380006, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERHFBI574ZE2BFHESU3WY4O5ZANCNFSM5SCAIYHA . You are receiving this because you were mentioned.Message ID: @.***>
still an issue with 0.27.2
@jacekn I tried to answer your questions here: https://github.com/runatlantis/atlantis/issues/2172#issuecomment-1438098475 (i.e have a required atlantis/apply status check + atlantis having ATLANTIS_GH_ALLOW_MERGEABLE_BYPASS_APPLY set to true + apply_requirements: [approved, mergeable, undiverged]), and it didn't respect the fact that there was no approving PR review in the branch protection rule :/
cc @nitrocode
@dimisjim I think you need to open a separate bug for your case and attach your branch protection configuration. Note that currently that flag in Atlantis only works with branch protection but not repository rulesets and also it only takes into account review state and required status checks and contains a couple of bugs, but there is an open PR to fix them.
Indeed: https://github.com/runatlantis/atlantis/pull/4193. Thanks for the tip
In GitHub there is feature that can be enabled per repo - merge queue, which allows running commands just in the middle between PR schedulled for merge and before the actual merging. We think this is the best place for applying terraform. Is there anyone who successfully configured atlantis to achieve this?