feat: discard pull request approval status on autoplan#6281
feat: discard pull request approval status on autoplan#6281verdel wants to merge 4 commits intorunatlantis:mainfrom
Conversation
Discard existing pull request approvals when autoplan runs on PR updates. This guarantees that any code changes introduced in the PR must be reviewed and approved again. Signed-off-by: Vadim Aleksandrov <valeksandrov@me.com>
Signed-off-by: Vadim Aleksandrov <valeksandrov@me.com>
dosubot
bot
added
feature
There was a problem hiding this comment.
Pull request overview
Adds parity between manual plan runs and autoplan by dismissing existing PR approvals when an autoplan-triggered plan runs, helping ensure approvals reflect the latest Terraform-relevant changes.
Changes:
- Invoke
DiscardReviewsduring autoplan execution when--discard-approval-on-planis enabled. - Add a unit test intended to verify approvals are discarded on autoplan.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
server/events/plan_command_runner.go |
Adds approval-discard logic to runAutoplan. |
server/events/command_runner_test.go |
Adds a test for the new autoplan approval-discard behavior. |
server/events/plan_command_runner.go
Outdated
| if p.DiscardApprovalOnPlan { | ||
| if err := p.pullUpdater.VCSClient.DiscardReviews(ctx.Log, baseRepo, pull); err != nil { | ||
| ctx.Log.Err("failed to remove approvals: %s", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
DiscardReviews is called before building autoplan commands, so approvals will be dismissed even when autoplan ends up running no plans (e.g., BuildAutoplanCommands returns an empty list / no Terraform changes). This can unnecessarily discard approvals on unrelated PR updates. Consider moving the discard logic to after BuildAutoplanCommands succeeds and only when there is at least one plan command to run.
| func TestRunAutoplanCommand_DiscardApprovals(t *testing.T) { | ||
| vcsClient := setup(t, func(testConfig *TestConfig) { | ||
| testConfig.discardApprovalOnPlan = true | ||
| }) | ||
|
|
||
| tmp := t.TempDir() | ||
| boltDB, err := boltdb.New(tmp) | ||
| t.Cleanup(func() { | ||
| boltDB.Close() | ||
| }) | ||
| Ok(t, err) | ||
| dbUpdater.Database = boltDB | ||
| applyCommandRunner.Database = boltDB | ||
|
|
||
| testdata.Pull.BaseRepo = testdata.GithubRepo | ||
| ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User) | ||
| vcsClient.VerifyWasCalledOnce().DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]()) | ||
| } |
There was a problem hiding this comment.
This test doesn't stub BuildAutoplanCommands, so it may pass even if autoplan produces no plan commands (and therefore doesn't validate the intended behavior when an actual autoplan plan runs). To make the coverage meaningful, set BuildAutoplanCommands to return at least one command.ProjectContext (and stub any required plan/workingDir interactions) and then assert DiscardReviews is invoked.
Signed-off-by: Vadim Aleksandrov <valeksandrov@me.com>
| CommandName: command.Plan, | ||
| }, | ||
| }, nil) | ||
|
|
There was a problem hiding this comment.
This test relies on unstubbed mock defaults for workingDir.GetPullDir and projectCommandRunner.Plan, which can cause it to run with an empty pull dir and a zero-value plan result. Consider stubbing those calls (as other tests in this file do) so the test exercises the realistic autoplan path and is less brittle to future changes.
| When(projectCommandRunner.Plan(Any[command.ProjectContext]())).ThenReturn( | |
| command.ProjectCommandOutput{ | |
| PlanSuccess: &models.PlanSuccess{}, | |
| }, | |
| ) | |
| When(workingDir.GetPullDir(Any[models.Repo](), Any[models.PullRequest]())). | |
| ThenReturn(tmp, nil) |
Signed-off-by: Vadim Aleksandrov <valeksandrov@me.com>
what
I am adding functionality to discard PR approvals when the autoplan runs.
This means approvals will be discarded not only when the atlantis plan command is triggered via a PR comment, but also when autoplan runs automatically on PR updates (such as new or modified commits).
why
Atlantis has an option
--discard-approval-on-plan, which is responsible for discarding approvals in a PR when theplancommand is executed. However, if changes are made to the PR after the required approvals have been obtained, their status is not reset.In this case, unwanted changes could be introduced into the PR after approvals, which would later be applied to the Terraform configuration.
I believe it makes more sense to also reset approval status when autoplan runs.
If needed, I can add an additional option
--discard-approval-on-autoplan, which would separately control the functionality of discarding approvals when autoplan is executed.tests
references
This PR closes #6280