public/atlantis
1
0
Fork 0
mirror of https://git.vectorsigma.ru/public/atlantis.git synced 2026-08-04 02:38:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(discardApprovalOnPlan): add missing conditional and tests (#2913)

Browse Source 
* fix(discardApprovalOnPlan): add missing conditional and tests

* tests(discardApprovalOnPlan): match for any Pull request or repo for turned off discarding

* Apply suggestions from code review

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>

* Update server/events/plan_command_runner.go

* Update server/events/plan_command_runner.go

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
This commit is contained in:
Sebastian Poxhofer
2023-01-03 17:21:37 +01:00
committed by GitHub
parent 8b9a70dfb7
commit a508bdb255
4 changed files with 75 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
server/controllers/events/events_controller_e2e_test.go
View File

@@ -972,6 +972,7 @@ type setupOption struct {
func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers.VCSEventsController, *vcsmocks.MockClient, *mocks.MockGithubPullGetter, *events.FileWorkspace) {
allowForkPRs := false
discardApprovalOnPlan := true
dataDir, binDir, cacheDir := mkSubDirs(t)
// Mocks.
@@ -1191,6 +1192,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
silenceNoProjects,
boltdb,
lockingClient,
discardApprovalOnPlan,
)
e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient)

78
server/events/command_runner_test.go
View File

