From e96524f0834ab65c1857294e5c9065139889a37c Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 12:06:02 -0400 Subject: [PATCH 01/11] Error on use of ImportStatePersist with plannable import --- .../importstate/import_block_as_first_step_test.go | 2 -- helper/resource/testing_new_import_state.go | 10 ++++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/helper/resource/importstate/import_block_as_first_step_test.go b/helper/resource/importstate/import_block_as_first_step_test.go index 919659ef1..91d97d28d 100644 --- a/helper/resource/importstate/import_block_as_first_step_test.go +++ b/helper/resource/importstate/import_block_as_first_step_test.go @@ -38,12 +38,10 @@ func Test_ImportBlock_AsFirstStep(t *testing.T) { ImportStateId: "examplecloud_container.test", ImportState: true, ImportStateKind: r.ImportBlockWithID, - // ImportStateVerify: true, Config: `resource "examplecloud_container" "test" { name = "somevalue" location = "westeurope" }`, - ImportStatePersist: true, ImportPlanChecks: r.ImportPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("examplecloud_container.test", plancheck.ResourceActionNoop), diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 3f74838d3..d1cb51514 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -30,6 +30,8 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest // step.ImportStateKind implicitly defaults to the zero-value (ImportCommandWithID) for backward compatibility kind := step.ImportStateKind + importStatePersist := step.ImportStatePersist + if kind.plannable() { // Instead of calling [t.Fatal], return an error. This package's unit tests can use [TestStep.ExpectError] to match on the error message. // An alternative, [plugintest.TestExpectTFatal], does not have access to logged error messages, so it is open to false positives on this @@ -37,6 +39,10 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest if err := requirePlannableImport(t, *helper.TerraformVersion()); err != nil { return err } + + if importStatePersist { + return fmt.Errorf("ImportStatePersist is not supported with plannable import blocks") + } } configRequest := teststep.PrepareConfigurationRequest{ @@ -145,7 +151,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest var importWd *plugintest.WorkingDir // Use the same working directory to persist the state from import - if step.ImportStatePersist { + if importStatePersist { importWd = wd } else { importWd = helper.RequireNewWorkingDir(ctx, t, "") @@ -157,7 +163,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest t.Fatalf("Error setting test config: %s", err) } - if !step.ImportStatePersist { + if !importStatePersist { err = runProviderCommand(ctx, t, func() error { logging.HelperResourceDebug(ctx, "Run terraform init") return importWd.Init(ctx) From 12842abb829ec3a88f85164542c9f60e84ac7d5c Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 15:40:18 -0400 Subject: [PATCH 02/11] Error on use of ImportStateVerify with plannable import --- .../import_block_in_config_directory_test.go | 9 +++--- .../import_block_in_config_file_test.go | 9 +++--- .../importstate/import_block_with_id_test.go | 32 ++++++++----------- helper/resource/testing_new_import_state.go | 32 ++++++++++--------- 4 files changed, 38 insertions(+), 44 deletions(-) diff --git a/helper/resource/importstate/import_block_in_config_directory_test.go b/helper/resource/importstate/import_block_in_config_directory_test.go index 2cb9ab6e1..d26b0ca0d 100644 --- a/helper/resource/importstate/import_block_in_config_directory_test.go +++ b/helper/resource/importstate/import_block_in_config_directory_test.go @@ -35,11 +35,10 @@ func Test_ImportBlock_InConfigDirectory(t *testing.T) { ConfigDirectory: config.StaticDirectory(`testdata/1`), }, { - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, - ConfigDirectory: config.StaticDirectory(`testdata/2`), + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ConfigDirectory: config.StaticDirectory(`testdata/2`), }, }, }) diff --git a/helper/resource/importstate/import_block_in_config_file_test.go b/helper/resource/importstate/import_block_in_config_file_test.go index 8b983e310..b9cc88698 100644 --- a/helper/resource/importstate/import_block_in_config_file_test.go +++ b/helper/resource/importstate/import_block_in_config_file_test.go @@ -35,11 +35,10 @@ func Test_ImportBlock_InConfigFile(t *testing.T) { ConfigFile: config.StaticFile(`testdata/1/examplecloud_container.tf`), }, { - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, - ConfigFile: config.StaticFile(`testdata/2/examplecloud_container_import.tf`), + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ConfigFile: config.StaticFile(`testdata/2/examplecloud_container_import.tf`), }, }, }) diff --git a/helper/resource/importstate/import_block_with_id_test.go b/helper/resource/importstate/import_block_with_id_test.go index 0f65af713..1db1fc42e 100644 --- a/helper/resource/importstate/import_block_with_id_test.go +++ b/helper/resource/importstate/import_block_with_id_test.go @@ -44,10 +44,9 @@ func Test_TestStep_ImportBlockId(t *testing.T) { }`, }, { - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, }, }, }) @@ -81,11 +80,10 @@ func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) { location = "eastus" name = "somevalue" }`, - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, - ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`), + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`), }, }, }) @@ -290,11 +288,9 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes resource "examplecloud_container" "test" { name = "somename" }`, - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"password"}, + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, }, }, }) @@ -375,11 +371,9 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore(t *testing.T) { Config: `resource "examplecloud_container" "test" {}`, }, { - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"password"}, + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, }, }, }) diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index d1cb51514..7d554a52f 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -9,8 +9,6 @@ import ( "reflect" "strings" - "github.com/hashicorp/go-version" - tfjson "github.com/hashicorp/terraform-json" "github.com/google/go-cmp/cmp" @@ -32,17 +30,8 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest kind := step.ImportStateKind importStatePersist := step.ImportStatePersist - if kind.plannable() { - // Instead of calling [t.Fatal], return an error. This package's unit tests can use [TestStep.ExpectError] to match on the error message. - // An alternative, [plugintest.TestExpectTFatal], does not have access to logged error messages, so it is open to false positives on this - // complex code path. - if err := requirePlannableImport(t, *helper.TerraformVersion()); err != nil { - return err - } - - if importStatePersist { - return fmt.Errorf("ImportStatePersist is not supported with plannable import blocks") - } + if err := importStatePreconditions(t, helper, step); err != nil { + return err } configRequest := teststep.PrepareConfigurationRequest{ @@ -408,15 +397,28 @@ func appendImportBlock(config string, resourceName string, importID string) stri resourceName, importID) } -func requirePlannableImport(t testing.T, versionUnderTest version.Version) error { +func importStatePreconditions(t testing.T, helper *plugintest.Helper, step TestStep) error { t.Helper() - if versionUnderTest.LessThan(tfversion.Version1_5_0) { + kind := step.ImportStateKind + versionUnderTest := *helper.TerraformVersion() + + // Instead of calling [t.Fatal], we return an error. This package's unit tests can use [TestStep.ExpectError] to match + // on the error message. An alternative, [plugintest.TestExpectTFatal], does not have access to logged error messages, + // so it is open to false positives on this complex code path. + switch { + case kind.plannable() && versionUnderTest.LessThan(tfversion.Version1_5_0): return fmt.Errorf( `ImportState steps using plannable import blocks require Terraform 1.5.0 or later. Either ` + `upgrade the Terraform version running the test or add a ` + "`TerraformVersionChecks`" + ` to ` + `the test case to skip this test.` + "\n\n" + `https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/tfversion-checks#skip-version-checks`) + + case kind.plannable() && step.ImportStatePersist: + return fmt.Errorf(`ImportStatePersist is not supported with plannable import blocks`) + + case kind.plannable() && step.ImportStateVerify: + return fmt.Errorf(`ImportStateVerify is not supported with plannable import blocks`) } return nil From 91b5a0c6d5c25cb50bf65e12f5d8e34ccfed5236 Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 15:25:42 -0400 Subject: [PATCH 03/11] Rename test functions for consistency and for running by pattern --- .../importstate/import_block_as_first_step_test.go | 2 +- .../import_block_in_config_directory_test.go | 2 +- .../import_block_in_config_file_test.go | 2 +- .../importstate/import_block_with_id_test.go | 14 ++++++-------- .../import_command_as_first_step_test.go | 2 +- .../importstate/import_command_with_id_test.go | 8 ++++---- 6 files changed, 14 insertions(+), 16 deletions(-) diff --git a/helper/resource/importstate/import_block_as_first_step_test.go b/helper/resource/importstate/import_block_as_first_step_test.go index 91d97d28d..0b8cf26ee 100644 --- a/helper/resource/importstate/import_block_as_first_step_test.go +++ b/helper/resource/importstate/import_block_as_first_step_test.go @@ -18,7 +18,7 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -func Test_ImportBlock_AsFirstStep(t *testing.T) { +func TestImportBlock_AsFirstStep(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ diff --git a/helper/resource/importstate/import_block_in_config_directory_test.go b/helper/resource/importstate/import_block_in_config_directory_test.go index d26b0ca0d..df6feecba 100644 --- a/helper/resource/importstate/import_block_in_config_directory_test.go +++ b/helper/resource/importstate/import_block_in_config_directory_test.go @@ -16,7 +16,7 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -func Test_ImportBlock_InConfigDirectory(t *testing.T) { +func TestImportBlock_InConfigDirectory(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ diff --git a/helper/resource/importstate/import_block_in_config_file_test.go b/helper/resource/importstate/import_block_in_config_file_test.go index b9cc88698..13ef62e19 100644 --- a/helper/resource/importstate/import_block_in_config_file_test.go +++ b/helper/resource/importstate/import_block_in_config_file_test.go @@ -16,7 +16,7 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -func Test_ImportBlock_InConfigFile(t *testing.T) { +func TestImportBlock_InConfigFile(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ diff --git a/helper/resource/importstate/import_block_with_id_test.go b/helper/resource/importstate/import_block_with_id_test.go index 1db1fc42e..21368cba8 100644 --- a/helper/resource/importstate/import_block_with_id_test.go +++ b/helper/resource/importstate/import_block_with_id_test.go @@ -21,7 +21,7 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -func Test_TestStep_ImportBlockId(t *testing.T) { +func TestImportBlock_WithID(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -52,7 +52,7 @@ func Test_TestStep_ImportBlockId(t *testing.T) { }) } -func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) { +func TestImportBlock_WithID_ExpectError(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -89,7 +89,7 @@ func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) { }) } -func TestTest_TestStep_ImportBlockId_FailWhenPlannableImportIsNotSupported(t *testing.T) { +func TestImportBlock_WithID_FailWhenNotSupported(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -128,7 +128,7 @@ func TestTest_TestStep_ImportBlockId_FailWhenPlannableImportIsNotSupported(t *te }) } -func TestTest_TestStep_ImportBlockId_SkipDataSourceState(t *testing.T) { +func TestImportBlock_WithID_SkipsDataSources(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -172,9 +172,7 @@ func TestTest_TestStep_ImportBlockId_SkipDataSourceState(t *testing.T) { }) } -// These tests currently pass but only because the `getState` function which is used on the imported resource -// to do the state comparison doesn't return an error if there is no state in the working directory -func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *testing.T) { +func TestImportBlock_WithID_WithBlankOptionalAttribute_GeneratesCorrectPlan(t *testing.T) { /* This test tries to imitate a real world example of behaviour we often see in the AzureRM provider which requires the use of `ImportStateVerifyIgnore` when testing the import of a resource using the import command. @@ -296,7 +294,7 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes }) } -func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore(t *testing.T) { +func TestImportBlock_WithID_WithBlankComputedAttribute_GeneratesCorrectPlan(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ diff --git a/helper/resource/importstate/import_command_as_first_step_test.go b/helper/resource/importstate/import_command_as_first_step_test.go index 771ea8dc4..14db18828 100644 --- a/helper/resource/importstate/import_command_as_first_step_test.go +++ b/helper/resource/importstate/import_command_as_first_step_test.go @@ -17,7 +17,7 @@ import ( r "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -func Test_ImportCommand_AsFirstStep(t *testing.T) { +func TestImportCommand_AsFirstStep(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ diff --git a/helper/resource/importstate/import_command_with_id_test.go b/helper/resource/importstate/import_command_with_id_test.go index df6e43460..34e211b67 100644 --- a/helper/resource/importstate/import_command_with_id_test.go +++ b/helper/resource/importstate/import_command_with_id_test.go @@ -20,7 +20,7 @@ import ( "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func Test_TestStep_ImportStateCheck_SkipDataSourceState(t *testing.T) { +func TestImportCommand_ImportStateCheckSkipsDataSources(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -123,7 +123,7 @@ func Test_TestStep_ImportStateCheck_SkipDataSourceState(t *testing.T) { }) } -func Test_TestStep_ImportStateVerify(t *testing.T) { +func TestImportCommand_ImportStateVerify(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -197,7 +197,7 @@ func Test_TestStep_ImportStateVerify(t *testing.T) { }) } -func Test_TestStep_ImportStateVerifyIgnore(t *testing.T) { +func TestImportCommand_ImportStateVerify_Ignore(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ @@ -281,7 +281,7 @@ func Test_TestStep_ImportStateVerifyIgnore(t *testing.T) { }) } -func Test_TestStep_ExpectError_ImportState(t *testing.T) { +func TestImportCommand_ExpectError(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ From 2456d713b76d27369cb95a8263e1d7a298f22e44 Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 16:20:29 -0400 Subject: [PATCH 04/11] Import block works for a resource with a dependency --- .../resource/importstate/examplecloud_test.go | 153 ++++++++++++++++++ ...ock_for_resource_with_a_dependency_test.go | 52 ++++++ helper/resource/testing_new_import_state.go | 64 +++++--- internal/plugintest/working_dir.go | 50 ++++++ 4 files changed, 296 insertions(+), 23 deletions(-) create mode 100644 helper/resource/importstate/import_block_for_resource_with_a_dependency_test.go diff --git a/helper/resource/importstate/examplecloud_test.go b/helper/resource/importstate/examplecloud_test.go index c7086d6c3..1c3fefea6 100644 --- a/helper/resource/importstate/examplecloud_test.go +++ b/helper/resource/importstate/examplecloud_test.go @@ -4,6 +4,8 @@ package importstate_test import ( + "context" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-testing/internal/testing/testprovider" @@ -116,3 +118,154 @@ func examplecloudResource() testprovider.Resource { }, } } + +// examplecloudZone is a test resource that mimics a DNS zone resource. +func examplecloudZone() testprovider.Resource { + return testprovider.Resource{ + CreateResponse: &resource.CreateResponse{ + NewState: tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "example.net"), + }, + ), + }, + ReadResponse: &resource.ReadResponse{ + NewState: tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "example.net"), + }, + ), + }, + ImportStateResponse: &resource.ImportStateResponse{ + State: tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "example.net"), + }, + ), + }, + SchemaResponse: &resource.SchemaResponse{ + Schema: &tfprotov6.Schema{ + Block: &tfprotov6.SchemaBlock{ + Attributes: []*tfprotov6.SchemaAttribute{ + { + Name: "id", + Type: tftypes.String, + Computed: true, + }, + { + Name: "name", + Type: tftypes.String, + Required: true, + }, + }, + }, + }, + }, + } +} + +// examplecloudZoneRecord is a test resource that mimics a DNS zone record resource. +// It models a resource dependency; specifically, it depends on a DNS zone ID and will +// plan a replacement if the zone ID changes. +func examplecloudZoneRecord() testprovider.Resource { + return testprovider.Resource{ + CreateResponse: &resource.CreateResponse{ + NewState: tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "zone_id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), + "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "www"), + }, + ), + }, + + PlanChangeFunc: func(ctx context.Context, req resource.PlanChangeRequest, resp *resource.PlanChangeResponse) { + resp.RequiresReplace = []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("zone_id"), + } + }, + ReadResponse: &resource.ReadResponse{ + NewState: tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "zone_id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), + "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "www"), + }, + ), + }, + ImportStateResponse: &resource.ImportStateResponse{ + State: tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "zone_id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), + "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "www"), + }, + ), + }, + SchemaResponse: &resource.SchemaResponse{ + Schema: &tfprotov6.Schema{ + Block: &tfprotov6.SchemaBlock{ + Attributes: []*tfprotov6.SchemaAttribute{ + { + Name: "id", + Type: tftypes.String, + Computed: true, + }, + { + Name: "zone_id", + Type: tftypes.String, + Required: true, + }, + { + Name: "name", + Type: tftypes.String, + Required: true, + }, + }, + }, + }, + }, + } +} diff --git a/helper/resource/importstate/import_block_for_resource_with_a_dependency_test.go b/helper/resource/importstate/import_block_for_resource_with_a_dependency_test.go new file mode 100644 index 000000000..2464929dd --- /dev/null +++ b/helper/resource/importstate/import_block_for_resource_with_a_dependency_test.go @@ -0,0 +1,52 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package importstate_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/internal/testing/testprovider" + "github.com/hashicorp/terraform-plugin-testing/internal/testing/testsdk/providerserver" + "github.com/hashicorp/terraform-plugin-testing/tfversion" +) + +func TestImportBlockForResourceWithADependency(t *testing.T) { + t.Parallel() + + config := ` +resource "examplecloud_zone" "zone" { + name = "example.net" +} + +resource "examplecloud_zone_record" "record" { + zone_id = examplecloud_zone.zone.id + name = "www" +} +` + r.UnitTest(t, r.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithID requires Terraform 1.5.0 or later + }, + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "examplecloud": providerserver.NewProviderServer(testprovider.Provider{ + Resources: map[string]testprovider.Resource{ + "examplecloud_zone": examplecloudZone(), + "examplecloud_zone_record": examplecloudZoneRecord(), + }, + }), + }, + Steps: []r.TestStep{ + { + Config: config, + }, + { + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ResourceName: "examplecloud_zone_record.record", + }, + }, + }) +} diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 7d554a52f..fa8917305 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -144,7 +144,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest importWd = wd } else { importWd = helper.RequireNewWorkingDir(ctx, t, "") - defer importWd.Close() //nolint:errcheck + defer importWd.Close() } err = importWd.SetConfig(ctx, testStepConfig, step.ConfigVariables) @@ -152,6 +152,22 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest t.Fatalf("Error setting test config: %s", err) } + if kind.plannable() { + if stepNumber > 1 { + err = importWd.CopyState(ctx, wd.StateFilePath()) + if err != nil { + t.Fatalf("copying state: %s", err) + } + + err = runProviderCommand(ctx, t, func() error { + return importWd.RemoveResource(ctx, resourceName) + }, importWd, providers) + if err != nil { + t.Fatalf("removing resource %s from copied state: %s", resourceName, err) + } + } + } + if !importStatePersist { err = runProviderCommand(ctx, t, func() error { logging.HelperResourceDebug(ctx, "Run terraform init") @@ -184,35 +200,37 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest return err } - if plan.ResourceChanges != nil { - logging.HelperResourceDebug(ctx, fmt.Sprintf("ImportBlockWithId: %d resource changes", len(plan.ResourceChanges))) + logging.HelperResourceDebug(ctx, fmt.Sprintf("ImportBlockWithId: %d resource changes", len(plan.ResourceChanges))) - for _, rc := range plan.ResourceChanges { - if rc.Address != resourceName { - // we're only interested in the changes for the resource being imported - continue - } - if rc.Change != nil && rc.Change.Actions != nil { - // should this be length checked and used as a condition, if it's a no-op then there shouldn't be any other changes here - for _, action := range rc.Change.Actions { - if action != "no-op" { - var stdout string - err = runProviderCommand(ctx, t, func() error { - var err error - stdout, err = importWd.SavedPlanRawStdout(ctx) - return err - }, importWd, providers) - if err != nil { - return fmt.Errorf("retrieving formatted plan output: %w", err) - } - - return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", rc.Address, action, stdout) + for _, rc := range plan.ResourceChanges { + if rc.Address != resourceName { + // we're only interested in the changes for the resource being imported + continue + } + if rc.Change != nil && rc.Change.Actions != nil { + // should this be length checked and used as a condition, if it's a no-op then there shouldn't be any other changes here + for _, action := range rc.Change.Actions { + if action != "no-op" { + var stdout string + err = runProviderCommand(ctx, t, func() error { + var err error + stdout, err = importWd.SavedPlanRawStdout(ctx) + return err + }, importWd, providers) + if err != nil { + return fmt.Errorf("retrieving formatted plan output: %w", err) } + + return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", rc.Address, action, stdout) } } } } + if len(plan.ResourceChanges) == 0 { + return fmt.Errorf("importing resource %s: expected a resource change, got no changes", resourceName) + } + // TODO compare plan to state from previous step if err := runPlanChecks(ctx, t, plan, step.ImportPlanChecks.PreApply); err != nil { diff --git a/internal/plugintest/working_dir.go b/internal/plugintest/working_dir.go index 0cff33408..d29425c32 100644 --- a/internal/plugintest/working_dir.go +++ b/internal/plugintest/working_dir.go @@ -6,6 +6,7 @@ package plugintest import ( "context" "fmt" + "io" "os" "path/filepath" @@ -173,6 +174,40 @@ func (wd *WorkingDir) ClearState(ctx context.Context) error { return nil } +func (wd *WorkingDir) CopyState(ctx context.Context, src string) error { + srcState, err := os.Open(src) + if err != nil { + return fmt.Errorf("failed to open statefile for read: %w", err) + } + + defer srcState.Close() + + dstState, err := os.Create(filepath.Join(wd.baseDir, "terraform.tfstate")) + if err != nil { + return fmt.Errorf("failed to open statefile for write: %w", err) + } + + defer dstState.Close() + + buf := make([]byte, 1024) + for { + n, err := srcState.Read(buf) + if err != nil { + if err == io.EOF { + break + } + return fmt.Errorf("failed to read from statefile: %w", err) + } + + _, err = dstState.Write(buf[:n]) + if err != nil { + return fmt.Errorf("failed to write to statefile: %w", err) + } + } + + return nil +} + // ClearPlan deletes any saved plan present in the working directory. func (wd *WorkingDir) ClearPlan(ctx context.Context) error { logging.HelperResourceTrace(ctx, "Clearing Terraform plan") @@ -295,6 +330,17 @@ func (wd *WorkingDir) HasSavedPlan() bool { return err == nil } +// RemoveResource removes a resource from state. +func (wd *WorkingDir) RemoveResource(ctx context.Context, address string) error { + logging.HelperResourceTrace(ctx, "Calling Terraform CLI state rm command") + + err := wd.tf.StateRm(context.Background(), address) + + logging.HelperResourceTrace(ctx, "Called Terraform CLI state rm command") + + return err +} + // SavedPlan returns an object describing the current saved plan file, if any. // // If no plan is saved or if the plan file cannot be read, SavedPlan returns @@ -349,6 +395,10 @@ func (wd *WorkingDir) State(ctx context.Context) (*tfjson.State, error) { return state, err } +func (wd *WorkingDir) StateFilePath() string { + return filepath.Join(wd.baseDir, "terraform.tfstate") +} + // Import runs terraform import func (wd *WorkingDir) Import(ctx context.Context, resource, id string) error { logging.HelperResourceTrace(ctx, "Calling Terraform CLI import command") From ccff9916cf066968ecd2080044836875c3deb14a Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 13:07:30 -0400 Subject: [PATCH 05/11] Simplify no-op action check --- .../importstate/import_block_with_id_test.go | 2 +- helper/resource/testing_new_import_state.go | 49 ++++++++++--------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/helper/resource/importstate/import_block_with_id_test.go b/helper/resource/importstate/import_block_with_id_test.go index 21368cba8..015cb78a7 100644 --- a/helper/resource/importstate/import_block_with_id_test.go +++ b/helper/resource/importstate/import_block_with_id_test.go @@ -83,7 +83,7 @@ func TestImportBlock_WithID_ExpectError(t *testing.T) { ResourceName: "examplecloud_container.test", ImportState: true, ImportStateKind: r.ImportBlockWithID, - ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`), + ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got \["update"\] action with plan(.?)`), }, }, }) diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index fa8917305..1cceeb86d 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -202,36 +202,37 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest logging.HelperResourceDebug(ctx, fmt.Sprintf("ImportBlockWithId: %d resource changes", len(plan.ResourceChanges))) - for _, rc := range plan.ResourceChanges { - if rc.Address != resourceName { - // we're only interested in the changes for the resource being imported - continue - } - if rc.Change != nil && rc.Change.Actions != nil { - // should this be length checked and used as a condition, if it's a no-op then there shouldn't be any other changes here - for _, action := range rc.Change.Actions { - if action != "no-op" { - var stdout string - err = runProviderCommand(ctx, t, func() error { - var err error - stdout, err = importWd.SavedPlanRawStdout(ctx) - return err - }, importWd, providers) - if err != nil { - return fmt.Errorf("retrieving formatted plan output: %w", err) - } - - return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", rc.Address, action, stdout) - } - } + // Verify reasonable things about the plan + var resourceChangeUnderTest *tfjson.ResourceChange + + if len(plan.ResourceChanges) == 0 { + return fmt.Errorf("importing resource %s: expected a resource change, got no changes", resourceName) + } + + for _, change := range plan.ResourceChanges { + if change.Address == resourceName { + resourceChangeUnderTest = change } } - if len(plan.ResourceChanges) == 0 { + if resourceChangeUnderTest == nil || resourceChangeUnderTest.Change == nil || resourceChangeUnderTest.Change.Actions == nil { return fmt.Errorf("importing resource %s: expected a resource change, got no changes", resourceName) } - // TODO compare plan to state from previous step + actions := resourceChangeUnderTest.Change.Actions + if !actions.NoOp() { + var stdout string + err = runProviderCommand(ctx, t, func() error { + var err error + stdout, err = importWd.SavedPlanRawStdout(ctx) + return err + }, importWd, providers) + if err != nil { + return fmt.Errorf("retrieving formatted plan output: %w", err) + } + + return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", resourceChangeUnderTest.Address, actions, stdout) + } if err := runPlanChecks(ctx, t, plan, step.ImportPlanChecks.PreApply); err != nil { return err From 35f3b06bd91282ca1e726ea0bcceda4de27d3f91 Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 17:08:18 -0400 Subject: [PATCH 06/11] Add prerelease changelog entries --- .changes/unreleased/BREAKING CHANGES-20250404-170656.yaml | 5 +++++ .changes/unreleased/BUG FIXES-20250404-170734.yaml | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changes/unreleased/BREAKING CHANGES-20250404-170656.yaml create mode 100644 .changes/unreleased/BUG FIXES-20250404-170734.yaml diff --git a/.changes/unreleased/BREAKING CHANGES-20250404-170656.yaml b/.changes/unreleased/BREAKING CHANGES-20250404-170656.yaml new file mode 100644 index 000000000..aaae02bab --- /dev/null +++ b/.changes/unreleased/BREAKING CHANGES-20250404-170656.yaml @@ -0,0 +1,5 @@ +kind: BREAKING CHANGES +body: 'importstate: `ImportStatePersist` and `ImportStateVerify` are not supported for plannable import (`ImportBlockWith*`) and will return an error' +time: 2025-04-04T17:06:56.900935-04:00 +custom: + Issue: "476" diff --git a/.changes/unreleased/BUG FIXES-20250404-170734.yaml b/.changes/unreleased/BUG FIXES-20250404-170734.yaml new file mode 100644 index 000000000..875128b40 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20250404-170734.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'importstate: plannable import (`ImportBlockWith*`) fixed for a resource with a dependency' +time: 2025-04-04T17:07:34.428542-04:00 +custom: + Issue: "476" From 90f56568a119d411a65392024abfefe169a9cd2e Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 17:11:01 -0400 Subject: [PATCH 07/11] Add a catch-up prerelease changelog entry --- .changes/unreleased/BREAKING CHANGES-20250404-171048.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/unreleased/BREAKING CHANGES-20250404-171048.yaml diff --git a/.changes/unreleased/BREAKING CHANGES-20250404-171048.yaml b/.changes/unreleased/BREAKING CHANGES-20250404-171048.yaml new file mode 100644 index 000000000..4c4e57876 --- /dev/null +++ b/.changes/unreleased/BREAKING CHANGES-20250404-171048.yaml @@ -0,0 +1,5 @@ +kind: BREAKING CHANGES +body: 'importstate: renamed `ImportStateWithId` to `ImportStateWithID` and renamed `ImportCommandWithId` to `ImportCommandWithID`.' +time: 2025-04-04T17:10:48.525611-04:00 +custom: + Issue: "465" From e472964574e92d48da8bbd38842f1ca9a91e6cfd Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 18:07:01 -0400 Subject: [PATCH 08/11] Refactor test provider functions --- .../resource/importstate/examplecloud_test.go | 167 +++++------------- helper/resource/importstate/types_test.go | 33 ++++ 2 files changed, 76 insertions(+), 124 deletions(-) create mode 100644 helper/resource/importstate/types_test.go diff --git a/helper/resource/importstate/examplecloud_test.go b/helper/resource/importstate/examplecloud_test.go index 1c3fefea6..93f7b8391 100644 --- a/helper/resource/importstate/examplecloud_test.go +++ b/helper/resource/importstate/examplecloud_test.go @@ -31,11 +31,7 @@ func examplecloudDataSource() testprovider.DataSource { Schema: &tfprotov6.Schema{ Block: &tfprotov6.SchemaBlock{ Attributes: []*tfprotov6.SchemaAttribute{ - { - Name: "id", - Type: tftypes.String, - Computed: true, - }, + ComputedStringAttribute("id"), }, }, }, @@ -97,21 +93,9 @@ func examplecloudResource() testprovider.Resource { Schema: &tfprotov6.Schema{ Block: &tfprotov6.SchemaBlock{ Attributes: []*tfprotov6.SchemaAttribute{ - { - Name: "id", - Type: tftypes.String, - Computed: true, - }, - { - Name: "location", - Type: tftypes.String, - Required: true, - }, - { - Name: "name", - Type: tftypes.String, - Required: true, - }, + ComputedStringAttribute("id"), + RequiredStringAttribute("location"), + RequiredStringAttribute("name"), }, }, }, @@ -121,63 +105,35 @@ func examplecloudResource() testprovider.Resource { // examplecloudZone is a test resource that mimics a DNS zone resource. func examplecloudZone() testprovider.Resource { + value := tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "example.net"), + }, + ) + return testprovider.Resource{ CreateResponse: &resource.CreateResponse{ - NewState: tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "id": tftypes.String, - "name": tftypes.String, - }, - }, - map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), - "name": tftypes.NewValue(tftypes.String, "example.net"), - }, - ), + NewState: value, }, ReadResponse: &resource.ReadResponse{ - NewState: tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "id": tftypes.String, - "name": tftypes.String, - }, - }, - map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), - "name": tftypes.NewValue(tftypes.String, "example.net"), - }, - ), + NewState: value, }, ImportStateResponse: &resource.ImportStateResponse{ - State: tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "id": tftypes.String, - "name": tftypes.String, - }, - }, - map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), - "name": tftypes.NewValue(tftypes.String, "example.net"), - }, - ), + State: value, }, SchemaResponse: &resource.SchemaResponse{ Schema: &tfprotov6.Schema{ Block: &tfprotov6.SchemaBlock{ Attributes: []*tfprotov6.SchemaAttribute{ - { - Name: "id", - Type: tftypes.String, - Computed: true, - }, - { - Name: "name", - Type: tftypes.String, - Required: true, - }, + ComputedStringAttribute("id"), + RequiredStringAttribute("name"), }, }, }, @@ -189,80 +145,43 @@ func examplecloudZone() testprovider.Resource { // It models a resource dependency; specifically, it depends on a DNS zone ID and will // plan a replacement if the zone ID changes. func examplecloudZoneRecord() testprovider.Resource { + value := tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "id": tftypes.String, + "zone_id": tftypes.String, + "name": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), + "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), + "name": tftypes.NewValue(tftypes.String, "www"), + }, + ) + return testprovider.Resource{ CreateResponse: &resource.CreateResponse{ - NewState: tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "id": tftypes.String, - "zone_id": tftypes.String, - "name": tftypes.String, - }, - }, - map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), - "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), - "name": tftypes.NewValue(tftypes.String, "www"), - }, - ), + NewState: value, }, - PlanChangeFunc: func(ctx context.Context, req resource.PlanChangeRequest, resp *resource.PlanChangeResponse) { resp.RequiresReplace = []*tftypes.AttributePath{ tftypes.NewAttributePath().WithAttributeName("zone_id"), } }, ReadResponse: &resource.ReadResponse{ - NewState: tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "id": tftypes.String, - "zone_id": tftypes.String, - "name": tftypes.String, - }, - }, - map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), - "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), - "name": tftypes.NewValue(tftypes.String, "www"), - }, - ), + NewState: value, }, ImportStateResponse: &resource.ImportStateResponse{ - State: tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "id": tftypes.String, - "zone_id": tftypes.String, - "name": tftypes.String, - }, - }, - map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "f00911be-e188-433d-9ccd-d0393a9f5d05"), - "zone_id": tftypes.NewValue(tftypes.String, "5381dd14-6d75-4f32-9096-47f5500b1507"), - "name": tftypes.NewValue(tftypes.String, "www"), - }, - ), + State: value, }, SchemaResponse: &resource.SchemaResponse{ Schema: &tfprotov6.Schema{ Block: &tfprotov6.SchemaBlock{ Attributes: []*tfprotov6.SchemaAttribute{ - { - Name: "id", - Type: tftypes.String, - Computed: true, - }, - { - Name: "zone_id", - Type: tftypes.String, - Required: true, - }, - { - Name: "name", - Type: tftypes.String, - Required: true, - }, + ComputedStringAttribute("id"), + RequiredStringAttribute("zone_id"), + RequiredStringAttribute("name"), }, }, }, diff --git a/helper/resource/importstate/types_test.go b/helper/resource/importstate/types_test.go new file mode 100644 index 000000000..8c491d30e --- /dev/null +++ b/helper/resource/importstate/types_test.go @@ -0,0 +1,33 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package importstate_test + +import ( + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +func ComputedStringAttribute(name string) *tfprotov6.SchemaAttribute { + return &tfprotov6.SchemaAttribute{ + Name: name, + Type: tftypes.String, + Computed: true, + } +} + +func OptionalStringAttribute(name string) *tfprotov6.SchemaAttribute { + return &tfprotov6.SchemaAttribute{ + Name: name, + Type: tftypes.String, + Optional: true, + } +} + +func RequiredStringAttribute(name string) *tfprotov6.SchemaAttribute { + return &tfprotov6.SchemaAttribute{ + Name: name, + Type: tftypes.String, + Required: true, + } +} From cde5001f1993eea9e550bf1bdb526ff2fc808d0b Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 19:51:01 -0400 Subject: [PATCH 09/11] Fix 'No changes. Your infrastructure matches the configuration.' false negatives --- .../importstate/import_block_with_id_test.go | 2 +- helper/resource/testing_new_import_state.go | 38 +++++++++++++------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/helper/resource/importstate/import_block_with_id_test.go b/helper/resource/importstate/import_block_with_id_test.go index 015cb78a7..150c84d35 100644 --- a/helper/resource/importstate/import_block_with_id_test.go +++ b/helper/resource/importstate/import_block_with_id_test.go @@ -83,7 +83,7 @@ func TestImportBlock_WithID_ExpectError(t *testing.T) { ResourceName: "examplecloud_container.test", ImportState: true, ImportStateKind: r.ImportBlockWithID, - ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got \["update"\] action with plan(.?)`), + ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op import operation, got.*\["update"\] action with plan(.?)`), }, }, }) diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 1cceeb86d..c77715729 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -219,19 +219,16 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest return fmt.Errorf("importing resource %s: expected a resource change, got no changes", resourceName) } - actions := resourceChangeUnderTest.Change.Actions - if !actions.NoOp() { - var stdout string - err = runProviderCommand(ctx, t, func() error { - var err error - stdout, err = importWd.SavedPlanRawStdout(ctx) - return err - }, importWd, providers) - if err != nil { - return fmt.Errorf("retrieving formatted plan output: %w", err) - } + change := resourceChangeUnderTest.Change + actions := change.Actions + importing := change.Importing - return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", resourceChangeUnderTest.Address, actions, stdout) + switch { + case importing == nil: + return fmt.Errorf("importing resource %s: expected an import operation, got %q action with plan \nstdout:\n\n%s", resourceChangeUnderTest.Address, actions, savedPlanRawStdout(ctx, t, importWd, providers)) + + case !actions.NoOp(): + return fmt.Errorf("importing resource %s: expected a no-op import operation, got %q action with plan \nstdout:\n\n%s", resourceChangeUnderTest.Address, actions, savedPlanRawStdout(ctx, t, importWd, providers)) } if err := runPlanChecks(ctx, t, plan, step.ImportPlanChecks.PreApply); err != nil { @@ -469,3 +466,20 @@ func runImportStateCheckFunction(ctx context.Context, t testing.T, importState * logging.HelperResourceTrace(ctx, "Called TestStep ImportStateCheck") } + +func savedPlanRawStdout(ctx context.Context, t testing.T, wd *plugintest.WorkingDir, providers *providerFactories) string { + t.Helper() + + var stdout string + + err := runProviderCommand(ctx, t, func() error { + var err error + stdout, err = wd.SavedPlanRawStdout(ctx) + return err + }, wd, providers) + + if err != nil { + return fmt.Sprintf("error retrieving formatted plan output: %w", err) + } + return stdout +} From 372066c3edda7d0e972678b623b59fcdf341c998 Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 19:51:30 -0400 Subject: [PATCH 10/11] Log (or do not log) consistently --- helper/resource/testing_new_import_state.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index c77715729..75c4b6d78 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -170,7 +170,6 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest if !importStatePersist { err = runProviderCommand(ctx, t, func() error { - logging.HelperResourceDebug(ctx, "Run terraform init") return importWd.Init(ctx) }, importWd, providers) if err != nil { @@ -183,7 +182,6 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest var opts []tfexec.PlanOption err = runProviderCommand(ctx, t, func() error { - logging.HelperResourceDebug(ctx, "Run terraform plan") return importWd.CreatePlan(ctx, opts...) }, importWd, providers) if err != nil { @@ -192,7 +190,6 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest err = runProviderCommand(ctx, t, func() error { var err error - logging.HelperResourceDebug(ctx, "Run terraform show") plan, err = importWd.SavedPlan(ctx) return err }, importWd, providers) From 526853cea27034f73638d721ac7e5d9a4869b169 Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Fri, 4 Apr 2025 19:55:41 -0400 Subject: [PATCH 11/11] Fix format string syntax --- helper/resource/testing_new_import_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 75c4b6d78..dc3b06ed2 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -476,7 +476,7 @@ func savedPlanRawStdout(ctx context.Context, t testing.T, wd *plugintest.Working }, wd, providers) if err != nil { - return fmt.Sprintf("error retrieving formatted plan output: %w", err) + return fmt.Sprintf("error retrieving formatted plan output: %s", err) } return stdout }