search

runatlantis / atlantis

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

Automerge should only merge if there is a plan #715

Open majormoses opened 5 years ago

majormoses commented 5 years ago

Expected Behavior

There are times where you want to make a plan without actually updating any files. This can be useful if you accidentally merge a PR without applying it, reverting a change that was done out of band, etc.

The following workflow would be supported:

  1. Open a PR with an "empty commit"
  2. Atlantis detects no files changed, does not create a plan
  3. Someone approves PR
  4. Branch is out of date (either because another change merged into the target branch or was behind when branch was created off target)
  5. Submitter rebases + pushes changes
  6. Atlantis does not auto merge. If there is no plan run then it should be up to a person to decide when it is merged as atlantis lacks any other potential context since it does not understand the change.

Current Behavior

When auto merging is turned on even if there is no plan it will auto merge on update.

Here is an example workflow to replicate:

  1. Open a PR with an "empty commit"
  2. Atlantis detects no files changed, does not create a plan
  3. Someone approves PR satisfying CODEOWNERS branch protection (optional)
  4. Submitter realizes their branch is behind the base
  5. When rebasing and pushing changes atlantis merges the
dedamico commented 5 years ago

based on initial testing and code review, this is not an automerge issue but an autoplan issue. When running with automerge set to false, this same behavior occurs. @majormoses if you get a chance to confirm this on your side that would be great (that this also happens when automerge is disabled).

I am continuing to investigate/debug.

dedamico commented 5 years ago

@majormoses I have been doing some testing on this. I am able to recreate the closing of the PR with a GitHub repo that is not tied to Atlantis whatsoever by following your recreate steps. So far this appears to be a GitHub issue (not Atlantis), but a few things that will help us confirm:

  1. When you see the merge/closed PR, is the user that closed the PR shown as the user who created the PR?

2) Are you able to see this same behavior in a GH repo that is not tied or related to Atlantis?

Thanks!

ddaws commented 1 year ago

Running into a similar issue. I have disabled Atlantis on a high traffic repo to avoid lock contention among teams (forcing team members to explicitly request a plan to grab the lock). There have been several times that team members have approved and applied without planning. Atlantis will apply 0 changes and automerge the MR (when automerging is enabled)

Ideally applies fail when there are no plans. Does it make sense to change Atlantis' behavior or is this something that should be expressed as a pre workflow hook?

nitrocode commented 1 year ago

@ddaws as an alternative, you may be able to disable locking https://github.com/runatlantis/atlantis/issues/2876

ddaws commented 1 year ago

@nitrocode do you mean disable locking but enable auto planning, so every MR produces a plan but they don't compete for a lock?

I think the issue is that the plan and apply for this repo can take some time (~10 min). We are working on breaking up the state but this is still a big window for conflicts to occur 😢