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

fix: set policy_check status to success for PRs with no modified projects #3780

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

finnag
Copy link
Contributor

@finnag finnag commented Sep 19, 2023

what

set policy_check status to successful after a atlantis plan command finds no projects that are modified. This makes
atlantis plan behave like autoplan in this regard.

why

If you have set policy_check as a required status in github, this would only work with autoplan for PRs that did not modify any projects. The code path for autoplan and "atlantis plan" were different in this regard, but autoplan is not always appropriate.

tests

  • I have tested my changes by make test-all

references

@finnag finnag requested a review from a team as a code owner September 19, 2023 08:23
@github-actions github-actions bot added the go Pull requests that update Go code label Sep 19, 2023
if err := p.commitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.PolicyCheck, 0, 0); err != nil {
ctx.Log.Warn("unable to update commit status: %s", err)
}
if err := p.commitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.Apply, 0, 0); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Can we make sure this doesn't affect issues like #3725 and cause a regression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I understand what you are asking here - this part of the code just replicates what autoplan does. Is autoplan broken wrt #3725?

Copy link
Member

@GenPage GenPage Oct 6, 2023

Choose a reason for hiding this comment

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

I wanted to clarify that we are considering all possible states and have tested for them. #3747 and #3378 are better contexts around commit statuses and the issues with trying to optimize them

Especially since we are not just adding PolicyCheck but Apply commit statuses as well with this PR

@finnag finnag force-pushed the policycheck-success-on-no-projects branch from 3244d82 to f95ff71 Compare September 27, 2023 10:09
@jamengual jamengual added the waiting-on-review Waiting for a review from a maintainer label Sep 27, 2023
@GenPage GenPage added waiting-on-response Waiting for a response from the user and removed waiting-on-review Waiting for a review from a maintainer waiting-on-response Waiting for a response from the user labels Dec 11, 2023
Autoplan would set the "policy_check" status to successful if
there were no modified projects in a PR, but "atlantis plan"
would not. Changed "atlantis plan" to behave like autoplan
in this regard.
@GenPage GenPage force-pushed the policycheck-success-on-no-projects branch from 49f3389 to ced9146 Compare December 11, 2023 18:48
@GenPage GenPage enabled auto-merge (squash) December 11, 2023 18:48
@GenPage GenPage added this to the v0.27.0 milestone Dec 11, 2023
@GenPage GenPage added bug Something isn't working conftest-policy labels Dec 11, 2023
@GenPage GenPage merged commit 562a151 into runatlantis:main Dec 11, 2023
24 checks passed
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
Autoplan would set the "policy_check" status to successful if
there were no modified projects in a PR, but "atlantis plan"
would not. Changed "atlantis plan" to behave like autoplan
in this regard.
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
Autoplan would set the "policy_check" status to successful if
there were no modified projects in a PR, but "atlantis plan"
would not. Changed "atlantis plan" to behave like autoplan
in this regard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working conftest-policy go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants