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 59664baa5..919659ef1 100644 --- a/helper/resource/importstate/import_block_as_first_step_test.go +++ b/helper/resource/importstate/import_block_as_first_step_test.go @@ -23,7 +23,7 @@ func Test_ImportBlock_AsFirstStep(t *testing.T) { r.UnitTest(t, r.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ - tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithId requires Terraform 1.5.0 or later + 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{ @@ -37,7 +37,7 @@ func Test_ImportBlock_AsFirstStep(t *testing.T) { ResourceName: "examplecloud_container.test", ImportStateId: "examplecloud_container.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, // ImportStateVerify: true, Config: `resource "examplecloud_container" "test" { name = "somevalue" diff --git a/helper/resource/importstate/import_block_in_config_directory_test.go b/helper/resource/importstate/import_block_in_config_directory_test.go new file mode 100644 index 000000000..2cb9ab6e1 --- /dev/null +++ b/helper/resource/importstate/import_block_in_config_directory_test.go @@ -0,0 +1,46 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package importstate_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + + "github.com/hashicorp/terraform-plugin-testing/config" + "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" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func Test_ImportBlock_InConfigDirectory(t *testing.T) { + t.Parallel() + + 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_container": examplecloudResource(), + }, + }), + }, + Steps: []r.TestStep{ + { + ConfigDirectory: config.StaticDirectory(`testdata/1`), + }, + { + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ImportStateVerify: true, + 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 new file mode 100644 index 000000000..8b983e310 --- /dev/null +++ b/helper/resource/importstate/import_block_in_config_file_test.go @@ -0,0 +1,46 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package importstate_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + + "github.com/hashicorp/terraform-plugin-testing/config" + "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" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func Test_ImportBlock_InConfigFile(t *testing.T) { + t.Parallel() + + 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_container": examplecloudResource(), + }, + }), + }, + Steps: []r.TestStep{ + { + 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`), + }, + }, + }) +} diff --git a/helper/resource/importstate/import_block_with_id_test.go b/helper/resource/importstate/import_block_with_id_test.go index e30717772..0f65af713 100644 --- a/helper/resource/importstate/import_block_with_id_test.go +++ b/helper/resource/importstate/import_block_with_id_test.go @@ -26,7 +26,7 @@ func Test_TestStep_ImportBlockId(t *testing.T) { r.UnitTest(t, r.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ - tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithId requires Terraform 1.5.0 or later + 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{ @@ -46,7 +46,7 @@ func Test_TestStep_ImportBlockId(t *testing.T) { { ResourceName: "examplecloud_container.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, ImportStateVerify: true, }, }, @@ -58,7 +58,7 @@ func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) { r.UnitTest(t, r.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ - tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithId requires Terraform 1.5.0 or later + 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{ @@ -83,9 +83,9 @@ func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) { }`, ResourceName: "examplecloud_container.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, ImportStateVerify: true, - ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test should be a no-op, but got action update with plan(.?)`), + ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`), }, }, }) @@ -122,7 +122,7 @@ func TestTest_TestStep_ImportBlockId_FailWhenPlannableImportIsNotSupported(t *te }`, ResourceName: "examplecloud_container.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, ImportStateVerify: true, ExpectError: regexp.MustCompile(`Terraform 1.5.0`), }, @@ -135,7 +135,7 @@ func TestTest_TestStep_ImportBlockId_SkipDataSourceState(t *testing.T) { r.UnitTest(t, r.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ - tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithId requires Terraform 1.5.0 or later + tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithID requires Terraform 1.5.0 or later }, ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ @@ -161,7 +161,7 @@ func TestTest_TestStep_ImportBlockId_SkipDataSourceState(t *testing.T) { { ResourceName: "examplecloud_thing.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, ImportStateCheck: func(is []*terraform.InstanceState) error { if len(is) > 1 { return fmt.Errorf("expected 1 state, got: %d", len(is)) @@ -204,7 +204,7 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes r.UnitTest(t, r.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ - tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithId requires Terraform 1.5.0 or later + 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{ @@ -292,7 +292,7 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes }`, ResourceName: "examplecloud_container.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"password"}, }, @@ -305,7 +305,7 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore(t *testing.T) { r.UnitTest(t, r.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ - tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithId requires Terraform 1.5.0 or later + 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{ @@ -377,7 +377,7 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore(t *testing.T) { { ResourceName: "examplecloud_container.test", ImportState: true, - ImportStateKind: r.ImportBlockWithId, + ImportStateKind: r.ImportBlockWithID, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"password"}, }, 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 2193dfc7e..771ea8dc4 100644 --- a/helper/resource/importstate/import_command_as_first_step_test.go +++ b/helper/resource/importstate/import_command_as_first_step_test.go @@ -33,11 +33,9 @@ func Test_ImportCommand_AsFirstStep(t *testing.T) { }, Steps: []r.TestStep{ { - ResourceName: "examplecloud_container.test", - ImportStateId: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportCommandWithId, - // ImportStateVerify: true, + ResourceName: "examplecloud_container.test", + ImportStateId: "examplecloud_container.test", + ImportState: true, Config: `resource "examplecloud_container" "test" { name = "somevalue" location = "westeurope" diff --git a/helper/resource/importstate/testdata/1/examplecloud_container.tf b/helper/resource/importstate/testdata/1/examplecloud_container.tf new file mode 100644 index 000000000..ccfb698e6 --- /dev/null +++ b/helper/resource/importstate/testdata/1/examplecloud_container.tf @@ -0,0 +1,7 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +resource "examplecloud_container" "test" { + name = "somevalue" + location = "westeurope" +} diff --git a/helper/resource/importstate/testdata/2/examplecloud_container_import.tf b/helper/resource/importstate/testdata/2/examplecloud_container_import.tf new file mode 100644 index 000000000..f7e9411f9 --- /dev/null +++ b/helper/resource/importstate/testdata/2/examplecloud_container_import.tf @@ -0,0 +1,12 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +resource "examplecloud_container" "test" { + name = "somevalue" + location = "westeurope" +} + +import { + to = examplecloud_container.test + id = "examplecloud_container.test" +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 2a3d8db96..cda787033 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -457,14 +457,28 @@ type ExternalProvider struct { type ImportStateKind byte const ( - // ImportCommandWithId imports the state using the import command - ImportCommandWithId ImportStateKind = iota - // ImportBlockWithId imports the state using an import block with an ID - ImportBlockWithId + // ImportCommandWithID tests import by using the ID string with the `terraform import` command + ImportCommandWithID ImportStateKind = iota + + // ImportBlockWithID tests import by using the ID string in an import configuration block with the `terraform plan` command + ImportBlockWithID + // ImportBlockWithResourceIdentity imports the state using an import block with a resource identity ImportBlockWithResourceIdentity ) +func (kind ImportStateKind) plannable() bool { + return kind == ImportBlockWithID || kind == ImportBlockWithResourceIdentity +} + +func (kind ImportStateKind) String() string { + return map[ImportStateKind]string{ + ImportCommandWithID: "ImportCommandWithID", + ImportBlockWithID: "ImportBlockWithID", + ImportBlockWithResourceIdentity: "ImportBlockWithResourceIdentity", + }[kind] +} + // TestStep is a single apply sequence of a test, done within the // context of a state. // @@ -963,17 +977,17 @@ func UnitTest(t testing.T, c TestCase) { Test(t, c) } -func testResource(c TestStep, state *terraform.State) (*terraform.ResourceState, error) { +func testResource(name string, state *terraform.State) (*terraform.ResourceState, error) { for _, m := range state.Modules { if len(m.Resources) > 0 { - if v, ok := m.Resources[c.ResourceName]; ok { + if v, ok := m.Resources[name]; ok { return v, nil } } } return nil, fmt.Errorf( - "Resource specified by ResourceName couldn't be found: %s", c.ResourceName) + "Resource specified by ResourceName couldn't be found: %s", name) } // ComposeTestCheckFunc lets you compose multiple TestCheckFuncs into diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 8d02d3c62..7581892dc 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/hashicorp/go-version" + tfjson "github.com/hashicorp/terraform-json" "github.com/google/go-cmp/cmp" @@ -24,32 +25,15 @@ import ( "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func requirePlannableImport(t testing.T, versionUnderTest version.Version) error { - t.Helper() - - if 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`) - } - - return nil -} - func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest.Helper, wd *plugintest.WorkingDir, step TestStep, cfgRaw string, providers *providerFactories, stepNumber int) error { t.Helper() - // Currently import modes `ImportBlockWithId` and `ImportBlockWithResourceIdentity` cannot support config file or directory - // since these modes append the import block to the configuration automatically - if step.ImportStateKind != ImportCommandWithId { - if step.ConfigFile != nil || step.ConfigDirectory != nil { - t.Fatalf("ImportStateKind %q is not supported for config file or directory", step.ImportStateKind) - } - } - - if step.ImportStateKind != ImportCommandWithId { + // step.ImportStateKind implicitly defaults to the zero-value (ImportCommandWithID) for backward compatibility + kind := step.ImportStateKind + 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 } @@ -67,7 +51,8 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest testStepConfig := teststep.Configuration(configRequest) - if step.ResourceName == "" { + resourceName := step.ResourceName + if resourceName == "" { t.Fatal("ResourceName is required for an import state test") } @@ -110,7 +95,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest default: logging.HelperResourceTrace(ctx, "Using resource identifier for import identifier") - resource, err := testResource(step, state) + resource, err := testResource(resourceName, state) if err != nil { t.Fatal(err) } @@ -125,6 +110,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest logging.HelperResourceTrace(ctx, fmt.Sprintf("Using import identifier: %s", importId)) + // Append to previous step config unless using ConfigFile or ConfigDirectory if testStepConfig == nil || step.Config != "" { importConfig := step.Config if importConfig == "" { @@ -132,19 +118,8 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest importConfig = cfgRaw } - // Update the test config dependent on the kind of import test being performed - switch step.ImportStateKind { - case ImportBlockWithResourceIdentity: - t.Fatalf("TODO implement me") - case ImportBlockWithId: - importConfig += fmt.Sprintf(` - import { - to = %s - id = %q - } - `, step.ResourceName, importId) - default: - // Not an import block test so nothing to do here + if kind.plannable() { + importConfig = appendImportBlock(importConfig, resourceName, importId) } confRequest := teststep.PrepareConfigurationRequest{ @@ -170,7 +145,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest importWd = wd } else { importWd = helper.RequireNewWorkingDir(ctx, t, "") - defer importWd.Close() + defer importWd.Close() //nolint:errcheck } err = importWd.SetConfig(ctx, testStepConfig, step.ConfigVariables) @@ -189,7 +164,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest } var plan *tfjson.Plan - if step.ImportStateKind == ImportBlockWithResourceIdentity || step.ImportStateKind == ImportBlockWithId { + if kind.plannable() { var opts []tfexec.PlanOption err = runProviderCommand(ctx, t, func() error { @@ -214,7 +189,7 @@ 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 != step.ResourceName { + if rc.Address != resourceName { // we're only interested in the changes for the resource being imported continue } @@ -232,7 +207,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest return fmt.Errorf("retrieving formatted plan output: %w", err) } - return fmt.Errorf("importing resource %s should be a no-op, but got action %s with plan \\nstdout:\\n\\n%s", rc.Address, action, stdout) + 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) } } } @@ -244,11 +219,9 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest if err := runPlanChecks(ctx, t, plan, step.ImportPlanChecks.PreApply); err != nil { return err } - } else { err = runProviderCommand(ctx, t, func() error { - logging.HelperResourceDebug(ctx, "Run terraform import") - return importWd.Import(ctx, step.ResourceName, importId) + return importWd.Import(ctx, resourceName, importId) }, importWd, providers) if err != nil { return err @@ -272,29 +245,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest // Go through the imported state and verify if step.ImportStateCheck != nil { logging.HelperResourceTrace(ctx, "Using TestStep ImportStateCheck") - - var states []*terraform.InstanceState - for address, r := range importState.RootModule().Resources { - if strings.HasPrefix(address, "data.") { - continue - } - - if r.Primary == nil { - continue - } - - is := r.Primary.DeepCopy() //nolint:staticcheck // legacy usage - is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type - states = append(states, is) - } - - logging.HelperResourceDebug(ctx, "Calling TestStep ImportStateCheck") - - if err := step.ImportStateCheck(states); err != nil { - t.Fatal(err) - } - - logging.HelperResourceDebug(ctx, "Called TestStep ImportStateCheck") + runImportStateCheckFunction(ctx, t, importState, step) } // Verify that all the states match @@ -437,3 +388,53 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest return nil } + +func appendImportBlock(config string, resourceName string, importID string) string { + return config + fmt.Sprintf(``+"\n"+ + `import {`+"\n"+ + ` to = %s`+"\n"+ + ` id = %q`+"\n"+ + `}`, + resourceName, importID) +} + +func requirePlannableImport(t testing.T, versionUnderTest version.Version) error { + t.Helper() + + if 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`) + } + + return nil +} + +func runImportStateCheckFunction(ctx context.Context, t testing.T, importState *terraform.State, step TestStep) { + t.Helper() + + var states []*terraform.InstanceState + for address, r := range importState.RootModule().Resources { + if strings.HasPrefix(address, "data.") { + continue + } + + if r.Primary == nil { + continue + } + + is := r.Primary.DeepCopy() //nolint:staticcheck // legacy usage + is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type + states = append(states, is) + } + + logging.HelperResourceTrace(ctx, "Calling TestStep ImportStateCheck") + + if err := step.ImportStateCheck(states); err != nil { + t.Fatal(err) + } + + logging.HelperResourceTrace(ctx, "Called TestStep ImportStateCheck") +}