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

feat: add a pending status for apply when running plan command #2053

Merged
merged 5 commits into from
Mar 4, 2022

Conversation

AndreZiviani
Copy link
Contributor

@AndreZiviani AndreZiviani commented Feb 9, 2022

This PR adds the following improvements:

  • add a pending apply status when a plan command is issued (including autoplan), this allows to configure the repository to only be mergeable if atlantis applied all changes
  • fix apply logic that changed the PR status before checking if it is mergeable
  • if only atlantis/apply status is pending/failing consider the PR mergeable

With these changes we can configure the GitHub repository to only be mergeable if: all status succeed, at least 1 approval from CODEOWNERS and only allow merging after atlantis is finished

Fix #1924, Fix #1791

@AndreZiviani AndreZiviani requested a review from a team as a code owner February 9, 2022 20:22
@AndreZiviani
Copy link
Contributor Author

I'm running with no problems for more than a month, can someone review please?

@AndreZiviani
Copy link
Contributor Author

@chenrui333 could you please review this 🙏

Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!!

@chenrui333 chenrui333 merged commit 1a7cc36 into runatlantis:master Mar 4, 2022
@chenrui333
Copy link
Member

@AndreZiviani thanks for the work on this. 🙏 Gonna cut a new release today!

@rayterrill
Copy link
Contributor

@AndreZiviani is there any configuration required to sue this new functionality? This sounds like exactly what we've been looking for - we want CODEOWNERS to be required before you can apply AND want to make atlantis/apply required.

@AndreZiviani
Copy link
Contributor Author

Hey @rayterrill these are my branch protection rules, working perfectly so far

image

@zxpower
Copy link

zxpower commented Mar 29, 2022

This feature completely broke our workflow in Github Actions where we relied on https://github.com/WyriHaximus/github-action-wait-for-status to continue.

@jamengual
Copy link
Contributor

@AndreZiviani I think this might have broken the status for all other VCS implementations:

look this please #2138

if we do not hear from you soon I will revert this change since is affecting quite a lot of people.

@chenrui333

@AndreZiviani
Copy link
Contributor Author

@jamengual sorry my change broke other workflows :(
I’m not sure how to proceed, I could try adding a feature flag for this, what do you suggest?

@jamengual
Copy link
Contributor

@AndreZiviani this is affecting github users as well so I think we need to revert it back then we can have a prerelease with this feature after you get feedback from people that this is working for them too, unless you can test in other VCSs by yourself.

@AndreZiviani
Copy link
Contributor Author

I agree that a rollback is necessary, I will try to gather feedback and improve this on the following days

krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
…lantis#2053)

* add a pending status for apply when running plan command

* fix status updater logic, we should only update the status after we check if the PR is mergeable like the doc next to it says

* do not dismiss the PR if the only our "atlantis/apply" status is pending/failing

* fix logic and tests

* linting
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

Successfully merging this pull request may close these issues.

atlantis/apply check passes before actual apply Apply Requirement - Approved by CODEOWNERS?
5 participants