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: introduce the --set-atlantis-apply-check-successful-if-no-changes flag to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes" #4332

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

carlosaml
Copy link

what

Introduce the --set-atlantis-apply-check-successful-if-no-changes to allow disabling the behavior that automatically
sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes".

An initial version of this flag had been implemented as part of #3378 by @chroju but during that (somewhat lengthy) PR's discussion it was decided that this would be the default behavior and no flag would be added, so a breaking change was introduced as part of v0.25.0.

I decided to use the latest name for the flag (--set-atlantis-apply-check-successful-if-no-changes) from the original PR (before it was removed), and I also left its default value set to true in order to keep backwards compatibility.

why

In our scenario, the motivation for adding this is flag is twofold:

  • We are in a highly regulated environment, so we have need to require an explicit terraform apply command before any changes are merged to the main branch.
  • We need to make sure the state files are always kept up-to-date with the latest code, and with Atlantis skipping the terraform apply step when there's a plan with "No Changes" we are having to manually run an apply command in order to keep the state file up-to-date.

tests

  • server/events/plan_command_runner_test.go
  • integration test run locally by building the app from source and running it in our environment in order to reproduce the issue and ensure that setting the flag to false correctly fixes it

references

This recent comment on #3378 faces the same issue.

I actually do want to require terraform apply to run on every single PR and I'm not seeing how to configure that.

…s to allow disabling the behavior that automatically sets the "atlantis/apply" status check to "passing" on a "no changes" plan
@carlosaml carlosaml requested review from a team as code owners March 10, 2024 22:43
@carlosaml carlosaml requested review from chenrui333, nitrocode and X-Guardian and removed request for a team March 10, 2024 22:43
@github-actions github-actions bot added docs Documentation go Pull requests that update Go code labels Mar 10, 2024
@carlosaml carlosaml changed the title Introduce the --set-atlantis-apply-check-successful-if-no-changes to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes". feat: introduce the --set-atlantis-apply-check-successful-if-no-changes to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes". Mar 10, 2024
@GenPage GenPage added feature New functionality/enhancement needs discussion Large change that needs review from community/maintainers waiting-on-review Waiting for a review from a maintainer labels Mar 10, 2024
@GenPage
Copy link
Member

GenPage commented Mar 10, 2024

@carlosaml Thank you for the PR Carlos. First off, I apologize for the change in behavior, its nor always easy anticipating all use cases. Might I suggest a different approach? In the original discussion, I advocated for adding the funcationlity as another "ApplyRequirement" as no-changes.

#3378 (comment)

I think we can do better. We already have an issue with an overabundance of flags, let alone the long name.
This fits better under command requirements. This would be a new requirement no-changes that is applicable to only ApplyRequirements.

To summarize, the option presented is:

  • Make this default with no flags/configuration.
  • flag to optionally --set-atlantis-apply-check-successful-if-no-changes
  • add a requirement for ApplyRequirements for no-changes instead of another flag

I advocate for #3, intending to remove it later for #1 in the long run.

Given the scenario more thought, #3 seems like the ideal long term solution. The only issue here is ApplyRequirements is intended for "requirements before you can apply", not "requiring applies when there are no changes".

@carlosaml carlosaml changed the title feat: introduce the --set-atlantis-apply-check-successful-if-no-changes to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes". feat: introduce the --set-atlantis-apply-check-successful-if-no-changes flag to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes". Mar 11, 2024
@carlosaml carlosaml changed the title feat: introduce the --set-atlantis-apply-check-successful-if-no-changes flag to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes". feat: introduce the --set-atlantis-apply-check-successful-if-no-changes flag to allow disabling the behavior that automatically sets the atlantis/apply status check to "passing" on a VCS pull request if the atlantis plan command results in "No Changes" Mar 11, 2024
@carlosaml
Copy link
Author

@GenPage thanks for the prompt response and suggestions.

I'm not sure I completely follow your suggestion, but I'm also not that familiar with the codebase so I might be missing something obvious here.

I don't see how an apply requirement for this would be descriptive, since it's certainly not a "requirement before you can apply" but it's also not a planning requirement either.

The only issue here is ApplyRequirements is intended for "requirements before you can apply", not "requiring applies when there are no changes".

How do you envision this option being used in a given project configuration, especially assuming this behavior (setting the apply check to "passing" when there are no changes) is the default now and we'd need a way to toggle it off, otherwise it'd be another breaking change.
Perhaps an example of how the configuration would look like in this case would help.

I could see this being a project-level option like set_atlantis_apply_check_successful_if_no_changes: true but I'm failing to see how this could be integrated with ApplyRequirements.

Thanks.

@carlosaml
Copy link
Author

@GenPage did you have a chance to look at my latest comment above? I'm happy to make changes or even open another PR but I'm not sure I understand how option 3 would tackle this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation feature New functionality/enhancement go Pull requests that update Go code needs discussion Large change that needs review from community/maintainers waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants