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/importstate/import_block_in_config_directory_test.go b/helper/resource/importstate/import_block_in_config_directory_test.go index 2cb9ab6e1..017477589 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,11 @@ 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, + ImportPlanVerify: 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 index 8b983e310..c68833746 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,11 @@ 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, + ImportPlanVerify: true, + ConfigFile: config.StaticFile(`testdata/2/examplecloud_container_import.tf`), }, }, }) diff --git a/helper/resource/importstate/import_block_verify_plan_test.go b/helper/resource/importstate/import_block_verify_plan_test.go new file mode 100644 index 000000000..3568c2f57 --- /dev/null +++ b/helper/resource/importstate/import_block_verify_plan_test.go @@ -0,0 +1,48 @@ +// 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/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_VerifyPlan(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{ + { + Config: ` + resource "examplecloud_container" "test" { + location = "westeurope" + name = "somevalue" + }`, + }, + { + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ImportPlanVerify: true, + }, + }, + }) +} diff --git a/helper/resource/importstate/import_block_with_id_test.go b/helper/resource/importstate/import_block_with_id_test.go index 0f65af713..f1678922b 100644 --- a/helper/resource/importstate/import_block_with_id_test.go +++ b/helper/resource/importstate/import_block_with_id_test.go @@ -44,10 +44,10 @@ 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, + ImportPlanVerify: true, }, }, }) @@ -81,11 +81,11 @@ 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, + ImportPlanVerify: true, + ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`), }, }, }) @@ -120,11 +120,11 @@ func TestTest_TestStep_ImportBlockId_FailWhenPlannableImportIsNotSupported(t *te location = "eastus" name = "somevalue" }`, - ResourceName: "examplecloud_container.test", - ImportState: true, - ImportStateKind: r.ImportBlockWithID, - ImportStateVerify: true, - ExpectError: regexp.MustCompile(`Terraform 1.5.0`), + ResourceName: "examplecloud_container.test", + ImportState: true, + ImportStateKind: r.ImportBlockWithID, + ImportPlanVerify: true, + ExpectError: regexp.MustCompile(`Terraform 1.5.0`), }, }, }) @@ -200,6 +200,11 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes I also need to omit the `password` in the import config, otherwise the value in the config is used when importing the with an import block and the test ends up passing regardless of whether `ImportStateVerifyIgnore` has been specified or not */ + + // In prerelease, we are choosing that ImportBlockWithID will not perform an apply, so it will not produce a new state, + // and there is no new state for ImportStateVerify to do anything meaningful with. + t.Skip() + t.Parallel() r.UnitTest(t, r.TestCase{ @@ -301,6 +306,10 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes } func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore(t *testing.T) { + // In prerelease, we are choosing that ImportBlockWithID will not perform an apply, so it will not produce a new state, + // and there is no new state for ImportStateVerify to do anything meaningful with. + t.Skip() + 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..48187bba9 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 Test_ImportCommmandWithId_SkipDataSourceState(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 Test_ImportCommandWithId_ImportStateVerify(t *testing.T) { t.Parallel() r.UnitTest(t, r.TestCase{ diff --git a/helper/resource/testing.go b/helper/resource/testing.go index cda787033..ac9a2c24a 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -700,6 +700,11 @@ type TestStep struct { // [plancheck]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck ImportPlanChecks ImportPlanChecks + // ImportPlanVerify checks that the planned resource state, created via a plannable import + // step (ImportBlockWithID or ImportBlockWithResourceIdentity), exactly matches + // the resource state of the previous test step. + ImportPlanVerify bool + // ImportStateVerify, if true, will also check that the state values // that are finally put into the state after import match for all the // IDs returned by the Import. Note that this checks for strict equality diff --git a/helper/resource/testing_new.go b/helper/resource/testing_new.go index 1a84188b8..8d8d5ffd6 100644 --- a/helper/resource/testing_new.go +++ b/helper/resource/testing_new.go @@ -68,7 +68,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest var statePreDestroy *terraform.State var err error err = runProviderCommand(ctx, t, func() error { - statePreDestroy, err = getState(ctx, t, wd) + _, statePreDestroy, err = getState(ctx, t, wd) if err != nil { return err } @@ -447,18 +447,18 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest } } -func getState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir) (*terraform.State, error) { +func getState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir) (*tfjson.State, *terraform.State, error) { t.Helper() jsonState, err := wd.State(ctx) if err != nil { - return nil, err + return nil, nil, err } state, err := shimStateFromJson(jsonState) if err != nil { t.Fatal(err) } - return state, nil + return jsonState, state, nil } func stateIsEmpty(state *terraform.State) bool { @@ -573,7 +573,7 @@ func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest. if err != nil { t.Fatalf("Error running terraform refresh: %s", err) } - state, err = getState(ctx, t, wd) + _, state, err = getState(ctx, t, wd) if err != nil { return err } diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 1456f7fba..04f8882d9 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -159,7 +159,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint } err = runProviderCommand(ctx, t, func() error { - stateBeforeApplication, err = getState(ctx, t, wd) + _, stateBeforeApplication, err = getState(ctx, t, wd) if err != nil { return err } @@ -200,7 +200,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint var state *terraform.State err := runProviderCommand(ctx, t, func() error { - state, err = getState(ctx, t, wd) + _, state, err = getState(ctx, t, wd) if err != nil { return err } @@ -384,7 +384,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint var state *terraform.State err = runProviderCommand(ctx, t, func() error { - state, err = getState(ctx, t, wd) + _, state, err = getState(ctx, t, wd) if err != nil { return err } diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 7581892dc..dbfe0d0b1 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -58,10 +58,11 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest // get state from check sequence var state *terraform.State + var stateJSON *tfjson.State var err error err = runProviderCommand(ctx, t, func() error { - state, err = getState(ctx, t, wd) + stateJSON, state, err = getState(ctx, t, wd) if err != nil { return err } @@ -72,7 +73,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest } // Determine the ID to import - var importId string + var importId string //nolint:revive switch { case step.ImportStateIdFunc != nil: logging.HelperResourceTrace(ctx, "Using TestStep ImportStateIdFunc for import identifier") @@ -185,37 +186,20 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest return err } - if plan.ResourceChanges != nil { + if len(plan.ResourceChanges) > 0 { 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) - } - } + if err := requireNoopResourceAction(ctx, t, plan, resourceName, importWd, providers); err != nil { + return err + } + + if step.ImportPlanVerify { + if err := teststep.VerifyImportPlan(plan, stateJSON); err != nil { + return err } } } - // TODO compare plan to state from previous step - if err := runPlanChecks(ctx, t, plan, step.ImportPlanChecks.PreApply); err != nil { return err } @@ -230,7 +214,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest var importState *terraform.State err = runProviderCommand(ctx, t, func() error { - importState, err = getState(ctx, t, importWd) + _, importState, err = getState(ctx, t, importWd) if err != nil { return err } @@ -275,6 +259,10 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest identifierAttribute = "id" } + if len(newResources) == 0 { + return fmt.Errorf("ImportStateVerify: no new resources imported") + } + for _, r := range newResources { rIdentifier, ok := r.Primary.Attributes[identifierAttribute] @@ -389,6 +377,46 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest return nil } +func requireNoopResourceAction(ctx context.Context, t testing.T, plan *tfjson.Plan, resourceName string, importWd *plugintest.WorkingDir, providers *providerFactories) error { + t.Helper() + + rc := findResourceChangeInPlan(t, plan, resourceName) + if rc == nil || rc.Change == nil || rc.Change.Actions == nil { + // does this matter? + return 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 != tfjson.ActionNoop { + 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) + } + } + + return nil +} + +func findResourceChangeInPlan(t testing.T, plan *tfjson.Plan, resourceName string) *tfjson.ResourceChange { + t.Helper() + + for _, rc := range plan.ResourceChanges { + if rc.Address == resourceName { + return rc + } + } + return nil +} + func appendImportBlock(config string, resourceName string, importID string) string { return config + fmt.Sprintf(``+"\n"+ `import {`+"\n"+ diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index 5c0e38758..4a3b362c1 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -22,7 +22,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo var err error // Explicitly ensure prior state exists before refresh. err = runProviderCommand(ctx, t, func() error { - _, err = getState(ctx, t, wd) + _, _, err = getState(ctx, t, wd) if err != nil { return err } @@ -41,7 +41,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo var refreshState *terraform.State err = runProviderCommand(ctx, t, func() error { - refreshState, err = getState(ctx, t, wd) + _, refreshState, err = getState(ctx, t, wd) if err != nil { return err } diff --git a/internal/teststep/verify_import_plan.go b/internal/teststep/verify_import_plan.go new file mode 100644 index 000000000..77a30a3df --- /dev/null +++ b/internal/teststep/verify_import_plan.go @@ -0,0 +1,73 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package teststep + +import ( + "fmt" + "strings" + + tfjson "github.com/hashicorp/terraform-json" +) + +// VerifyImportPlan compares a Terraform plan against a known good state +func VerifyImportPlan(plan *tfjson.Plan, state *tfjson.State) error { + if state == nil { + return fmt.Errorf("state is nil") + } + if plan == nil { + return fmt.Errorf("plan is nil") + } + + // TODO: if state.Values is nil ... + + resourcesInState := make(map[string]*tfjson.StateResource) + for _, resource := range state.Values.RootModule.Resources { + if !strings.HasPrefix(resource.Address, "data.") { + resourcesInState[resource.Address] = resource + } + } + for _, rc := range plan.ResourceChanges { + if rc.Change == nil || rc.Change.Actions == nil { + // does this matter? + continue + } + + if !rc.Change.Actions.NoOp() { + return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action", rc.Address, rc.Change.Actions) + } + + if rc.Change.Importing == nil { + return fmt.Errorf("importing resource %s: expected importing to be true", rc.Address) + } + } + + for _, rc := range plan.ResourceChanges { + after, ok := rc.Change.After.(map[string]interface{}) + if !ok { + panic(fmt.Sprintf("unexpected type %T", rc.Change.After)) + } + + for k, v := range after { + vs, ok := v.(string) + if !ok { + panic(fmt.Sprintf("unexpected type %T", v)) + } + + resourceInState := resourcesInState[rc.Address] + if resourceInState == nil { + // does this matter? + return fmt.Errorf("importing resource %s: expected resource %s to exist in known state", rc.Address, rc.Change.Importing.ID) + } + + attr, ok := resourceInState.AttributeValues[k] + if !ok { + return fmt.Errorf("importing resource %s: expected %s in known state to exist", rc.Address, k) + } + if attr != vs { + return fmt.Errorf("importing resource %s: expected %s in known state to be %q, got %q", rc.Address, k, attr, vs) + } + } + } + return nil +} diff --git a/internal/teststep/verify_import_plan_test.go b/internal/teststep/verify_import_plan_test.go new file mode 100644 index 000000000..9ce71133c --- /dev/null +++ b/internal/teststep/verify_import_plan_test.go @@ -0,0 +1,99 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package teststep_test + +import ( + "testing" + + tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-testing/internal/teststep" +) + +func TestVerifyImportPlan(t *testing.T) { + t.Parallel() + + resource := &tfjson.StateResource{ + Address: "example_resource.instance-1", + Type: "example_resource", + Name: "instance-1", + AttributeValues: map[string]interface{}{ + "attr1": "value1", + }, + } + + state := new(tfjson.State) + state.Values = &tfjson.StateValues{ + RootModule: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{resource}, + }, + } + + plan := new(tfjson.Plan) + plan.ResourceChanges = []*tfjson.ResourceChange{ + { + Address: "example_resource.instance-1", + Change: &tfjson.Change{ + Actions: []tfjson.Action{tfjson.ActionNoop}, + After: map[string]interface{}{ + "attr1": "value1", + }, + Before: map[string]interface{}{ + "attr1": "value1", + }, + Importing: &tfjson.Importing{ + ID: "instance-1", + }, + }, + }, + } + + if err := teststep.VerifyImportPlan(plan, state); err != nil { + t.Fatal(err) + } + +} + +func TestVerifyImportPlan_AttributeValueMismatch(t *testing.T) { + t.Parallel() + + resource := &tfjson.StateResource{ + Address: "example_resource.instance-1", + Type: "example_resource", + Name: "instance-1", + AttributeValues: map[string]interface{}{ + "attr1": "value1", + }, + } + + state := new(tfjson.State) + state.Values = &tfjson.StateValues{ + RootModule: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{resource}, + }, + } + + plan := new(tfjson.Plan) + plan.ResourceChanges = []*tfjson.ResourceChange{ + { + Address: "example_resource.instance-1", + Change: &tfjson.Change{ + Actions: []tfjson.Action{tfjson.ActionNoop}, + After: map[string]interface{}{ + "attr1": "value5", + }, + Before: map[string]interface{}{ + "attr1": "value5", + }, + Importing: &tfjson.Importing{ + ID: "instance-1", + }, + }, + }, + } + + if err := teststep.VerifyImportPlan(plan, state); err == nil { + t.Fatal("expected error, got nil") + } + +}