Cherry picked refactor apply requirements from upstream (#93)

#1729

server/controllers/events/events_controller_e2e_test.go

@@ -789,10 +789,13 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 			TerraformExecutor: terraformClient,
 			DefaultTFVersion:  defaultTFVersion,
 		},
-		PullApprovedChecker: e2eVCSClient,
-		WorkingDir:          workingDir,
-		Webhooks:            &mockWebhookSender{},
-		WorkingDirLocker:    locker,
+		WorkingDir:       workingDir,
+		Webhooks:         &mockWebhookSender{},
+		WorkingDirLocker: locker,
+		AggregateApplyRequirements: &events.AggregateApplyRequirements{
+			PullApprovedChecker: e2eVCSClient,
+			WorkingDir:          workingDir,
+		},
 		TerraformOutputChan: tempchan,
 	}
 

server/events/apply_requirement_handler.go

@@ -0,0 +1,50 @@
+package events
+
+import (
+	"github.com/pkg/errors"
+	"github.com/runatlantis/atlantis/server/events/models"
+	"github.com/runatlantis/atlantis/server/events/runtime"
+	"github.com/runatlantis/atlantis/server/events/yaml/raw"
+	"github.com/runatlantis/atlantis/server/events/yaml/valid"
+)
+
+//go:generate pegomock generate -m --package mocks -o mocks/mock_apply_handler.go ApplyRequirement
+type ApplyRequirement interface {
+	ValidateProject(repoDir string, ctx models.ProjectCommandContext) (string, error)
+}
+
+type AggregateApplyRequirements struct {
+	PullApprovedChecker runtime.PullApprovedChecker
+	WorkingDir          WorkingDir
+}
+
+func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx models.ProjectCommandContext) (failure string, err error) {
+
+	for _, req := range ctx.ApplyRequirements {
+		switch req {
+		case raw.ApprovedApplyRequirement:
+			approved, err := a.PullApprovedChecker.PullIsApproved(ctx.Pull.BaseRepo, ctx.Pull) // nolint: vetshadow
+			if err != nil {
+				return "", errors.Wrap(err, "checking if pull request was approved")
+			}
+			if !approved {
+				return "Pull request must be approved by at least one person other than the author before running apply.", nil
+			}
+		// this should come before mergeability check since mergeability is a superset of this check.
+		case valid.PoliciesPassedApplyReq:
+			if ctx.ProjectPlanStatus == models.ErroredPolicyCheckStatus {
+				return "All policies must pass for project before running apply", nil
+			}
+		case raw.MergeableApplyRequirement:
+			if !ctx.PullMergeable {
+				return "Pull request must be mergeable before running apply.", nil
+			}
+		case raw.UnDivergedApplyRequirement:
+			if a.WorkingDir.HasDiverged(ctx.Log, repoDir) {
+				return "Default branch must be rebased onto pull request before running apply.", nil
+			}
+		}
+	}
+	// Passed all apply requirements configured.
+	return "", nil
+}

server/events/project_command_runner.go

@@ -21,9 +21,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/events/models"
-	"github.com/runatlantis/atlantis/server/events/runtime"
 	"github.com/runatlantis/atlantis/server/events/webhooks"
-	"github.com/runatlantis/atlantis/server/events/yaml/raw"
 	"github.com/runatlantis/atlantis/server/events/yaml/valid"
 	"github.com/runatlantis/atlantis/server/logging"
 )
@@ -110,22 +108,23 @@ type ProjectCommandRunner interface {
 }
 
 // DefaultProjectCommandRunner implements ProjectCommandRunner.
