feat: set atlantis/apply check to successful if all plans are `No…

… Changes` (#3378) * mod: rename updateCommitStatus func * feat: add PlannedNoChangesPlanStatus * Add skipApplyNoChanges option to PlanCommandRunner * Add skipApplyNoChanges option to ApplyCommandRunner * Add --skip-apply-no-changes flag * Fix typo Co-authored-by: nitrocode <[email protected]> * Rename --skip-apply-no-changes flag * Refactor updateCommitStatus functions * chore(docs): add detailed use case for the flag * test: add plan_command_runner set apply status * feat: set apply status to successful by default when result is 'No Changes' --------- Co-authored-by: chroju <[email protected]> Co-authored-by: nitrocode <[email protected]>
runatlantis · Aug 3, 2023 · fbb12fe · fbb12fe
1 parent d6ba8f5
commit fbb12fe
Show file tree

Hide file tree

Showing 7 changed files with 412 additions and 15 deletions.
diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go
@@ -199,7 +199,7 @@ func (a *ApplyCommandRunner) updateCommitStatus(ctx *command.Context, pullStatus
 	var numErrored int
 	status := models.SuccessCommitStatus
 
-	numSuccess = pullStatus.StatusCount(models.AppliedPlanStatus)
+	numSuccess = pullStatus.StatusCount(models.AppliedPlanStatus) + pullStatus.StatusCount(models.PlannedNoChangesPlanStatus)
 	numErrored = pullStatus.StatusCount(models.ErroredApplyStatus)
 
 	if numErrored > 0 {

diff --git a/server/events/command/project_result.go b/server/events/command/project_result.go
@@ -57,6 +57,8 @@ func (p ProjectResult) PlanStatus() models.ProjectPlanStatus {
 			return models.ErroredPlanStatus
 		} else if p.Failure != "" {
 			return models.ErroredPlanStatus
+		} else if p.PlanSuccess.NoChanges() {
+			return models.PlannedNoChangesPlanStatus
 		}
 		return models.PlannedPlanStatus
 	case PolicyCheck, ApprovePolicies:

diff --git a/server/events/command/project_result_test.go b/server/events/command/project_result_test.go
@@ -79,6 +79,15 @@ func TestProjectResult_PlanStatus(t *testing.T) {
 			},
 			expStatus: models.PlannedPlanStatus,
 		},
+		{
+			p: command.ProjectResult{
+				Command: command.Plan,
+				PlanSuccess: &models.PlanSuccess{
+					TerraformOutput: "No changes. Infrastructure is up-to-date.",
+				},
+			},
+			expStatus: models.PlannedNoChangesPlanStatus,
+		},
 		{
 			p: command.ProjectResult{
 				Command: command.Apply,

diff --git a/server/events/command_runner_internal_test.go b/server/events/command_runner_internal_test.go
@@ -67,6 +67,22 @@ func TestApplyUpdateCommitStatus(t *testing.T) {
 			expNumSuccess: 1,
 			expNumTotal:   3,
 		},
+		"apply, one planned no changes": {
+			cmd: command.Apply,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.AppliedPlanStatus,
+					},
+					{
+						Status: models.PlannedNoChangesPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.SuccessCommitStatus,
+			expNumSuccess: 2,
+			expNumTotal:   2,
+		},
 	}
 
 	for name, c := range cases {
@@ -86,7 +102,7 @@ func TestApplyUpdateCommitStatus(t *testing.T) {
 	}
 }
 
-func TestPlanUpdateCommitStatus(t *testing.T) {
+func TestPlanUpdatePlanCommitStatus(t *testing.T) {
 	cases := map[string]struct {
 		cmd           command.Name
 		pullStatus    models.PullStatus
@@ -137,7 +153,104 @@ func TestPlanUpdateCommitStatus(t *testing.T) {
 			cr := &PlanCommandRunner{
 				commitStatusUpdater: csu,
 			}
-			cr.updateCommitStatus(&command.Context{}, c.pullStatus)
+			cr.updateCommitStatus(&command.Context{}, c.pullStatus, command.Plan)
+			Equals(t, models.Repo{}, csu.CalledRepo)
+			Equals(t, models.PullRequest{}, csu.CalledPull)
+			Equals(t, c.expStatus, csu.CalledStatus)
+			Equals(t, c.cmd, csu.CalledCommand)
+			Equals(t, c.expNumSuccess, csu.CalledNumSuccess)
+			Equals(t, c.expNumTotal, csu.CalledNumTotal)
+		})
+	}
+}
+
+func TestPlanUpdateApplyCommitStatus(t *testing.T) {
+	cases := map[string]struct {
+		cmd           command.Name
+		pullStatus    models.PullStatus
+		expStatus     models.CommitStatus
+		expNumSuccess int
+		expNumTotal   int
+	}{
+		"all plans success with no changes": {
+			cmd: command.Apply,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.PlannedNoChangesPlanStatus,
+					},
+					{
+						Status: models.PlannedNoChangesPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.SuccessCommitStatus,
+			expNumSuccess: 2,
+			expNumTotal:   2,
+		},
+		"one plan, one plan success with no changes": {
+			cmd: command.Apply,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.PlannedNoChangesPlanStatus,
+					},
+					{
+						Status: models.PlannedPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.PendingCommitStatus,
+			expNumSuccess: 1,
+			expNumTotal:   2,
+		},
+		"one plan, one apply, one plan success with no changes": {
+			cmd: command.Apply,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.PlannedNoChangesPlanStatus,
+					},
+					{
+						Status: models.AppliedPlanStatus,
+					},
+					{
+						Status: models.PlannedPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.PendingCommitStatus,
+			expNumSuccess: 2,
+			expNumTotal:   3,
+		},
+		"one apply error, one apply, one plan success with no changes": {
+			cmd: command.Apply,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.PlannedNoChangesPlanStatus,
+					},
+					{
+						Status: models.AppliedPlanStatus,
+					},
+					{
+						Status: models.ErroredApplyStatus,
+					},
+				},
+			},
+			expStatus:     models.FailedCommitStatus,
+			expNumSuccess: 2,
+			expNumTotal:   3,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			csu := &MockCSU{}
+			cr := &PlanCommandRunner{
+				commitStatusUpdater: csu,
+			}
+			cr.updateCommitStatus(&command.Context{}, c.pullStatus, command.Apply)
 			Equals(t, models.Repo{}, csu.CalledRepo)
 			Equals(t, models.PullRequest{}, csu.CalledPull)
 			Equals(t, c.expStatus, csu.CalledStatus)

diff --git a/server/events/models/models.go b/server/events/models/models.go
@@ -562,6 +562,9 @@ const (
 	// PlannedPlanStatus means that a plan has been successfully generated but
 	// not yet applied.
 	PlannedPlanStatus
+	// PlannedNoChangesPlanStatus means that a plan has been successfully
+	// generated with "No changes" and not yet applied.
+	PlannedNoChangesPlanStatus
 	// ErroredApplyStatus means that a plan has been generated but there was an
 	// error while applying it.
 	ErroredApplyStatus
@@ -586,6 +589,8 @@ func (p ProjectPlanStatus) String() string {
 		return "plan_errored"
 	case PlannedPlanStatus:
 		return "planned"
+	case PlannedNoChangesPlanStatus:
+		return "planned_no_changes"
 	case ErroredApplyStatus:
 		return "apply_errored"
 	case AppliedPlanStatus:

diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go
@@ -148,7 +148,8 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
 		ctx.Log.Err("writing results: %s", err)
 	}
 
-	p.updateCommitStatus(ctx, pullStatus)
+	p.updateCommitStatus(ctx, pullStatus, command.Plan)
+	p.updateCommitStatus(ctx, pullStatus, command.Apply)
 
 	// Check if there are any planned projects and if there are any errors or if plans are being deleted
 	if len(policyCheckCmds) > 0 &&
@@ -218,7 +219,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {
 					return
 				}
 				ctx.Log.Debug("resetting VCS status")
-				p.updateCommitStatus(ctx, *pullStatus)
+				p.updateCommitStatus(ctx, *pullStatus, command.Plan)
 			} else {
 				// With a generic plan, we set successful commit statuses
 				// with 0/0 projects planned successfully because some users require
@@ -272,7 +273,8 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {
 		return
 	}
 
-	p.updateCommitStatus(ctx, pullStatus)
+	p.updateCommitStatus(ctx, pullStatus, command.Plan)
+	p.updateCommitStatus(ctx, pullStatus, command.Apply)
 
 	// Runs policy checks step after all plans are successful.
 	// This step does not approve any policies that require approval.
@@ -291,26 +293,39 @@ func (p *PlanCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
 	}
 }
 
-func (p *PlanCommandRunner) updateCommitStatus(ctx *command.Context, pullStatus models.PullStatus) {
+func (p *PlanCommandRunner) updateCommitStatus(ctx *command.Context, pullStatus models.PullStatus, commandName command.Name) {
 	var numSuccess int
 	var numErrored int
 	status := models.SuccessCommitStatus
 
-	numErrored = pullStatus.StatusCount(models.ErroredPlanStatus)
-	// We consider anything that isn't a plan error as a plan success.
-	// For example, if there is an apply error, that means that at least a
-	// plan was generated successfully.
-	numSuccess = len(pullStatus.Projects) - numErrored
+	if commandName == command.Plan {
+		numErrored = pullStatus.StatusCount(models.ErroredPlanStatus)
+		// We consider anything that isn't a plan error as a plan success.
+		// For example, if there is an apply error, that means that at least a
+		// plan was generated successfully.
+		numSuccess = len(pullStatus.Projects) - numErrored
 
-	if numErrored > 0 {
-		status = models.FailedCommitStatus
+		if numErrored > 0 {
+			status = models.FailedCommitStatus
+		}
+	} else if commandName == command.Apply {
+		numSuccess = pullStatus.StatusCount(models.AppliedPlanStatus) + pullStatus.StatusCount(models.PlannedNoChangesPlanStatus)
+		numErrored = pullStatus.StatusCount(models.ErroredApplyStatus)
+
+		if numErrored > 0 {
+			status = models.FailedCommitStatus
+		} else if numSuccess < len(pullStatus.Projects) {
+			// If there are plans that haven't been applied yet, we'll use a pending
+			// status.
+			status = models.PendingCommitStatus
+		}
 	}
 
 	if err := p.commitStatusUpdater.UpdateCombinedCount(
 		ctx.Pull.BaseRepo,
 		ctx.Pull,
 		status,
-		command.Plan,
+		commandName,
 		numSuccess,
 		len(pullStatus.Projects),
 	); err != nil {