-
Notifications
You must be signed in to change notification settings - Fork 9.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jsonconfig: fix keys for default providers #30525
Conversation
Fixes provider config keys to reflect implicit provider inheritance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much for submitting this!
The general shape of this makes sense to me, but I think there's a problem which prevents it from being correct. In the findSourceProviderKey
function, we traverse up the module hierarchy looking for a provider configuration, matching by local name. Terraform's logic for doing this (in ProviderTransformer
and in (AbsProviderConfig) Inherited
) uses the fully-qualified provider address.
In practice, this means that this branch does not map configuration correctly in the (obscure) case where two modules use the same local name to refer to different providers.
I verified this locally with a root module using hashicorp/consul
with an explicit provider config, calling a child module which is using a requirements block with alisdair/consul
. Terraform behaviour was such that the child module's resources had no provider configuration, using only defaults, but the terraform show -json
output had the provider config key pointing at the root module's provider configuration for hashicorp/consul
.
At the moment I don't have any direct suggestions for how to fix this. I had hoped to be able to unify the provider resolution logic, such that we can determine exactly which provider configuration is used for a given resource using only the configuration (not the graph), but that seems like a much larger task than I'd hoped.
If you have any idea for an isolated change to the jsonconfig
package which would allow it to operate correctly in this case, that would be excellent. I'll continue to think about this as well and update you if I have any ideas. Please let me know if anything I've said here is unclear!
@alisdair Thank you for the review and detailed exaplanation! I was able to reproduce the case you mentioned, and added an example project here. This project uses I'll take some time to think about this issue as well. I guess we would need to somehow...
|
@alisdair I've just pushed two additional commits. Please let me know if you have any comments/feedback! Fix for local name conflictsThe basic idea is to check if the fully-qualified provider address matches with the parent one during traversing up the module call tree. This commit consists of the following changes.
Fix for existing test casesApart from the case above, we needed to catch default providers implicitly created in the root module. this commit fixes the logic and affected test cases as well.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great, thank you! 🙌 I really appreciate you breaking out the latter changes into two logical commits, that made it much easier to understand.
Thanks again for working on this!
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes provider config keys to reflect implicit provider inheritance/creation.
Issue 1. Implicit provider inheritance in child modules
The issue occurs when a parent module calls a child module without passing any providers. A child module implictly inherit default providers from the parent. From the implementation perspective, there could be three cases.
required_providers
block for the implicitly inherited provider.required_providers
andprovider
blocks for the implicitly inherited provider.required_providers
block and it contains a provider with a local name which exists in the parent module, but those providers have different fully-qualified provider addresses. Child module won't inherit this mis-matching provider, then create a separate default provider.Issue 2. Implicit provider creation in the root module
The issue occurs when a resource is defined without corresponding
provider
orrequired_providers
block. Terraform implicitly creates a default provider in this case if no provider inheritance occurs, but currentlyterraform show -json
does not produce aprovider_config
entry for these implicitly created providers.Changes in this PR
This PR contains following changes.
provider_config
entries for implicitly created providers (Issue 2).findSourceProviderKey()
to reuse the same logic.Fixes #30513.