-type DefaultProjectCommandRunner struct { //create object and test
-	Locker                ProjectLocker
-	LockURLGenerator      LockURLGenerator
-	InitStepRunner        StepRunner
-	PlanStepRunner        StepRunner
-	ShowStepRunner        StepRunner
-	ApplyStepRunner       StepRunner
-	PolicyCheckStepRunner StepRunner
-	RunStepRunner         CustomStepRunner
-	EnvStepRunner         EnvStepRunner
-	PullApprovedChecker   runtime.PullApprovedChecker
-	WorkingDir            WorkingDir
-	Webhooks              WebhooksSender
-	WorkingDirLocker      WorkingDirLocker
-	TerraformOutputChan   chan<- *models.TerraformOutputLine
-	LogStreamURLGenerator LogStreamURLGenerator
+type DefaultProjectCommandRunner struct {
+	Locker                     ProjectLocker
+	LockURLGenerator           LockURLGenerator
+	InitStepRunner             StepRunner
+	PlanStepRunner             StepRunner
+	ShowStepRunner             StepRunner
+	ApplyStepRunner            StepRunner
+	PolicyCheckStepRunner      StepRunner
+	VersionStepRunner          StepRunner
+	RunStepRunner              CustomStepRunner
+	EnvStepRunner              EnvStepRunner
+	WorkingDir                 WorkingDir
+	Webhooks                   WebhooksSender
+	WorkingDirLocker           WorkingDirLocker
+	AggregateApplyRequirements ApplyRequirement
+	TerraformOutputChan        chan<- *models.TerraformOutputLine
+	LogStreamURLGenerator      LogStreamURLGenerator
 }
 
 // Plan runs terraform plan for the project described by ctx.
@@ -326,31 +325,11 @@ func (p *DefaultProjectCommandRunner) doApply(ctx models.ProjectCommandContext)
 		return "", "", DirNotExistErr{RepoRelDir: ctx.RepoRelDir}
 	}
 
