-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Able to apply despite GitHub's PR not being mergeable #2112
Comments
@andoriyu is it similar to this issue ? |
@chenrui333 Hard to tell. According to atlantis documentation, mergeable requirement includes review from CODEOWNERS already. My issue isn't that I want PR to mergeable only after atlantis did apply, my issue is that atlantis applies unapproved changes. |
Got you. |
I found that adding the env var of |
Noticed another thing, I was able to trigger apply as Org admin that can technically merge anytime by clicking checkbox. A regular contributor wasn't able to apply before approval. It kinda makes sense, but it still goes against what documentation (and code comments) say what should happen - can't apply before button is green. |
Ran into same issue, we have a GitHub organization with default
|
I think there's potentially a regression after #2053 If I understand correctly, the condition in atlantis/server/events/vcs/github_client.go Line 319 in 76d7080
atlantis/apply and it'd return true for PullIsMergeable even if it hasn't been approved.
Furthermore, even if we have
anyone can approve the PR and merge it, as long as the only check missing is |
I've got a workaround working using a custom apply step (we use
This uses the GraphQL API to query for the PR It's definitely a quick workaround and could be improved, but it saves us from having changes applied with any review (not coming from a code owner). I'd like to find some time and add this back into the codebase. Hopefully next week. |
We recently upgraded to |
I reached out as requested to |
if someone has time to create a PR or look into where the issue was introduced, please let me know. |
We reverted the code that caused the regression. Could you help verify if the issue is resolved in v0.19.7-pre.20220713? Thank you! |
Seems to me it would be better to move |
@lilincmu I will try to find time to test it out next week. |
Any chance we could go this route instead of giving up on having a way to enforce users don't merge PRs without applying? |
@chicocvenancio Unfortunately we had to revert it due to the regression. However, #2436 is another attempt to add in the feature without breaking existing use cases. It would be highly appreciated if you can share your knowledge there. Hopefully, we can get it right this time 🤞 |
is this still happening with |
I will see if I can test this in our org. |
This seems to have been fixed for us. Previously it was possible to run Info from our environment in case it helps anyone:
|
Make sure to change ref: https://www.runatlantis.io/docs/server-configuration.html#gh-allow-mergeable-bypass-apply |
I actually realized earlier and made this other comment about it and still pasted here the wrong name 🤦🏻♂️ . Edited now, thanks! 😄 |
I'm closing this for now since this seems fixed with the new flag mentioned above. Please use that flag with the latest version to see if it works as expected. |
Community Note
Overview of the Issue
Atlantis applies change set even when PR has pending CODEOWNERS reviews.
Merge button is read:
![pr_merge](https://user-images.githubusercontent.com/148123/156667035-af611b2d-1c06-42ac-9030-0d55f331f2e7.PNG)
Mergeable_status is blocked:
Reproduction Steps
Open a PR that requires CODEOWNERS approval.
Comment
atlantis apply
Atlantis will apply plan, but fail automerge because its blocked.
Logs
Environment details
If not already included, please provide the following:
atlantis server --config /etc/atlantis/config.yaml
Atlantis server-side config file:
Repo
atlantis.yaml
file:Additional Context
The text was updated successfully, but these errors were encountered: