helper/resource: add ImportBlockWithResourceIdentity kind

[bbasata]

To merge after #476. This pull request adds resource identity support to ImportState test steps. For example:

terraform-plugin-testing/helper/resource/importstate/import_block_with_resource_identity_test.go

Lines 32 to 48 in c2a5b46

	Steps: []r.TestStep{
	{
	Config: `
	resource "examplecloud_container" "test" {
	location = "westeurope"
	name = "somevalue"
	}`,
	},
	{
	RefreshState: true,
	},
	{
	ResourceName: "examplecloud_container.test",
	ImportState: true,
	ImportStateKind: r.ImportBlockWithResourceIdentity,
	},
	},

In this example, the test author writes in-line config in Step 1. In Step 3 with ImportBlockWithResourceIdentity, the test appends an import block. The test constructs this import block using resource identity values from the Step 1 state:

terraform-plugin-testing/helper/resource/testing_new_import_state.go

Lines 118 to 126 in c2a5b46

	if kind.plannable() && kind.resourceIdentity() {
	priorIdentityValues = identityValuesFromState(stateJSON, resourceName)
	if len(priorIdentityValues) == 0 {
	return fmt.Errorf("importing resource %s: expected prior state to have resource identity values, got none", resourceName)
	}
	importConfig = appendImportBlockWithIdentity(importConfig, resourceName, priorIdentityValues)
	} else if kind.plannable() {
	importConfig = appendImportBlock(importConfig, resourceName, importId)
	}

The test generates a plan and asserts that the plan is a no-op import of a single resource:

terraform-plugin-testing/helper/resource/testing_new_import_state.go

Lines 221 to 227 in c2a5b46

	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))
	}

Finally, the test compares the Step 3 identity values with the identity values in Step 1 state. 💭 I want to take another look to consider whether this is necessary or duplicative of the "no-op import" assertion. 💭

At the time of writing this PR, I observed that a generated plan does not include resource identity values. And I reasoned that it's safe to run apply by this point -- the test applies a no-op import plan to a "sandboxed" statefile. Cross-ref #476 "Error on use of ImportStatePersist with plannable import"

terraform-plugin-testing/helper/resource/testing_new_import_state.go

Lines 233 to 250 in c2a5b46

	{
	if kind.resourceIdentity() {
	err := runProviderCommandApply(ctx, t, wd, providers)
	if err != nil {
	return fmt.Errorf("applying plan with import config: %s", err)
	}
	newStateJSON, err := runProviderCommandGetStateJSON(ctx, t, wd, providers)
	if err != nil {
	return fmt.Errorf("getting state after applying plan with import config: %s", err)
	}
	newIdentityValues := identityValuesFromState(newStateJSON, resourceName)
	if !cmp.Equal(priorIdentityValues, newIdentityValues) {
	return fmt.Errorf("importing resource %s: expected identity values %v, got %v", resourceName, priorIdentityValues, newIdentityValues)
	}
	}
	}

[austinvalle]

The code is looking great! I do have some specific questions and suggestions around the version check

[austinvalle]

I feel like I'm more leaning towards not applying and just waiting for TF core to give us the data during plan for comparison (it sounds like it should be included)

At least how it is ATM, core is validating that the data doesn't change during apply, and if they remove that logic, the SDKs should probably add it.

[austinvalle]

Mostly because we can't fully trust providers to apply what they plan (since the testing framework also supports SDKv2 😆 ), so if we can avoid it, I think we should

[bbasata]

Sounds good to me. Since we have a working implementation in this PR, it should be no trouble to switch to planned values & expect tests to remain green.

[austinvalle]

We talked about this offline and we're good with keeping apply with the caveat that we'll remove it before full release

[bbasata]

💭 also need to handle lists of numbers and lists of booleans

[bbasata]

or not? I added list of numbers and lists of booleans to the test resource schema and ... still green 🟢.

[austinvalle]

LGTM, great work on this @bbasata!

[austinvalle]

🚀