Expected behaviour from --discard-approval-on-plan flag

[serhatcetinkaya]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

Overview of the Issue

We started using --discard-approval-on-plan flag on our atlantis setup expecting it will discard any approvals before plan is executed. The observed behaviour is different from what we expected.

Reproduction Steps

Expected behaviour:

• create a pull request changing file_A causing a change in resource_A
• before plan is executed and posted to PR, someone approves the PR
• atlantis runs plan and while posting the output to the PR, discards any approvals

Observed behaviour - scenario 1:

• create a pull request changing file_A causing a change in resource_A
• before plan is executed and posted to PR, someone approves the PR
• atlantis runs plan and posts plan output to the PR, approval is still there

Observed behaviour - scenario 2:

• create a pull request changing file_A causing a change in resource_A
• atlantis runs plan and posts plan output to the PR
• someone approves the PR
• do another commit in the same branch changing something in file_B causing a change in resource_B (so that plan output changes)
• atlantis runs plan again posts the new output to the PR and approve is still there

Observed behaviour - scenario 3:

• create a pull request changing file_A causing a change in resource_A
• atlantis runs plan and posts plan output to the PR
• someone approves the PR
• do another commit in the same branch changing something in file_B causing a change in resource_B (so that plan output changes)
• atlantis runs plan again posts the new output to the PR and approve is still there
• comment atlantis plan in the PR to trigger a new plan
• atlantis runs plan again posts the new output to the PR and approve is still there

Observed behaviour - scenario 4:

• create a pull request changing file_A causing a change in resource_A
• atlantis runs plan and posts plan output to the PR
• someone approves the PR
• comment PR atlantis apply
• apply takes too long on resource_A which causes timeout error but the change is reflected in the resource_A
• comment atlantis plan in the PR to trigger a new plan
• atlantis runs plan and posts no changes comment and discards the approval with comment Dismissing reviews because of plan changes

Logs

Logs contain sensitive information I have to redact them first, it will be uploaded if needed. I checked there are no errors or anything related to approval discard.

Environment details

• Atlantis version: v0.23.3
• Deployment method: running on ecs & deployed with terraform
• If not running the latest Atlantis version have you tried to reproduce this issue on the latest version: No
• Flag is enabled with environment variable ATLANTIS_DISCARD_APPROVAL_ON_PLAN=true

Additional Context

We are trying to understand what is the expected behaviour for this flag. We were expecting it to discard approvals in scenarios 1,2 & 3. Can you confirm this is the correct behaviour ?

In the code it says:

// DiscardApprovalOnPlan controls if all already existing approvals should be removed/dismissed before executing
// a plan

what is the test criteria to decide if any approval should be removed ?

[nitrocode]

See references

• feat: --discard-approval-on-plan to dismiss approvals when planning #2696
• After PR approval, it's possible to plan and apply any directory #1508

I defer to the PR author @secustor for more details as I have not used this feature

[secustor]

What do you use as VCS?
My PR enables this only for Github.

[serhatcetinkaya]

@nitrocode thanks, @secustor we use github enterprise, it works but I guess only when state changes out of PR. is that correct ?

[serhatcetinkaya]

another point is I didn't mean it doesn't work, it works but I didn't expect it to work this way. for example if you check scenario 1, I would expect the flag to dismiss that approval but it didn't. in scenario 4 it worked fine

what I am trying to understand is what should we expect from this flag, what conditions should be met for this to dismiss approvals ?

[secustor]

You can go to https://docs.github.com/en/graphql/overview/explorer:
And use following query ( obviously adapted to your repos ) to check what reviews are is considered for dismissal.

{
  repository(owner: "secustor", name: "renovate_terraform_lock_in_subdirectory") {
    pullRequest(number: 13) {
      reviewDecision
      reviews(first: 10, states: APPROVED) {
        pageInfo {
          endCursor
          hasNextPage
        }
        nodes {
          id
          submittedAt
          state
          author {
            login
          }
        }
      }
    }
  }
}

[serhatcetinkaya]

thanks for the information provided, but I didn't really understand how is this going to help me understand what conditions is expected to dismiss an approval. should I come up with different kinds of scenarios and trace each one to see if approvals are considered for dismissal ?

wouldn't it be easier to update docs saying something like: "this, this and that condition needs to be met to dismiss a review" ?

[serhatcetinkaya]

@nitrocode @secustor hello again,

can you please let me know if you have any intention to update documentation to make it clear how this flag works ? if you think it is already clear, I will close this ticket not to waste any more time from both sides.

thanks in advance

[nitrocode]

@serhatcetinkaya Please feel free to propose a pr to make the docs clearer

[brandon-fryslie]

As far as I can tell, this isn't documented anywhere at all.

[jamengual]

yes, sadly somehow we all missed the fact that docs where not added.
if you have time we will really appreciate the PR.