helper/resource: Support Expected/Unexpected Diff Changes in TestStep

[bflad]

In the resource acceptance testing framework, we run into cases where we need to test the update behaviors of a resource, however the framework does not have a method to ensure the resource update is not actually a create/destroy (forces new resource), which may or may not be expected.

Expected Behavior

When we have acceptance testing that should only be an update, that it performs an update, not create/destroy.

Actual Behavior

Silently always passes as this is not supported at the moment. We need to manually check for create/destroy actions on update in the acceptance test debug logs to ensure we are not recreating resources.

Steps to Reproduce

e.g. in the AWS provider repository:
make testacc TEST=./aws TESTARGS='-run=TestAccAwsXXX_MyUpdateTest'

Additional Context

I'm linking to the latest reference of this issue in the AWS provider, but I have assuredly run into this myself in the past.

References

• resource/aws_dynamodb_table: (actually) fix disabling stream terraform-provider-aws#3224

Implementation Proposal

(Sorry if this looks incorrect, its my first time diving into the diff code 😄 )

Option(s) are available on the resource.TestStep struct for either expected or unexpected diff actions. A few competing ideas:

• ExpectedDiffChanges []string for testing all resources in the TestStep, e.g. terraform.DiffUpdate
• ExpectedDiffChanges map[string]string for targeted diff behavior at the resource layer, e.g. "resource.name": terraform.DiffUpdate

I think I lean towards the latter since it more succinctly captures what you are concerned about and could ignore other resources that are undefined in the map.

These will in turn add logic inside a test step to ensure the Terraform diff for the/all resource match, or fails the test step.

for k, v := range step.ExpectedDiffActions {
  if p.Diff == nil || p.Diff.Empty() {
    return state, fmt.Errorf("Expected a non-empty plan, but got an empty plan!")
  }
  instanceDiff, ok := p.Diff.RootModule().Resources[k]
  if !ok {
    return state, fmt.Errorf("Expected a non-empty diff for %s, but got an empty diff!", k)
  }
  if instanceDiff.ChangeType() != v {
    // It doesn't look like we provide a mapping for DiffChangeType to something friendly
    // For plans the human readable operation is buried in ModuleDiff.String()
    // So this just would notify the tester who would need to check out the debug logs
    return state, fmt.Errorf("Unexpected diff for %s!", k)
  }
}

[pdecat]

I was exploring how to implement a solution for hashicorp/terraform-plugin-testing#61, then found this issue, and they look very similar.

As already stated in both issues, this would be much needed to detect unexpected recreations of resources, in my case to address issues like hashicorp/terraform-provider-kubernetes#769 (comment)

I too like your ExpectedDiffChanges map[string]string option better for targeted diff behavior at the resource layer.

[paddycarver]

Hey @bflad, as @pdecat mentioned, I think this is a duplicate of hashicorp/terraform-plugin-testing#61. I'm going to close it out to try and consolidate tracking on this, but definitely subscribe to hashicorp/terraform-plugin-testing#61 for updates. If you think these issues are distinct, let me know in a comment to this issue and we can reopen it while we investigate.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.