Make policy checks its own apply requirement. (#61)

runatlantis.io/docs/policy-checking.md

@@ -17,10 +17,6 @@ Enabling "policy checking" in addition to the [mergeable apply requirement](http
 
 ![Policy Check Apply Status Failure](./images/policy-check-apply-status-failure.png)
 
-:::warning
-Without the mergeable requirement applies will still go through in the event of a policy failure.
-:::
-
 Any failures need to either be addressed in a successive commit, or approved by a blessed owner. This approval is independent of the approval apply requirement which can coexist in the policy checking workflow. After an approval, the apply can proceed.
 
 ![Policy Check Approval](./images/policy-check-approval.png)

server/events/apply_command_runner.go

@@ -93,13 +93,6 @@ func (a *ApplyCommandRunner) Run(ctx *CommandContext, cmd *CommentCommand) {
 		ctx.Log.Warn("unable to get mergeable status: %s. Continuing with mergeable assumed false", err)
 	}
 
-	// TODO: This needs to be revisited and new PullMergeable like conditions should
-	// be added to check against it.
-	if a.anyFailedPolicyChecks(pull) {
-		ctx.PullMergeable = false
-		ctx.Log.Warn("when using policy checks all policies have to be approved or pass. Continuing with mergeable assumed false")
-	}
-
 	ctx.Log.Info("pull request mergeable status: %t", ctx.PullMergeable)
 
 	if err = a.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil {
@@ -182,16 +175,6 @@ func (a *ApplyCommandRunner) updateCommitStatus(ctx *CommandContext, pullStatus
 	}
 }
 
-func (a *ApplyCommandRunner) anyFailedPolicyChecks(pull models.PullRequest) bool {
-	policyCheckPullStatus, _ := a.DB.GetPullStatus(pull)
-	if policyCheckPullStatus != nil && policyCheckPullStatus.StatusCount(models.ErroredPolicyCheckStatus) > 0 {
-		return true
-	}
-
-	return false
-
-}
-
 // applyAllDisabledComment is posted when apply all commands (i.e. "atlantis apply")
 // are disabled and an apply all command is issued.
 var applyAllDisabledComment = "**Error:** Running `atlantis apply` without flags is disabled." +

server/events/command_context.go

@@ -46,5 +46,7 @@ type CommandContext struct {
 	// required the Atlantis status to be successful prior to merging.
 	PullMergeable bool
 
+	PullStatus *models.PullStatus
+
 	Trigger CommandTrigger
 }

server/events/command_runner.go

@@ -113,6 +113,7 @@ type DefaultCommandRunner struct {
 	CommentCommandRunnerByCmd     map[models.CommandName]CommentCommandRunner
 	Drainer                       *Drainer
 	PreWorkflowHooksCommandRunner PreWorkflowHooksCommandRunner
+	PullStatusFetcher             PullStatusFetcher
 }
 
 // RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated.
@@ -127,12 +128,19 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
 
 	log := c.buildLogger(baseRepo.FullName, pull.Num)
 	defer c.logPanics(baseRepo, pull.Num, log)
+	status, err := c.PullStatusFetcher.GetPullStatus(pull)
+
+	if err != nil {
+		log.Err("Unable to fetch pull status, this is likely a bug.", err)
+	}
+
 	ctx := &CommandContext{
-		User:     user,
-		Log:      log,
-		Pull:     pull,
-		HeadRepo: headRepo,
-		Trigger:  Auto,
+		User:       user,
+		Log:        log,
+		Pull:       pull,
+		HeadRepo:   headRepo,
+		PullStatus: status,
+		Trigger:    Auto,
 	}
 	if !c.validateCtxAndComment(ctx) {
 		return
@@ -141,7 +149,7 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
 		return
 	}
 
-	err := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx)
+	err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx)
 
 	if err != nil {
 		ctx.Log.Err("Error running pre-workflow hooks %s. Proceeding with %s command.", err, models.PlanCommand)
@@ -174,12 +182,19 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
 		return
 	}
 
+	status, err := c.PullStatusFetcher.GetPullStatus(pull)
+
+	if err != nil {
+		log.Err("Unable to fetch pull status, this is likely a bug.", err)
+	}
+
 	ctx := &CommandContext{
-		User:     user,
-		Log:      log,
-		Pull:     pull,
-		HeadRepo: headRepo,
-		Trigger:  Comment,
+		User:       user,
+		Log:        log,
+		Pull:       pull,
+		PullStatus: status,
+		HeadRepo:   headRepo,
+		Trigger:    Comment,
 	}
 
 	if !c.validateCtxAndComment(ctx) {

server/events/command_runner_test.go

@@ -123,6 +123,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		policyCheckCommandRunner,
 		autoMerger,
 		parallelPoolSize,
+		defaultBoltDB,
 	)
 
 	applyCommandRunner = events.NewApplyCommandRunner(
@@ -175,6 +176,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		AllowForkPRsFlag:              "allow-fork-prs-flag",
 		Drainer:                       drainer,
 		PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner,
+		PullStatusFetcher:             defaultBoltDB,
 	}
 	return vcsClient
 }
@@ -531,16 +533,14 @@ func TestApplyMergeablityWhenPolicyCheckFails(t *testing.T) {
 	When(ch.VCSClient.PullIsMergeable(fixtures.GithubRepo, modelPull)).ThenReturn(true, nil)
 
 	When(projectCommandBuilder.BuildApplyCommands(matchers.AnyPtrToEventsCommandContext(), matchers.AnyPtrToEventsCommentCommand())).Then(func(args []Param) ReturnValues {
-		ctx := args[0].(*events.CommandContext)
-		Equals(t, false, ctx.PullMergeable)
-
 		return ReturnValues{
 			[]models.ProjectCommandContext{
 				{
-					CommandName: models.ApplyCommand,
-					ProjectName: "default",
-					Workspace:   "default",
-					RepoRelDir:  ".",
+					CommandName:       models.ApplyCommand,
+					ProjectName:       "default",
+					Workspace:         "default",
+					RepoRelDir:        ".",
+					ProjectPlanStatus: models.ErroredPolicyCheckStatus,
 				},
 			},
 			nil,

server/events/comment_parser.go

@@ -57,6 +57,8 @@ type CommentParsing interface {
 	Parse(comment string, vcsHost models.VCSHostType) CommentParseResult
 }
 
+//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_comment_building.go CommentBuilder
+
 // CommentBuilder builds comment commands that can be used on pull requests.
 type CommentBuilder interface {
 	// BuildPlanComment builds a plan comment for the specified args.