search

runatlantis / atlantis

Terraform Pull Request Automation
https://www.runatlantis.io
Other
7.82k stars 1.06k forks source link

Manually planned projects/directories keep `atlantis/apply` check at `0/0` which allows merging without applying #3852

Open nitrocode opened 1 year ago

nitrocode commented 1 year ago

Community Note

Overview of the Issue

Prior to v0.26.0 and v0.25.0, atlantis would allow us to plan projects/directories manually and this would prevent the atlantis/apply check from being green until an atlantis apply was commented

image

Reproduction Steps

  1. Create a PR as a no-op PR that doesn't affect any projects
  2. Plan a project outside of the scope of the PR's file changes
  3. Notice that atlantis/plan is 1/1 and atlantis/apply is 0/0

Logs

Environment details

Additional Context

lukemassa commented 1 year ago

I think it is definitely related, but I'm still trying to understand what's going on. This is what I'm seeing:

0.24.0: When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 0/0
manual unrelated plan with some change 1/1 0/0
0.25.0 (which introduced #3378 to mark as apply successful if nothing to do): When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 1/1
manual unrelated plan with some change 1/1 0/1
0.26.0 (where I added introduced #3747 to prevent gitlab from being stuck in pending state): When Plan check Apply Check
Created MR 0/0 00
manual unrelated plan with no change 1/1 1/1
manual unrelated plan with some change 1/1 0/0

@nitrocode what do you expect to see? Semantically, given #3378, where we are now seems correct to me: after the plan where changes were introduced, we have done 1/1 plans and, having not attempted any applies, we've done 0/0 of them.

It might be a subtle gitlab vs github thing; in gitlab "atlantis/apply" shows up from the very beginning, as green. Is that not true in github?

nitrocode commented 1 year ago

Thanks @lukemassa for looking into this and providing all the necessary PRs.

It might be a subtle gitlab vs github thing; in gitlab "atlantis/apply" shows up from the very beginning, as green. Is that not true in github?

I think this is the difference. In github, the atlantis/apply does not show up as green and needs to be turned green once an atlantis apply comment is applied which then allows the PR to get merged.

jamengual commented 1 year ago

@lukemassa do you think you could work on a fix for this? and if that is the case document the nuance between gitlab and github for the status too.

lukemassa commented 1 year ago

I'm happy to work on this but I'm not sure yet what should be done. If I understand correctly, and if @nitrocode is indeed seeing this starting on 0.25.0, it seems like it might be a "feature not a bug" of #3378. That PR added logic that updated the apply check on the plan run if there were no changes, which I believe would cover this case. Short of reverting that feature, I'm not sure what could be done. I might be missing something though.

jamengual commented 1 year ago

@chroju do you have any input on this?

chroju commented 1 year ago

Sorry, but I do not understand what this issue is aiming to resolve. I will only explain the changes made by #3378.

With the merger of #3378, running atlantis plan on a PR will now update the atlantis/apply status. In this case, if the number of PullStatus.Projects matches the number of projects for which apply is successful, or if the plan result is No Changes, then the atlantis/apply status will be considered successful. Therefore, if a atlantis plan is executed for a project that has not been modified by the PR, as @lukemassa wrote, the apply status will be 0/1. This would be pending status on GitHub.

I'm not familiar with the situation prior to v0.25.0. I believe that at that time, even if you ran atlantis plan, the apply status would not have been updated. Therefore, if a PR with no *.tf files changes was made, the atlantis/apply status would be green, and if a plan was executed, it should have continued to be green at 0/0.

brandon-fryslie commented 10 months ago

With the merger of https://github.com/runatlantis/atlantis/pull/3378, running atlantis plan on a PR will now update the atlantis/apply status.

This is a huge mistake. It could be acceptable to automatically run apply if plan shows 0 changes, but plan doesn't contain any of the logic for apply and having atlantis show 0 changes on a plan is not equivalent to running a successful apply in quite a few more complicated environments. Look at how many bugs are caused by this, and this is just what is currently reported.

It feels strange to ask, but can we have a way to require Atlantis actually run apply? If I'm not mistaken there isn't actually any way to require apply to run before setting the atlantis/apply status to "success" anymore. I consider that a core feature of Atlantis.

GenPage commented 9 months ago

With the merger of #3378, running atlantis plan on a PR will now update the atlantis/apply status.

This is a huge mistake. It could be acceptable to automatically run apply if the plan shows 0 changes, but plan doesn't contain any of the logic for apply, and having Atlantis show 0 changes on a plan is not equivalent to running a successful apply in quite a few more complicated environments. Look at how many bugs are caused by this, and this is just what is currently reported.

It feels strange to ask, but can we have a way to require Atlantis actually run apply? If I'm not mistaken there isn't actually any way to require apply to run before setting the atlantis/apply status to "success" anymore. I consider that a core feature of Atlantis.

Before considering any action, we need to understand the problem correctly. There are multiple. The ask to change #3378 is not related to this issue about allowing manual plan/applies of projects unrelated to the PR. It should be a separate issue referencing #3378.

This issue is that the apply status checks have different behavior in GitLab vs. GitHub. However, the logic for handling these checks is in the core event logic that affects both VCS providers. It might be best to investigate moving logic to their client implementations respectfully.

I want to advocate for not imposing an opinionated way people must run their workflows so we're not constantly ping-ponging on the issue. Workflows in GH vs GL are different and continues to bite us. @lukemassa @chroju @nitrocode Let's continue the discussion around the title of the issue, "Manually planned projects/directories keep atlantis/apply check at 0/0 which allows merging without applying".

I think it is definitely related, but I'm still trying to understand what's going on. This is what I'm seeing:

0.24.0:

When Plan check Apply Check Created MR 0/0 00 manual unrelated plan with no change 1/1 0/0 manual unrelated plan with some change 1/1 0/0 0.25.0 (which introduced #3378 to mark as apply successful if nothing to do):

When Plan check Apply Check Created MR 0/0 00 manual unrelated plan with no change 1/1 1/1 manual unrelated plan with some change 1/1 0/1 0.26.0 (where I added introduced #3747 to prevent gitlab from being stuck in pending state):

When Plan check Apply Check Created MR 0/0 00 manual unrelated plan with no change 1/1 1/1 manual unrelated plan with some change 1/1 0/0 @nitrocode what do you expect to see? Semantically, given #3378, where we are now seems correct to me: after the plan where changes were introduced, we have done 1/1 plans and, having not attempted any applies, we've done 0/0 of them.

It might be a subtle gitlab vs github thing; in gitlab "atlantis/apply" shows up from the very beginning, as green. Is that not true in github?

This is a good summary by Luke. Do we need to compensate somewhere in GH for the fact the atlantis/apply is not immediately available in GH like in GL?