-	for _, req := range ctx.ApplyRequirements {
-		switch req {
-		case raw.ApprovedApplyRequirement:
-			approved, err := p.PullApprovedChecker.PullIsApproved(ctx.Pull.BaseRepo, ctx.Pull) // nolint: vetshadow
-			if err != nil {
-				return "", "", errors.Wrap(err, "checking if pull request was approved")
-			}
-			if !approved {
-				return "", "Pull request must be approved by at least one person other than the author before running apply.", nil
-			}
-		// this should come before mergeability check since mergeability is a superset of this check.
-		case valid.PoliciesPassedApplyReq:
-			if ctx.ProjectPlanStatus == models.ErroredPolicyCheckStatus {
-				return "", "All policies must pass for project before running apply", nil
-			}
-		case raw.MergeableApplyRequirement:
-			if !ctx.PullMergeable {
-				return "", "Pull request must be mergeable before running apply.", nil
-			}
-		case raw.UnDivergedApplyRequirement:
-			if p.WorkingDir.HasDiverged(ctx.Log, repoDir) {
-				return "", "Default branch must be rebased onto pull request before running apply.", nil
-			}
-		}
+	failure, err = p.AggregateApplyRequirements.ValidateProject(repoDir, ctx)
+	if failure != "" || err != nil {
+		return "", failure, err
 	}
+
 	// Acquire internal lock for the directory we're going to operate in.
 	unlockFn, err := p.WorkingDirLocker.TryLock(ctx.Pull.BaseRepo.FullName, ctx.Pull.Num, ctx.Workspace)
 	if err != nil {

server/events/project_command_runner_test.go

@@ -42,20 +42,21 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) {
 	mockWorkingDir := mocks.NewMockWorkingDir()
 	mockLocker := mocks.NewMockProjectLocker()
 	mockChannel := make(chan *models.TerraformOutputLine)
+	mockApplyReqHandler := mocks.NewMockApplyRequirement()
 
 	runner := events.DefaultProjectCommandRunner{
-		Locker:              mockLocker,
-		LockURLGenerator:    mockURLGenerator{},
-		InitStepRunner:      mockInit,
-		PlanStepRunner:      mockPlan,
-		ApplyStepRunner:     mockApply,
-		RunStepRunner:       mockRun,
-		EnvStepRunner:       &realEnv,
-		PullApprovedChecker: nil,
-		WorkingDir:          mockWorkingDir,
-		Webhooks:            nil,
-		WorkingDirLocker:    events.NewDefaultWorkingDirLocker(),
-		TerraformOutputChan: mockChannel,
+		Locker:                     mockLocker,
+		LockURLGenerator:           mockURLGenerator{},
+		InitStepRunner:             mockInit,
+		PlanStepRunner:             mockPlan,
+		ApplyStepRunner:            mockApply,
+		RunStepRunner:              mockRun,
+		EnvStepRunner:              &realEnv,
+		WorkingDir:                 mockWorkingDir,
+		Webhooks:                   nil,
+		WorkingDirLocker:           events.NewDefaultWorkingDirLocker(),
+		AggregateApplyRequirements: mockApplyReqHandler,
+		TerraformOutputChan:        mockChannel,
 	}
 
 	repoDir, cleanup := TempDir(t)
@@ -150,9 +151,12 @@ func TestDefaultProjectCommandRunner_ApplyNotApproved(t *testing.T) {
 	mockWorkingDir := mocks.NewMockWorkingDir()
 	mockApproved := mocks2.NewMockPullApprovedChecker()
 	runner := &events.DefaultProjectCommandRunner{
-		WorkingDir:          mockWorkingDir,
-		PullApprovedChecker: mockApproved,
-		WorkingDirLocker:    events.NewDefaultWorkingDirLocker(),
+		WorkingDir:       mockWorkingDir,
+		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+		AggregateApplyRequirements: &events.AggregateApplyRequirements{
+			PullApprovedChecker: mockApproved,
+			WorkingDir:          mockWorkingDir,
+		},
 	}
 	ctx := models.ProjectCommandContext{
 		ApplyRequirements: []string{"approved"},
@@ -173,6 +177,9 @@ func TestDefaultProjectCommandRunner_ApplyNotMergeable(t *testing.T) {
 	runner := &events.DefaultProjectCommandRunner{
 		WorkingDir:       mockWorkingDir,
 		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+		AggregateApplyRequirements: &events.AggregateApplyRequirements{
+			WorkingDir: mockWorkingDir,
+		},
 	}
 	ctx := models.ProjectCommandContext{
 		PullMergeable:     false,
@@ -193,6 +200,9 @@ func TestDefaultProjectCommandRunner_ApplyDiverged(t *testing.T) {
 	runner := &events.DefaultProjectCommandRunner{
 		WorkingDir:       mockWorkingDir,
 		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+		AggregateApplyRequirements: &events.AggregateApplyRequirements{
+			WorkingDir: mockWorkingDir,
+		},
 	}
 	ctx := models.ProjectCommandContext{
 		ApplyRequirements: []string{"undiverged"},
@@ -296,19 +306,23 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) {
 			mockWorkingDir := mocks.NewMockWorkingDir()
 			mockLocker := mocks.NewMockProjectLocker()
 			mockSender := mocks.NewMockWebhooksSender()
-
-			runner := events.DefaultProjectCommandRunner{
-				Locker:              mockLocker,
-				LockURLGenerator:    mockURLGenerator{},
-				InitStepRunner:      mockInit,
-				PlanStepRunner:      mockPlan,
-				ApplyStepRunner:     mockApply,
-				RunStepRunner:       mockRun,
-				EnvStepRunner:       mockEnv,
+			applyReqHandler := &events.AggregateApplyRequirements{
 				PullApprovedChecker: mockApproved,
 				WorkingDir:          mockWorkingDir,
-				Webhooks:            mockSender,
-				WorkingDirLocker:    events.NewDefaultWorkingDirLocker(),
+			}
+
+			runner := events.DefaultProjectCommandRunner{
+				Locker:                     mockLocker,
+				LockURLGenerator:           mockURLGenerator{},
+				InitStepRunner:             mockInit,
+				PlanStepRunner:             mockPlan,
+				ApplyStepRunner:            mockApply,
+				RunStepRunner:              mockRun,
+				EnvStepRunner:              mockEnv,
+				WorkingDir:                 mockWorkingDir,
+				Webhooks:                   mockSender,
+				WorkingDirLocker:           events.NewDefaultWorkingDirLocker(),
+				AggregateApplyRequirements: applyReqHandler,
 			}
 			repoDir, cleanup := TempDir(t)
 			defer cleanup()
@@ -378,14 +392,13 @@ func TestDefaultProjectCommandRunner_RunEnvSteps(t *testing.T) {
 	mockLocker := mocks.NewMockProjectLocker()
 
 	runner := events.DefaultProjectCommandRunner{
-		Locker:              mockLocker,
-		LockURLGenerator:    mockURLGenerator{},
-		RunStepRunner:       &run,
-		EnvStepRunner:       &env,
-		PullApprovedChecker: nil,
-		WorkingDir:          mockWorkingDir,
-		Webhooks:            nil,
-		WorkingDirLocker:    events.NewDefaultWorkingDirLocker(),
+		Locker:           mockLocker,
+		LockURLGenerator: mockURLGenerator{},
+		RunStepRunner:    &run,
+		EnvStepRunner:    &env,
+		WorkingDir:       mockWorkingDir,
+		Webhooks:         nil,
+		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
 	}
 
 	repoDir, cleanup := TempDir(t)