module provider inheritance #16379
module provider inheritance #16379
Conversation
|
could this theoretically fix #15520? |
|
Hi @vvondra, Unfortunately not. This still will configure the provider for each provider instance in the configuration. I am exploring some paths that might be able to reduce the number of virtual "provider instances" in the graph, but optimizing for lots of providers is still a ways off. |
Add the Version and Providers fields to the module config. Add ProviderConfig.Scope, which will be used to record the original path of a ProviderConfig for interpolation.
This implements provider inheritance during config loading, rather than during graph evaluation. At this point it's much simpler to find the desired configuration, and once all providers are declared, all the inheritance code in the graph can be removed. The inheritance is dome by simply copying the RawConfig from the parent ProviderConfig into the module. Since this happens before any evaluation, we record the original interpolation scope in the ProviderConfig so that it can be properly resolved later on.
Use the configured providers directly, rather than looking for inherited provider configuration during graph evaluation. First remove the provider config cache, and the associated SetProviderConfig and ParentProviderConfig methods on the eval context. Every provider must be configured, so there's no need to look for configuration from other provider instances. The config.ProviderConfig struct now has a Scope field which stores the proper path for the interpolation scope. To get this metadata to the interpolator, we add an EvalInterpolatProvider node which can carry the ProviderConfig, and an InterpolateProvider context method to carry the ProviderConfig.Scope into the InterplationScope. Some of the tests could be adjusted to account for the new inheritance behavior, and some were simply no longer valid and will be removed. The remaining tests have questions on how they should work in practice. This mostly concerns orphaned modules where there is no longer a way to obtain a provider. In some cases we may require that a minimal provider config be present to handle the destroy process, but we need further testing. All disabled code was commented out in this commit to record any additional comments. The following commit will be a cleanup pass.
Previously when looking up cached provider input, the Input was taken in its entirety, and only provider configuration fields that weren't in the saved input were added. This would cause providers in modules to use the entire configuration from parent modules, even if they themselves had entirely different configs. Note: this is only marginally beter than the old behavior. It may be slightly more correct, but stil can't account for the user's intent, and may be adding configured values from one provider into another. Change the PathCacheKey to just join the path on a non-path character (|), which makes for easier debugging.
While merging the cached Input configs in the correct order prevents overwriting existing config values, it doesn't prevent an earlier provider from inserting unwanted values into later provider configurations. Diff the key-values returned by Input with the pre-input config, and store only the "answers" that were added during the Input call. Always call Input, even if we already have some values, since a previously cached config may not be complete.
Though it's intended for "interpolation scope", Path is generally used for this elsewhere.
Importing into a module requires a provider config. Update the inheritance test to reflect the new import restrictions.
jbardin
force-pushed
the
jbardin/module-provider-inheritance
branch
from
2fc8fe6
to
97ce298
Compare
|
Orphaned modules will have to wait, merging this into a dev branch. |
|
I'm not sure what you're looking for exactly; does the documentation help answer your question? https://www.terraform.io/docs/modules/usage.html#providers-within-modules |
|
@jbardin Where do I find the issue to again support teardown of orphaned modules? |
|
@bukzor: Removing modules is working as intended. If the provider configuration is removed, there's no way to reliably destroy the resources in a module. If the provider configuration is passed into the module, then you can retain the configuration separate from the module source. |
|
I'm hearing that the issue doesn't exist. I've worked up a demo and explained the problems in this github repo. Should I file each of those gripes as an issue, or would you be willing to do triage? There's two misfeatures and one outright bug here, by my count. https://github.com/bukzor/terraform-gke-k8s-demo |
|
Hi @bukzor, I haven't gone through that repo in detail, but the first thing that stands out is that there are provider configurations within the module, which is what isn't going to work. We do have some known issues around providers, like #13018, and some new things implemented like #16835. Of course feel free to file any other issues you have that aren't covered. |
|
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
The primary goal of this PR is to define the new provider inheritance model, while hoisting the provider inheritance mechanisms into the config itself. The new providers field in the module block allows users to declare a provider configuration to apply within a module. Since this new inheritance mechanism is configuration based, it is much simpler to apply all the inheritance rules within the config, rather than searching for provider configurations during graph evaluation.
The only remnant of "inheritance" in core is now through the provider input cache. Though the cached Input result no longer force unintended inheritance, the fields added by a provider's Input call are added to the config of other providers by the same name. It might be advantageous to remove the input cache just as we've done with the config cache, but that would cause every Provider missing fields to prompt, which may be inconvenient for users relying on manual Inputs.
Due to the removal of inheritance in the terraform package, orphaned module resources can no longer be destroyed without a provider config. This is not optimal, but can be handled in a later PR to keep this more manageable.