Skip to content

Commit

Permalink
fix: When GitHub/GitLab Auto-Merge Is Used with Atlantis Pre Workflow…
Browse files Browse the repository at this point in the history
… Hooks, the PR will be Merged Prematurely (#3880)

* Fix Premature Auto-Merge

* Add CommitStatusUpdater to commandRunner
  • Loading branch information
X-Guardian authored Nov 4, 2023
1 parent 3056701 commit 1b45fb1
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
29 changes: 28 additions & 1 deletion server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package events

import (
"fmt"
"github.com/runatlantis/atlantis/server/utils"
"strconv"

"github.com/google/go-github/v54/github"
Expand All @@ -28,6 +27,7 @@ import (
"github.com/runatlantis/atlantis/server/logging"
"github.com/runatlantis/atlantis/server/metrics"
"github.com/runatlantis/atlantis/server/recovery"
"github.com/runatlantis/atlantis/server/utils"
tally "github.com/uber-go/tally/v4"
gitlab "github.com/xanzy/go-gitlab"
)
Expand Down Expand Up @@ -128,6 +128,7 @@ type DefaultCommandRunner struct {
PullStatusFetcher PullStatusFetcher
TeamAllowlistChecker *TeamAllowlistChecker
VarFileAllowlistChecker *VarFileAllowlistChecker
CommitStatusUpdater CommitStatusUpdater
}

// RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated.
Expand Down Expand Up @@ -186,6 +187,19 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo

if c.FailOnPreWorkflowHookError {
ctx.Log.Err("'fail-on-pre-workflow-hook-error' set, so not running %s command.", command.Plan)

// Update the plan or apply commit status to pending whilst the pre workflow hook is running so that the PR can't be merged.
switch cmd.Name {
case command.Plan:
if err := c.CommitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); err != nil {
ctx.Log.Warn("unable to update plan commit status: %s", err)
}
case command.Apply:
if err := c.CommitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Apply); err != nil {
ctx.Log.Warn("unable to update apply commit status: %s", err)
}
}

return
}

Expand Down Expand Up @@ -317,6 +331,19 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead

if c.FailOnPreWorkflowHookError {
ctx.Log.Err("'fail-on-pre-workflow-hook-error' set, so not running %s command.", cmd.Name.String())

// Update the plan or apply commit status to pending whilst the pre workflow hook is running so that the PR can't be merged.
switch cmd.Name {
case command.Plan:
if err := c.CommitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); err != nil {
ctx.Log.Warn("unable to update plan commit status: %s", err)
}
case command.Apply:
if err := c.CommitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Apply); err != nil {
ctx.Log.Warn("unable to update apply commit status: %s", err)
}
}

return
}

Expand Down
18 changes: 15 additions & 3 deletions server/events/pre_workflow_hooks_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ 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.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.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: baseRepo,
Expand Down Expand Up @@ -132,21 +144,21 @@ func (w *DefaultPreWorkflowHooksCommandRunner) runHooks(
}

if err := w.CommitStatusUpdater.UpdatePreWorkflowHook(ctx.Pull, models.PendingCommitStatus, hookDescription, "", url); err != nil {
ctx.Log.Warn("unable to pre workflow hook status: %s", err)
ctx.Log.Warn("unable to update pre workflow hook status: %s", err)
return err
}

_, runtimeDesc, err := w.PreWorkflowHookRunner.Run(ctx, hook.RunCommand, shell, shellArgs, repoDir)

if err != nil {
if err := w.CommitStatusUpdater.UpdatePreWorkflowHook(ctx.Pull, models.FailedCommitStatus, hookDescription, runtimeDesc, url); err != nil {
ctx.Log.Warn("unable to pre workflow hook status: %s", err)
ctx.Log.Warn("unable to update pre workflow hook status: %s", err)
}
return err
}

if err := w.CommitStatusUpdater.UpdatePreWorkflowHook(ctx.Pull, models.SuccessCommitStatus, hookDescription, runtimeDesc, url); err != nil {
ctx.Log.Warn("unable to pre workflow hook status: %s", err)
ctx.Log.Warn("unable to update pre workflow hook status: %s", err)
return err
}
}
Expand Down
1 change: 1 addition & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
PullStatusFetcher: backend,
TeamAllowlistChecker: githubTeamAllowlistChecker,
VarFileAllowlistChecker: varFileAllowlistChecker,
CommitStatusUpdater: commitStatusUpdater,
}
repoAllowlist, err := events.NewRepoAllowlistChecker(userConfig.RepoAllowlist)
if err != nil {
Expand Down

0 comments on commit 1b45fb1

Please sign in to comment.