diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index ff85371469..29037ec9a0 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -33,6 +33,7 @@ type APIController struct { RepoAllowlistChecker *events.RepoAllowlistChecker Scope tally.Scope VCSClient vcs.Client + CommitStatusUpdater events.CommitStatusUpdater } type APIRequest struct { @@ -150,6 +151,11 @@ func (a *APIController) apiPlan(request *APIRequest, ctx *command.Context) (*com return nil, err } + // Update the combined plan commit status to pending + if err := a.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("unable to update plan commit status: %s", err) + } + var projectResults []command.ProjectResult for i, cmd := range cmds { err = a.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cc[i]) @@ -173,6 +179,11 @@ func (a *APIController) apiApply(request *APIRequest, ctx *command.Context) (*co return nil, err } + // Update the combined apply commit status to pending + if err := a.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { + ctx.Log.Warn("unable to update apply commit status: %s", err) + } + var projectResults []command.ProjectResult for i, cmd := range cmds { err = a.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cc[i]) diff --git a/server/controllers/api_controller_test.go b/server/controllers/api_controller_test.go index 3b3aa520aa..778c41bee2 100644 --- a/server/controllers/api_controller_test.go +++ b/server/controllers/api_controller_test.go @@ -94,6 +94,10 @@ func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, When(postWorkflowHooksCommandRunner.RunPostHooks(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(nil) + commitStatusUpdater := NewMockCommitStatusUpdater() + + When(commitStatusUpdater.UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]())).ThenReturn(nil) + ac := controllers.APIController{ APISecret: []byte(atlantisToken), Locker: locker, @@ -107,6 +111,7 @@ func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, VCSClient: vcsClient, RepoAllowlistChecker: repoAllowlistChecker, + CommitStatusUpdater: commitStatusUpdater, } return ac, projectCommandBuilder, projectCommandRunner } diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 4588b04127..d06b237b9f 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -1647,6 +1647,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, PullStatusFetcher: backend, DisableAutoplan: opt.disableAutoplan, + CommitStatusUpdater: commitStatusUpdater, } repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 6c69032910..c68110e518 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -94,10 +94,6 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { return } - if err = a.commitStatusUpdater.UpdateCombined(ctx.Log, baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - // Get the mergeable status before we set any build statuses of our own. // We do this here because when we set a "Pending" status, if users have // required the Atlantis status checks to pass, then we've now changed diff --git a/server/events/command_runner.go b/server/events/command_runner.go index daa66356d2..30a82105a9 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -202,6 +202,12 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo cmd := &CommentCommand{ Name: command.Autoplan, } + + // Update the combined plan commit status to pending + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("unable to update plan commit status: %s", err) + } + err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if err != nil { @@ -354,6 +360,18 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } + // Update the combined plan or apply commit status to pending + switch cmd.Name { + case command.Plan: + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("unable to update plan commit status: %s", err) + } + case command.Apply: + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { + ctx.Log.Warn("unable to update apply commit status: %s", err) + } + } + err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if err != nil { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index b3b88c40d0..4e32994824 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -253,6 +253,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, PullStatusFetcher: testConfig.backend, + CommitStatusUpdater: commitUpdater, } return vcsClient @@ -440,15 +441,8 @@ func TestRunCommentCommandApply_NoProjects_SilenceEnabled(t *testing.T) { ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Apply}) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) - commitUpdater.VerifyWasCalledOnce().UpdateCombinedCount( - Any[logging.SimpleLogging](), - Any[models.Repo](), - Any[models.PullRequest](), - Eq[models.CommitStatus](models.SuccessCommitStatus), - Eq[command.Name](command.Apply), - Eq(0), - Eq(0), - ) + commitUpdater.VerifyWasCalledOnce().UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq(models.PendingCommitStatus), Eq(command.Apply)) } func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) { @@ -463,15 +457,6 @@ func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.ApprovePolicies}) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) - commitUpdater.VerifyWasCalledOnce().UpdateCombinedCount( - Any[logging.SimpleLogging](), - Any[models.Repo](), - Any[models.PullRequest](), - Eq[models.CommitStatus](models.SuccessCommitStatus), - Eq[command.Name](command.PolicyCheck), - Eq(0), - Eq(0), - ) } func TestRunCommentCommandUnlock_NoProjects_SilenceEnabled(t *testing.T) { @@ -485,6 +470,8 @@ func TestRunCommentCommandUnlock_NoProjects_SilenceEnabled(t *testing.T) { ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) vcsClient.VerifyWasCalled(Never()).CreateComment(Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq(models.PendingCommitStatus), Any[command.Name]()) } func TestRunCommentCommandImport_NoProjects_SilenceEnabled(t *testing.T) { @@ -535,7 +522,7 @@ func TestRunCommentCommand_DisableAutoplan(t *testing.T) { CommandName: command.Plan, }, }, nil) - + When(commitUpdater.UpdateCombinedCount(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[int](), Any[int]())).ThenReturn(nil) ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, modelPull, testdata.User) projectCommandBuilder.VerifyWasCalled(Never()).BuildAutoplanCommands(Any[*command.Context]()) } @@ -831,6 +818,10 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Fal ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User) pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp) lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num) + commitUpdater.VerifyWasCalledOnce().UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Eq(models.PendingCommitStatus), Eq(command.Plan)) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Eq(models.FailedCommitStatus), Any[command.Name]()) } func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_True(t *testing.T) { @@ -853,6 +844,8 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Tru ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User) pendingPlanFinder.VerifyWasCalled(Never()).DeletePlans(Any[string]()) lockingLocker.VerifyWasCalled(Never()).UnlockByPull(Any[string](), Any[int]()) + commitUpdater.VerifyWasCalledOnce().UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Eq(models.PendingCommitStatus), Eq(command.Plan)) } func TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_False(t *testing.T) { diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index c1cc3e81e0..a9652b0cab 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -114,11 +114,6 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { return } - // At this point we are sure Atlantis has work to do, so set commit status to pending - if err := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - // discard previous plans that might not be relevant anymore ctx.Log.Debug("deleting previous plans and locks") p.deletePlans(ctx) @@ -188,10 +183,6 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { } } - if err = p.commitStatusUpdater.UpdateCombined(ctx.Log, baseRepo, pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - projectCmds, err := p.prjCmdBuilder.BuildPlanCommands(ctx, cmd) if err != nil { if statusErr := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); statusErr != nil { diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go index 7d152c7328..be175d0b7d 100644 --- a/server/events/pre_workflow_hooks_command_runner.go +++ b/server/events/pre_workflow_hooks_command_runner.go @@ -69,18 +69,6 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, escapedArgs = escapeArgs(cmd.Flags) } - // Update the plan or apply commit status to pending whilst the pre workflow hook is running - switch cmd.Name { - case command.Plan: - if err := w.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - case command.Apply: - if err := w.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { - ctx.Log.Warn("unable to update apply commit status: %s", err) - } - } - err = w.runHooks( models.WorkflowHookCommandContext{ BaseRepo: ctx.Pull.BaseRepo,