@@ -70,8 +70,27 @@ var importCommandRunner *events.ImportCommandRunner
var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner
var postWorkflowHooksCommandRunner events.PostWorkflowHooksCommandRunner
func setup(t *testing.T) *vcsmocks.MockClient {
type TestConfig struct {
parallelPoolSize int
SilenceNoProjects bool
StatusName string
discardApprovalOnPlan bool
}
func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.MockClient {
RegisterMockTestingT(t)
testConfig := &TestConfig{
parallelPoolSize: 1,
SilenceNoProjects: false,
StatusName: "atlantis-test",
discardApprovalOnPlan: false,
}
for _, op := range options {
op(testConfig)
}
projectCommandBuilder = mocks.NewMockProjectCommandBuilder()
eventParsing = mocks.NewMockEventParsing()
vcsClient := vcsmocks.NewMockClient()
@@ -107,15 +126,12 @@ func setup(t *testing.T) *vcsmocks.MockClient {
GlobalAutomerge: false,
}
parallelPoolSize := 1
SilenceNoProjects := false
StatusName := "atlantis-test"
policyCheckCommandRunner = events.NewPolicyCheckCommandRunner(
dbUpdater,
pullUpdater,
commitUpdater,
projectCommandRunner,
parallelPoolSize,
testConfig.parallelPoolSize,
false,
false,
)
@@ -133,10 +149,11 @@ func setup(t *testing.T) *vcsmocks.MockClient {
pullUpdater,
policyCheckCommandRunner,
autoMerger,
parallelPoolSize,
SilenceNoProjects,
testConfig.parallelPoolSize,
testConfig.SilenceNoProjects,
defaultBoltDB,
lockingLocker,
testConfig.discardApprovalOnPlan,
)
pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient)
@@ -152,10 +169,10 @@ func setup(t *testing.T) *vcsmocks.MockClient {
pullUpdater,
dbUpdater,
defaultBoltDB,
parallelPoolSize,
SilenceNoProjects,
testConfig.parallelPoolSize,
testConfig.SilenceNoProjects,
false,
StatusName,
testConfig.StatusName,
pullReqStatusFetcher,
)
@@ -165,22 +182,22 @@ func setup(t *testing.T) *vcsmocks.MockClient {
projectCommandRunner,
pullUpdater,
dbUpdater,
SilenceNoProjects,
testConfig.SilenceNoProjects,
false,
)
unlockCommandRunner = events.NewUnlockCommandRunner(
deleteLockCommand,
vcsClient,
SilenceNoProjects,
testConfig.SilenceNoProjects,
)
versionCommandRunner := events.NewVersionCommandRunner(
pullUpdater,
projectCommandBuilder,
projectCommandRunner,
parallelPoolSize,
SilenceNoProjects,
testConfig.parallelPoolSize,
testConfig.SilenceNoProjects,
)
importCommandRunner = events.NewImportCommandRunner(
@@ -226,6 +243,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner,
PullStatusFetcher: defaultBoltDB,
}
return vcsClient
}
@@ -609,7 +627,8 @@ func TestRunSpecificPlanCommandDoesnt_DeletePlans(t *testing.T) {
// Test that if one plan fails and we are using automerge, that
// we delete the plans.
func TestRunAutoplanCommandWithError_DeletePlans(t *testing.T) {
setup(t)
vcsClient := setup(t)
tmp := t.TempDir()
boltDB, err := db.New(tmp)
Ok(t, err)
@@ -652,6 +671,35 @@ func TestRunAutoplanCommandWithError_DeletePlans(t *testing.T) {
ch.RunAutoplanCommand(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User)
// gets called twice: the first time before the plan starts, the second time after the plan errors
pendingPlanFinder.VerifyWasCalled(Times(2)).DeletePlans(tmp)
vcsClient.VerifyWasCalled(Times(0)).DiscardReviews(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())
}
func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) {
vcsClient := setup(t, func(testConfig *TestConfig) {
testConfig.discardApprovalOnPlan = true
})
tmp := t.TempDir()
boltDB, err := db.New(tmp)
Ok(t, err)
dbUpdater.Backend = boltDB
applyCommandRunner.Backend = boltDB
autoMerger.GlobalAutomerge = true
defer func() { autoMerger.GlobalAutomerge = false }()
When(projectCommandRunner.Plan(matchers.AnyCommandProjectContext())).ThenReturn(command.ProjectResult{PlanSuccess: &models.PlanSuccess{}})
When(workingDir.GetPullDir(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn(tmp, nil)
pull := &github.PullRequest{State: github.String("open")}
modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, State: models.OpenPullState, Num: fixtures.Pull.Num}
When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(pull, nil)
When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
fixtures.Pull.BaseRepo = fixtures.GithubRepo
ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: command.Plan})
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
lockingLocker.VerifyWasCalledOnce().UnlockByPull(fixtures.Pull.BaseRepo.FullName, fixtures.Pull.Num)
vcsClient.VerifyWasCalledOnce().DiscardReviews(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())
}
func TestFailedApprovalCreatesFailedStatusUpdate(t *testing.T) {

11
server/events/plan_command_runner.go
View File

@@ -24,6 +24,7 @@ func NewPlanCommandRunner(
SilenceNoProjects bool,
pullStatusFetcher PullStatusFetcher,
lockingLocker locking.Locker,
discardApprovalOnPlan bool,
) *PlanCommandRunner {
return &PlanCommandRunner{
silenceVCSStatusNoPlans: silenceVCSStatusNoPlans,
@@ -42,6 +43,7 @@ func NewPlanCommandRunner(
SilenceNoProjects: SilenceNoProjects,
pullStatusFetcher: pullStatusFetcher,
lockingLocker: lockingLocker,
DiscardApprovalOnPlan: discardApprovalOnPlan,
}
}
@@ -68,6 +70,9 @@ type PlanCommandRunner struct {
parallelPoolSize int
pullStatusFetcher PullStatusFetcher
lockingLocker locking.Locker
// DiscardApprovalOnPlan controls if all already existing approvals should be removed/dismissed before executing
// a plan.
DiscardApprovalOnPlan bool
}
func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
@@ -163,8 +168,10 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {
baseRepo := ctx.Pull.BaseRepo
pull := ctx.Pull
if err = p.pullUpdater.VCSClient.DiscardReviews(baseRepo, pull); err != nil {
ctx.Log.Err("failed to remove approvals: %s", err)
if p.DiscardApprovalOnPlan {
if err = p.pullUpdater.VCSClient.DiscardReviews(baseRepo, pull); err != nil {
ctx.Log.Err("failed to remove approvals: %s", err)
}
}
if err = p.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, command.Plan); err != nil {

1
server/server.go
View File

@@ -684,6 +684,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
userConfig.SilenceNoProjects,
backend,
lockingClient,
userConfig.DiscardApprovalOnPlanFlag,
)
pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient)