When parsing provider requirements we should check the type of the
source and version attributes rather than assuming that they are
strings. Otherwise an invalid attribute value will cause a panic.
In a recent PR, we changed the provider requirements code to permit
per-module requirements gathering, to enhance the provider command
output. This had an incorrect implementation of recursive requirements
gathering for the normal case, which resulted in only depth-1 modules
being inspected.
This commit fixes the broken recursion and adds a grandchild module to
the unit tests as test coverage. This also demanded fixing the
testNestedModuleConfigFromDir helper function to cope with nested
modules in test configs.
* Refactor provider validation into separate func & recurse
Refactors the validate provider functions into a separate function
that can recursively search above a module to check and see if
any parents of the module contain count/for_each configs to be
considered
The new provider installer code is using a new version constraint parser
because it produces better error messages than the one we were using
before. However, it has some cases where it returns errors that the old
parser (which was entirely regex-match-based) didn't catch.
In the long run we should consistently use the new parser everywhere, but
until then we'll avoid panicking then the two disagree, by returning
diagnostic messages instead of using MustParseVersionConstraints.
For now, we only hit these error cases if the user enters something that
the old parser allows but the new parser does not.
When initializing a configuration which refers to re-namespaced legacy
providers, we attempt to detect this and display a diagnostic message.
Previously this message would direct the user to run the 0.13upgrade
command, but without specifying in which directories.
This commit detects which modules are using the providers in question,
and for local modules displays a list of upgrade commands which specify
the source directories of these modules.
For remote modules, we display a separate list noting that they need to
be upgraded elsewhere, providing both the local module call name and the
module source address.
Providers can be required from multiple sources. The previous
implementation of the providers sub-command displayed only a flat list
of provider requirements, which made it difficult to see which modules
required each provider.
This commit reintroduces the tree display of provider requirements, and
adds a separate output block for providers required by existing state.
addProviderRequirements() was incorrectly using the map keys from the module
provider configs when looking up the provider FQN. The map keys include
alias, so this resulted in a panic. Update addProviderRequirements() to
use the provider's name (only) when looking up the FQN.
All of the feedback from the experiment described enhancements that can
potentially be added later without breaking changes, so this change simply
removes the experiment gate from the feature as originally implemented
with no changes to its functionality.
Further enhancements may follow in later releases, but the goal of this
change is just to ship the feature exactly as it was under the experiment.
Most of the changes here are cleaning up the experiment opt-ins from our
test cases. The most important parts are in configs/experiments.go and in
experiments/experiment.go .
* addrs: replace NewLegacyProvider with NewDefaultProvider in ParseProviderSourceString
ParseProviderSourceString was still defaulting to NewLegacyProvider when
encountering single-part strings. This has been fixed.
This commit also adds a new function, IsProviderPartNormalized, which
returns a bool indicating if the string given is the same as a
normalized version (as normalized by ParseProviderPart) or an error.
This is intended for use by the configs package when decoding provider
configurations.
* terraform: fix provider local names in tests
* configs: validate that all provider names are normalized
The addrs package normalizes all source strings, but not the local
names. This caused very odd behavior if for e.g. a provider local name
was capitalized in one place and not another. We considered enabling
case-sensitivity for provider local names, but decided that since this
was not something that worked in previous versions of terraform (and we
have yet to encounter any use cases for this feature) we could generate
an error if the provider local name is not normalized. This error also
provides instructions on how to fix it.
* configs: refactor decodeProviderRequirements to consistently not set an FQN when there are errors
Validate providers in expanding modules. Expanding modules cannot have provider configurations with non-empty configs, which includes having a version configured. If an empty or alias-only block is passed, the provider must be passed through the providers argument on the module call
This commit implements most of the intended functionality of the upgrade
command for rewriting configurations.
For a given module, it makes a list of all providers in use. Then it
attempts to detect the source address for providers without an explicit
source.
Once this step is complete, the tool rewrites the relevant configuration
files. This results in a single "required_providers" block for the
module, with a source for each provider.
Any providers for which the source cannot be detected (for example,
unofficial providers) will need a source to be defined by the user. The
tool writes an explanatory comment to the configuration to help with
this.
Previously, resources without explicit provider configuration (i.e. a
`provider =` attribute) would be assigned a default provider based upon
the resource type. For example, a resource `foo_bar` would be assigned
provider `hashicorp/foo`.
This behaviour did not work well with community or partner providers,
with sources configured in `terraform.required_providers` blocks. With
the following configuration:
terraform {
required_providers {
foo = {
source = "acme/foo"
}
}
}
resource foo_bar "a" { }
the resource would be configured with the `hashicorp/foo` provider.
This commit fixes this implied provider behaviour. First we look for a
provider with local name matching the resource type in the module's
required providers map. If one is found, this provider is assigned to
the resource. Otherwise, we still fall back to a default provider.
We now permit at most one `required_providers` block per module (except
for overrides). This prevents users (and Terraform) from struggling to
understand how to merge multiple `required_providers` configurations,
with `version` and `source` attributes split across multiple blocks.
Because only one `required_providers` block is permitted, there is no
need to concatenate version constraints and resolve them. This allows us
to simplify the structs used to represent provider requirements,
aligning more closely with other structs in this package.
This commit also fixes a semantic use-before-initialize bug, where
resources defined before a `required_providers` block would be unable to
use its source attribute. We achieve this by processing the module's
`required_providers` configuration (and overrides) before resources.
Overrides for `required_providers` work as before, replacing the entire
block per provider.
The providers command has been refactored to use the modern provider types and
ProviderRequirements() functions. This resulted in a breaking change to
the output: it no longer outputs the providers by module and no longer
prints `(inherited)` or `(from state)` to show why a provider is
included. We decided that at this time it was best to stick with the
existing functions and make this change, but if we get feedback from the
community we will revisit.
Additional tests to exercise providers in modules and providers from
state have been included.
This PR adds iteration through any provider configuration blocks in the
config in addProviderRequirements().
A stale comment (of mine!) would leave one expecting the
module.ProviderRequirements to include any requirements from provider
configs. The comment was inaccurate and has been updated.
With provider dependencies now appearing inside a nested block, it seems
likely that configuration examples showing dependencies out of context
will sometimes mislead users into thinking that required_providers is
toplevel.
To give better feedback in that situation, we'll produce a specialized
error in that case hinting the correct structure to the user.
This encapsulates the logic for selecting an implied FQN for an
unqualified type name, which could either come from a local name used in
a module without specifying an explicit source for it or from the prefix
of a resource type on a resource that doesn't explicitly set "provider".
This replaces the previous behavior of just directly calling
NewDefaultProvider everywhere so that we can use a different implication
for the local name "terraform", to refer to the built-in terraform
provider rather than the stale one that's on registry.terraform.io for
compatibility with other Terraform versions.
We've been using the models from the "moduledeps" package to represent our
provider dependencies everywhere since the idea of provider dependencies
was introduced in Terraform 0.10, but that model is not convenient to use
for any use-case other than the "terraform providers" command that needs
individual-module-level detail.
To make things easier for new codepaths working with the new-style
provider installer, here we introduce a new model type
getproviders.Requirements which is based on the type the new installer was
already taking as its input. We have new methods in the states, configs,
and earlyconfig packages to produce values of this type, and a helper
to merge Requirements together so we can combine config-derived and
state-derived requirements together during installation.
The advantage of this new model over the moduledeps one is that all of
recursive module walking is done up front and we produce a simple, flat
structure that is more convenient for the main use-cases of selecting
providers for installation and then finding providers in the local cache
to use them for other operations.
This new model is _not_ suitable for implementing "terraform providers"
because it does not retain module-specific requirement details. Therefore
we will likely keep using moduledeps for "terraform providers" for now,
and then possibly at a later time consider specializing the moduledeps
logic for only what "terraform providers" needs, because it seems to be
the only use-case that needs to retain that level of detail.
* terraform: large refactor to use Provider from configs.Resource
configs.Resource.ImpliedProvider() now returns a string; it is the
callers' responsibility to turn that into an addrs.Provider if needed.
GraphNodeProviderConsumer ProvidedBy() no longer returns nil (reverting
to earlier, pre-provider-fqn behavior): it will return either the
provider set in config, provider set in state, or the default provider.
This function can already produce suitable diagnostic messages which we'd
like to preserve, but it cannot produce source location information, and
so we'll amend the diagnostics to include that on the way out while
retaining all of the other values in the diagnostics.
* configs: parse provider source string during module merge
This was the smallest unit of work needed to start writing provider
source tests!
* Update configs/parser_test.go
Co-Authored-By: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Change ModuleInstance to Module in AbsProviderConfig, because providers
need to be handled before module expansion, and should not be used
defined inside an expanded module at all.
Renaming of the addrs type can happen later, when there's less work
in-flight around provider configuration.
* add Config to AttachSchemaTransformer for providerFqn lookup
* terraform: refactor ProvidedBy() to return nil when provider is not set
in config or state
Implement a new provider_meta block in the terraform block of modules, allowing provider-keyed metadata to be communicated from HCL to provider binaries.
Bundled in this change for minimal protocol version bumping is the addition of markdown support for attribute descriptions and the ability to indicate when an attribute is deprecated, so this information can be shown in the schema dump.
Co-authored-by: Paul Tyng <paul@paultyng.net>
The provider FQN is becoming our primary identifier for a provider, so
it's important that we are clear about the equality rules for these
addresses and what characters are valid within them.
We previously had a basic regex permitting ASCII letters and digits for
validation and no normalization at all. We need to do at least case
folding and UTF-8 normalization because these names will appear in file
and directory names in case-insensitive filesystems and in repository
names such as on GitHub.
Since we're already using DNS-style normalization and validation rules
for the hostname part, rather than defining an entirely new set of rules
here we'll just treat the provider namespace and type as if they were
single labels in a DNS name. Aside from some internal consistency, that
also works out nicely because systems like GitHub use organization and
repository names as part of hostnames (e.g. with GitHub Pages) and so
tend to apply comparable constraints themselves.
This introduces the possibility of names containing letters from alphabets
other than the latin alphabet, and for latin letters with diacritics.
That's consistent with our introduction of similar support for identifiers
in the language in Terraform 0.12, and is intended to be more friendly to
Terraform users throughout the world that might prefer to name their
products using a different alphabet. This is also a further justification
for using the DNS normalization rules: modern companies tend to choose
product names that make good domain names, and now such names will be
usable as Terraform provider names too.
Added configs.Module.ProviderForLocalProviderConfig which allows
terraform.ProviderTransformer to get the provider FQN from the module,
instead of assuming NewLegacyProvider.
a large refactor to addrs.AbsProviderConfig, embedding the addrs.Provider instead of a Type string. I've added and updated tests, added some Legacy functions to support older state formats and shims, and added a normalization step when reading v4 (current) state files (not the added tests under states/statefile/roundtrip which work with both current and legacy-style AbsProviderConfig strings).
The remaining 'fixme' and 'todo' comments are mostly going to be addressed in a subsequent PR and involve looking up a given local provider config's FQN. This is fine for now as we are only working with default assumption.
* configs: added map of ProviderLocalNames to configs.Module
We will need to lookup any user-supplied local names for a given FQN.
This PR adds a map of ProviderLocalNames to the Module, along with
adding tests for this and for decodeRequiredProvidersBlock.
This also introduces the appearance of support for a required_provider
"source" attribute, but ignores any user-supplied source and instead
continues to assume that addrs.NewLegacyProvider is the way to go.
This is a stepping-stone PR for the provider source project. In this PR
"legcay-stype" FQNs are created from the provider name string. Future
work involves encoding the FQN directly in the AbsProviderConfig and
removing the calls to addrs.NewLegacyProvider().
* Introduce "Local" terminology for non-absolute provider config addresses
In a future change AbsProviderConfig and LocalProviderConfig are going to
become two entirely distinct types, rather than Abs embedding Local as
written here. This naming change is in preparation for that subsequent
work, which will also include introducing a new "ProviderConfig" type
that is an interface that AbsProviderConfig and LocalProviderConfig both
implement.
This is intended to be largely just a naming change to get started, so
we can deal with all of the messy renaming. However, this did also require
a slight change in modeling where the Resource.DefaultProviderConfig
method has become Resource.DefaultProvider returning a Provider address
directly, because this method doesn't have enough information to construct
a true and accurate LocalProviderConfig -- it would need to refer to the
configuration to know what this module is calling the provider it has
selected.
In order to leave a trail to follow for subsequent work, all of the
changes here are intended to ensure that remaining work will become
obvious via compile-time errors when all of the following changes happen:
- The concept of "legacy" provider addresses is removed from the addrs
package, including removing addrs.NewLegacyProvider and
addrs.Provider.LegacyString.
- addrs.AbsProviderConfig stops having addrs.LocalProviderConfig embedded
in it and has an addrs.Provider and a string alias directly instead.
- The provider-schema-handling parts of Terraform core are updated to
work with addrs.Provider to identify providers, rather than legacy
strings.
In particular, there are still several codepaths here making legacy
provider address assumptions (in order to limit the scope of this change)
but I've made sure each one is doing something that relies on at least
one of the above changes not having been made yet.
* addrs: ProviderConfig interface
In a (very) few special situations in the main "terraform" package we need
to make runtime decisions about whether a provider config is absolute
or local.
We currently do that by exploiting the fact that AbsProviderConfig has
LocalProviderConfig nested inside of it and so in the local case we can
just ignore the wrapping AbsProviderConfig and use the embedded value.
In a future change we'll be moving away from that embedding and making
these two types distinct in order to represent that mapping between them
requires consulting a lookup table in the configuration, and so here we
introduce a new interface type ProviderConfig that can represent either
AbsProviderConfig or LocalProviderConfig decided dynamically at runtime.
This also includes the Config.ResolveAbsProviderAddr method that will
eventually be responsible for that local-to-absolute translation, so
that callers with access to the configuration can normalize to an
addrs.AbsProviderConfig given a non-nil addrs.ProviderConfig. That's
currently unused because existing callers are still relying on the
simplistic structural transform, but we'll switch them over in a later
commit.
* rename LocalType to LocalName
Co-authored-by: Kristin Laemmert <mildwonkey@users.noreply.github.com>
Renamed file.ProviderRequirements to file.RequiredProviders to match the
name of the block in the configuration. file.RequiredProviders contains
the contents of the file(s); module.ProviderRequirements contains the
parsed and merged provider requirements.
Extended decodeRequiredProvidersBlock to parse the new provider source
syntax (version only, it will ignore any other attributes).
Added some tests; swapped deep.Equal with cmp.Equal in the
terraform/module_dependencies_test.go because deep was not catching
incorrect constraints.
Alisdair McDiarmid