From ba4b604a523c06119692cdc12d43af35e2a80008 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 4 Nov 2022 11:13:12 -0400 Subject: [PATCH] helper/resource: Skip data source states with TestStep.ImportStateCheck (#1089) * helper/resource: Skip data source states with TestStep.ImportStateCheck Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/1060 The `TestStep` type `ImportStateCheck` functionality is for verifying two specific scenarios over `ImportStateVerify`: - Resources which create multiple resources during import (an implementation detail which is an legacy anti-pattern that should no longer be present in resources). - Resources where importing may cause attributes to have syntactically different but semantically/functionally equivalent values that requires special logic to check. Terraform 1.3 and later can include data source states when importing resources. Rather than forcing provider developers to account for this Terraform version-specific behavior, the `ImportStateCheck` logic will now only receive managed resource states to match the intended usage. Previously with Terraform 1.2.9 and earlier: ``` --- PASS: TestTest_TestStep_ImportStateCheck_SkipDataSourceState (1.58s) ``` Previously with Terraform 1.3.4: ``` --- FAIL: TestTest_TestStep_ImportStateCheck_SkipDataSourceState (0.63s) /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/resource/testing_new_import_state_test.go:16: expected 1 state, got: 2 ``` Now with Terraform 1.3.4: ``` --- PASS: TestTest_TestStep_ImportStateCheck_SkipDataSourceState (0.91s) ``` * Update CHANGELOG for #1089 --- .changelog/1089.txt | 3 + helper/resource/testing.go | 10 +++ helper/resource/testing_new_import_state.go | 16 ++-- .../resource/testing_new_import_state_test.go | 82 +++++++++++++++++++ 4 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 .changelog/1089.txt create mode 100644 helper/resource/testing_new_import_state_test.go diff --git a/.changelog/1089.txt b/.changelog/1089.txt new file mode 100644 index 00000000000..395f3e8bd74 --- /dev/null +++ b/.changelog/1089.txt @@ -0,0 +1,3 @@ +```release-note:bug +helper/resource: Fixed `TestStep` type `ImportStateCheck` field so that it only matches against resources following a change in behaviour in Terraform 1.3 that imports both resources and data sources into state +``` diff --git a/helper/resource/testing.go b/helper/resource/testing.go index c77f0629ac6..caadc3ddc5f 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -551,6 +551,16 @@ type TestStep struct { // ImportStateCheck checks the results of ImportState. It should be // used to verify that the resulting value of ImportState has the // proper resources, IDs, and attributes. + // + // Prefer ImportStateVerify over ImportStateCheck, unless the resource + // import explicitly is expected to create multiple resources (not a + // recommended resource implementation) or if attributes are imported with + // syntactically different but semantically/functionally equivalent values + // where special logic is needed. + // + // Terraform versions 1.3 and later can include data source states during + // import, which the testing framework will skip to prevent the need for + // Terraform version specific logic in provider testing. ImportStateCheck ImportStateCheckFunc // ImportStateVerify, if true, will also check that the state values diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 17b4c2add43..0ec007168bb 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -137,12 +137,18 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest logging.HelperResourceTrace(ctx, "Using TestStep ImportStateCheck") var states []*terraform.InstanceState - for _, r := range importState.RootModule().Resources { - if r.Primary != nil { - is := r.Primary.DeepCopy() - is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type - states = append(states, is) + for address, r := range importState.RootModule().Resources { + if strings.HasPrefix(address, "data.") { + continue } + + if r.Primary == nil { + continue + } + + is := r.Primary.DeepCopy() + is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type + states = append(states, is) } logging.HelperResourceDebug(ctx, "Calling TestStep ImportStateCheck") diff --git a/helper/resource/testing_new_import_state_test.go b/helper/resource/testing_new_import_state_test.go new file mode 100644 index 00000000000..ad4774e4605 --- /dev/null +++ b/helper/resource/testing_new_import_state_test.go @@ -0,0 +1,82 @@ +package resource + +import ( + "context" + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +func TestTest_TestStep_ImportStateCheck_SkipDataSourceState(t *testing.T) { + t.Parallel() + + UnitTest(t, TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "examplecloud": func() (*schema.Provider, error) { //nolint:unparam // required signature + return &schema.Provider{ + DataSourcesMap: map[string]*schema.Resource{ + "examplecloud_thing": { + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("datasource-test") + + return nil + }, + Schema: map[string]*schema.Schema{ + "id": { + Type: schema.TypeString, + Computed: true, + }, + }, + }, + }, + ResourcesMap: map[string]*schema.Resource{ + "examplecloud_thing": { + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("resource-test") + + return nil + }, + DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + ReadContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + Schema: map[string]*schema.Schema{ + "id": { + Computed: true, + Type: schema.TypeString, + }, + }, + Importer: &schema.ResourceImporter{ + StateContext: schema.ImportStatePassthroughContext, + }, + }, + }, + }, nil + }, + }, + Steps: []TestStep{ + { + Config: ` + data "examplecloud_thing" "test" {} + resource "examplecloud_thing" "test" {} + `, + }, + { + ResourceName: "examplecloud_thing.test", + ImportState: true, + ImportStateCheck: func(is []*terraform.InstanceState) error { + if len(is) > 1 { + return fmt.Errorf("expected 1 state, got: %d", len(is)) + } + + return nil + }, + }, + }, + }) +}