From 75af38e9d122b88ad5d47880fede4f0f8cdb4ee6 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 30 Nov 2023 13:16:51 +0100 Subject: [PATCH] plancheck: Ensure `ExpectEmptyPlan` and `ExpectNonEmptyPlan` account for output changes in addition to resource changes (#232) Reference: https://github.com/hashicorp/terraform-plugin-testing/issues/222 Previously with new unit tests: ``` --- FAIL: Test_ExpectNonEmptyPlan_OutputChanges_None (0.90s) expect_non_empty_plan_test.go:17: Step 2/2 error: Pre-apply plan check(s) failed: expected a non-empty plan, but got an empty plan --- FAIL: Test_ExpectEmptyPlan_OutputChanges_Error (1.00s) expect_empty_plan_test.go:41: Step 2/2, expected an error but got none ``` --- .../unreleased/BUG FIXES-20231129-154136.yaml | 6 + plancheck/expect_empty_plan.go | 10 +- plancheck/expect_empty_plan_test.go | 108 +++++++++++++----- plancheck/expect_non_empty_plan.go | 8 +- plancheck/expect_non_empty_plan_test.go | 92 ++++++++++++--- 5 files changed, 177 insertions(+), 47 deletions(-) create mode 100644 .changes/unreleased/BUG FIXES-20231129-154136.yaml diff --git a/.changes/unreleased/BUG FIXES-20231129-154136.yaml b/.changes/unreleased/BUG FIXES-20231129-154136.yaml new file mode 100644 index 000000000..e565a8891 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20231129-154136.yaml @@ -0,0 +1,6 @@ +kind: BUG FIXES +body: 'plancheck: Ensured `ExpectEmptyPlan` and `ExpectNonEmptyPlan` account for output + changes' +time: 2023-11-29T15:41:36.391182-05:00 +custom: + Issue: "222" diff --git a/plancheck/expect_empty_plan.go b/plancheck/expect_empty_plan.go index 14b65a248..8df2e281e 100644 --- a/plancheck/expect_empty_plan.go +++ b/plancheck/expect_empty_plan.go @@ -17,6 +17,12 @@ type expectEmptyPlan struct{} func (e expectEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { var result []error + for output, change := range req.Plan.OutputChanges { + if !change.Actions.NoOp() { + result = append(result, fmt.Errorf("expected empty plan, but output %q has planned action(s): %v", output, change.Actions)) + } + } + for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { result = append(result, fmt.Errorf("expected empty plan, but %s has planned action(s): %v", rc.Address, rc.Change.Actions)) @@ -26,8 +32,8 @@ func (e expectEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, re resp.Error = errors.Join(result...) } -// ExpectEmptyPlan returns a plan check that asserts that there are no resource changes in the plan. -// All resource changes found will be aggregated and returned in a plan check error. +// ExpectEmptyPlan returns a plan check that asserts that there are no output or resource changes in the plan. +// All output and resource changes found will be aggregated and returned in a plan check error. func ExpectEmptyPlan() PlanCheck { return expectEmptyPlan{} } diff --git a/plancheck/expect_empty_plan_test.go b/plancheck/expect_empty_plan_test.go index 516b2a106..8a223c148 100644 --- a/plancheck/expect_empty_plan_test.go +++ b/plancheck/expect_empty_plan_test.go @@ -9,74 +9,128 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func Test_ExpectEmptyPlan(t *testing.T) { +func Test_ExpectEmptyPlan_OutputChanges_NoChanges(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version0_14_0), // outputs before 0.14 always show as created + }, + // Avoid our own validation that requires at least one provider config. ExternalProviders: map[string]r.ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, + Steps: []r.TestStep{ + { + Config: `output "test" { value = "original" }`, + }, + { + Config: `output "test" { value = "original" }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, }, }, + }) +} + +func Test_ExpectEmptyPlan_OutputChanges_Error(t *testing.T) { + t.Parallel() + + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version0_14_0), // outputs before 0.14 always show as created + }, + // Avoid our own validation that requires at least one provider config. + ExternalProviders: map[string]r.ExternalProvider{ + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, Steps: []r.TestStep{ { - Config: `resource "random_string" "one" { - length = 16 - }`, + Config: `output "test" { value = "original" }`, }, { - Config: `resource "random_string" "one" { - length = 16 - }`, + Config: `output "test" { value = "new" }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectEmptyPlan(), }, }, + ExpectError: regexp.MustCompile(`output \"test\" has planned action\(s\): \[update\]`), }, }, }) } -func Test_ExpectEmptyPlan_Error(t *testing.T) { +func Test_ExpectEmptyPlan_ResourceChanges_NoChanges(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ + r.UnitTest(t, r.TestCase{ ExternalProviders: map[string]r.ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version1_4_0), + }, + Steps: []r.TestStep{ + { + Config: `resource "terraform_data" "test" {}`, + }, + { + Config: `resource "terraform_data" "test" {}`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, }, }, + }) +} + +func Test_ExpectEmptyPlan_ResourceChanges_Error(t *testing.T) { + t.Parallel() + + r.UnitTest(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version1_4_0), + }, Steps: []r.TestStep{ { - Config: `resource "random_string" "one" { - length = 16 + Config: `resource "terraform_data" "one" { + triggers_replace = ["original"] } - resource "random_string" "two" { - length = 16 + resource "terraform_data" "two" { + triggers_replace = ["original"] } - resource "random_string" "three" { - length = 16 + resource "terraform_data" "three" { + triggers_replace = ["original"] }`, }, { - Config: `resource "random_string" "one" { - length = 12 + Config: `resource "terraform_data" "one" { + triggers_replace = ["new"] } - resource "random_string" "two" { - length = 16 + resource "terraform_data" "two" { + triggers_replace = ["original"] } - resource "random_string" "three" { - length = 12 + resource "terraform_data" "three" { + triggers_replace = ["new"] }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectEmptyPlan(), }, }, - ExpectError: regexp.MustCompile(`.*?(random_string.one has planned action\(s\): \[delete create\])\n.*?(random_string.three has planned action\(s\): \[delete create\])`), + ExpectError: regexp.MustCompile(`.*?(terraform_data.one has planned action\(s\): \[delete create\])\n.*?(terraform_data.three has planned action\(s\): \[delete create\])`), }, }, }) diff --git a/plancheck/expect_non_empty_plan.go b/plancheck/expect_non_empty_plan.go index 74acf034d..482321ece 100644 --- a/plancheck/expect_non_empty_plan.go +++ b/plancheck/expect_non_empty_plan.go @@ -14,6 +14,12 @@ type expectNonEmptyPlan struct{} // CheckPlan implements the plan check logic. func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { + for _, change := range req.Plan.OutputChanges { + if !change.Actions.NoOp() { + return + } + } + for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { return @@ -23,7 +29,7 @@ func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp.Error = errors.New("expected a non-empty plan, but got an empty plan") } -// ExpectNonEmptyPlan returns a plan check that asserts there is at least one resource change in the plan. +// ExpectNonEmptyPlan returns a plan check that asserts there is at least one output or resource change in the plan. func ExpectNonEmptyPlan() PlanCheck { return expectNonEmptyPlan{} } diff --git a/plancheck/expect_non_empty_plan_test.go b/plancheck/expect_non_empty_plan_test.go index 9c6afae4b..ffe919ce3 100644 --- a/plancheck/expect_non_empty_plan_test.go +++ b/plancheck/expect_non_empty_plan_test.go @@ -9,26 +9,83 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func Test_ExpectNonEmptyPlan(t *testing.T) { +func Test_ExpectNonEmptyPlan_OutputChanges(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version0_14_0), // outputs before 0.14 always show as created + }, + // Avoid our own validation that requires at least one provider config. ExternalProviders: map[string]r.ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, + Steps: []r.TestStep{ + { + Config: `output "test" { value = "original" }`, + }, + { + Config: `output "test" { value = "new" }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectNonEmptyPlan(), + }, + }, }, }, + }) +} + +func Test_ExpectNonEmptyPlan_OutputChanges_Error(t *testing.T) { + t.Parallel() + + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version0_14_0), // outputs before 0.14 always show as created + }, + // Avoid our own validation that requires at least one provider config. + ExternalProviders: map[string]r.ExternalProvider{ + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, Steps: []r.TestStep{ { - Config: `resource "random_string" "one" { - length = 16 + Config: `output "test" { value = "original" }`, + }, + { + Config: `output "test" { value = "original" }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectNonEmptyPlan(), + }, + }, + ExpectError: regexp.MustCompile(`expected a non-empty plan, but got an empty plan`), + }, + }, + }) +} + +func Test_ExpectNonEmptyPlan_ResourceChanges(t *testing.T) { + t.Parallel() + + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version1_4_0), + }, + ExternalProviders: map[string]r.ExternalProvider{ + "terraform": {Source: "terraform.io/builtin/terraform"}, + }, + Steps: []r.TestStep{ + { + Config: `resource "terraform_data" "one" { + triggers_replace = ["original"] }`, }, { - Config: `resource "random_string" "one" { - length = 12 + Config: `resource "terraform_data" "one" { + triggers_replace = ["new"] }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ @@ -40,24 +97,25 @@ func Test_ExpectNonEmptyPlan(t *testing.T) { }) } -func Test_ExpectNonEmptyPlan_Error(t *testing.T) { +func Test_ExpectNonEmptyPlan_ResourceChanges_Error(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version1_4_0), + }, ExternalProviders: map[string]r.ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, + "terraform": {Source: "terraform.io/builtin/terraform"}, }, Steps: []r.TestStep{ { - Config: `resource "random_string" "one" { - length = 16 + Config: `resource "terraform_data" "one" { + triggers_replace = ["original"] }`, }, { - Config: `resource "random_string" "one" { - length = 16 + Config: `resource "terraform_data" "one" { + triggers_replace = ["original"] }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{