Skip to content

Commit

Permalink
Make policy checks its own apply requirement. (#61) (#1499)
Browse files Browse the repository at this point in the history
* Make policy checks its own apply requirement. (#61)

* Remove warning from docs.
  • Loading branch information
nishkrishnan authored Apr 13, 2021
1 parent 7e052d3 commit df106de
Show file tree
Hide file tree
Showing 47 changed files with 857 additions and 109 deletions.
4 changes: 0 additions & 4 deletions runatlantis.io/docs/policy-checking.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 0 additions & 17 deletions server/events/apply_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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." +
Expand Down
2 changes: 2 additions & 0 deletions server/events/command_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,7 @@ type CommandContext struct {
// required the Atlantis status to be successful prior to merging.
PullMergeable bool

PullStatus *models.PullStatus

Trigger CommandTrigger
}
37 changes: 26 additions & 11 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
policyCheckCommandRunner,
autoMerger,
parallelPoolSize,
defaultBoltDB,
)

applyCommandRunner = events.NewApplyCommandRunner(
Expand Down Expand Up @@ -175,6 +176,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
AllowForkPRsFlag: "allow-fork-prs-flag",
Drainer: drainer,
PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner,
PullStatusFetcher: defaultBoltDB,
}
return vcsClient
}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions server/events/comment_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 14 additions & 1 deletion server/events/mocks/matchers/events_commentparseresult.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion server/events/mocks/matchers/models_vcshosttype.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions server/events/mocks/matchers/slice_of_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit df106de

Please sign in to comment.