Conversation
Some of the tests in helper/resource have to shim between legacy and modern states. Terraform expects modern states to have the Provider set for each resource (and not be a legacy provider). This PR adds a wrapper around terraform.ShimLegacyState which iterates through the resources in the state (after the shim) and adds the provider FQN.
ghost
added
the
sdkv1
apparentlymart
left a comment
apparentlymart
left a comment
There was a problem hiding this comment.
LGTM! 🎉
Just want to note here for future reference that we made an intentional decision to duplicate a bit of functionality in the helper/resource package here, rather than modifying the main behavior to support it, because helper/resource is a legacy system in this codebase and so we think it's desirable to make it as self-contained as possible so that it can keep working as we evolve the rest of the codebase, and so that we can evolve the rest of the codebase without being forever held back by these legacy constraints. We hope to plan a real future for helper/resource in a later project, but it's out of scope for now.
alisdair
left a comment
alisdair
left a comment
There was a problem hiding this comment.
Looks good! 👍 I have one question inline.
helper/resource/state_shim.go
Outdated
| if state.HasResources() { | ||
| for _, module := range state.Modules { | ||
| for name, resource := range module.Resources { | ||
| module.Resources[name].ProviderConfig.Provider = addrs.NewDefaultProvider(resource.Addr.Resource.ImpliedProvider()) |
There was a problem hiding this comment.
Should this line use addrs.ImpliedProviderForUnqualifiedType? I imagine it's unlikely that we'll have test states with a builtin provider, so it's not really necessary, but it might be safer.
There was a problem hiding this comment.
Excellent point, thank you! Better have in there and not need it than have a test blow up unnecessarily.
* helper/resource: remove provider resolver test * repl tests passing * helper/resource: add some extra shimming to ShimLegacyState Some of the tests in helper/resource have to shim between legacy and modern states. Terraform expects modern states to have the Provider set for each resource (and not be a legacy provider). This PR adds a wrapper around terraform.ShimLegacyState which iterates through the resources in the state (after the shim) and adds the provider FQN.
* helper/resource: remove provider resolver test * repl tests passing * helper/resource: add some extra shimming to ShimLegacyState Some of the tests in helper/resource have to shim between legacy and modern states. Terraform expects modern states to have the Provider set for each resource (and not be a legacy provider). This PR adds a wrapper around terraform.ShimLegacyState which iterates through the resources in the state (after the shim) and adds the provider FQN.
|
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. |
ghost
locked and limited conversation to collaborators
3 participants
The last commit is the interesting one: