* WIP: dynamic expand
* WIP: add variable and local support
* WIP: outputs
* WIP: Add referencer
* String representation, fixing tests it impacts
* Fixes TestContext2Apply_outputOrphanModule
* Fix TestContext2Apply_plannedDestroyInterpolatedCount
* Update DestroyOutputTransformer and associated types to reflect PlannableOutputs
* Remove comment about locals
* Remove module count enablement
* Removes allowing count for modules, and reverts the test,
while adding a Skip()'d test that works when you re-enable
the config
* update TargetDownstream signature to match master
* remove unnecessary method
Co-authored-by: James Bardin <j.bardin@gmail.com>
During destroy, the for expression may be unknown and evaluation will
fail. Destroy provisioners however can only reference the key value,
which is known in the address.
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.
configs.Module is accessible.
Continuing the work of removing all calls to addrs.NewLegacyProvider,
this commit uses configs.Module.ProviderForLocalConfig wherever the
caller has access to that Module.
EvalContext.InitProvider no longer needs the redundant typ String
terraform.contextComponentFactory refactored to take an addrs.Provider
instead of a string.
Added configs.Module.ProviderForLocalProviderConfig which allows
terraform.ProviderTransformer to get the provider FQN from the module,
instead of assuming NewLegacyProvider.
We're not far enough along yet to be able to actually use the
RepetitionData instances provided by the instances package, but having
these types be considered identical will help us to gradually migrate over
as we prepare the rest of Terraform to properly populate the Expander.
This is a minimal integration of instances.Expander used just for resource
count and for_each, for now just forcing modules to always be singletons
because the rest of Terraform Core isn't ready to deal with expanding
module calls yet.
This doesn't integrate super cleanly yet because we still have some
cleanup work to do in the design of the plan walk, to make it explicit
that the nodes in the plan graph represent static configuration objects
rather than expanded instances, including for modules. To make this work
in the meantime, there is some shimming between addrs.Module and
addrs.ModuleInstance to correct for the discontinuities that result from
the fact that Terraform currently assumes that modules are always
singletons.
This is not used yet, but in future commits will be used as a
"blackboard" to centrally aggregate the information pertaining to
expansion of resources and modules (using "count" or "for_each") to help
ensure consistent treatment of the expansion process during a graph walk.
In practice this only really makes sense for the plan walk, because the
apply walk doesn't do any dynamic expansion.
This package aims to encapsulate the module/resource repetition problem
so that Terraform Core's graph node DynamicExpand implementations can be
simpler.
This is also a building block on the path towards module repetition, by
modelling the recursive expansion of modules and their contents. This will
allow the Terraform Core plan graph to have one node per configuration
construct, each of which will DynamicExpand into as many sub-nodes as
necessary to cover all of the recursive module instantiations.
For the moment this is just dead code, because Terraform Core isn't yet
updated to use it.
* fix outdated syntax in comments
* test for non-strings in ParseAbsProviderConfig
* ProviderConfigDefault and ProviderConfigAliased now take Providers
instead of strings
We no longer need special cases for most things during a full destroy,
so remove those from the graph transformations.
The only remaining cases are:
- remove the root outputs, so destroy ends up with a clean state
- reverse the target deps when targeting a destroy.
Remove all the destroy provisioner tests that are testing what is no
longer allowed.
Add missing state dependencies to remaining tests that require it.
The AttachStateTransformer was never run in the destroy plan. This means
that resource without configuration that used a non-default provider
would not be connected to the correct provider for the plan.
The test that was attempting to catch this only worked because the
temporary graph used in the DestroyEdgeTransformer would add the state
and detect some issues.
Start by removing the DestroyEdge type altogether. This is only used to
detect the natural edge between a resource's create and destroy nodes,
but that's not necessary for any transformations. The custom edge type
also interferes with normal graph manipulations, because you can't
delete an arbitrary edge without knowing the type, so deletion of the
edge based only on the endpoints is often done incorrectly. The dag
package itself does this incorrectly in TransitiveReduction, which
always assumes the BasicEdge type.
Now that inter-resource destroy dependencies are already connected in the
DestroyEdgeTransformer (from the stored deps in state), there's no need
to search out all dependant resources in the CBD transformation, as they
should all be connected. This makes the CBD transformation rule quite
simple: reverse any edges from create nodes.
This special edge type is no longer used. While we still have the option
of encoding more meaning into the edged themselves, having one special
edge type used only in one specific case was easily overlooked, as
dag.BasicEdge is assumed in all other cases.
The requires destroy dependencies are what is stored in the state, and
have already been connected. There is no need to search further, since
new nodes from the config may interfere with the original destroy
ordering.
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.
There was an order-of-operations bug where the import graph builder was
validating that the provider did not have any resource references before
references were actually being attached. This PR fixes the order of
operations and adds a test (in the command package).
Fixes#22804
A bug in ConfigTreeDependencies, where a pointer was being updated
instead of the map value, meant that only the first provider config
version constraing to be processes was being stored. This fixes that
bug, so now the returned moduledeps.Providers could have multiple
version constraints.
The responsibility for resolving provider version selection continues to
lie in the command package's ProviderResolver (under plugins.go).
configs.Module.ProviderRequirements
This is close to a no-op - we aren't accepting the provider source
attribute yet, so the only entires in the ProviderRequirements will
already be legacy provider addrs.
This PR also removes the unused `uid` field from ResourceProvider and
ResourceProvisioner. It's unused now and even less likely to be useful
now that we have a specific addrs.Provider type.
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.
The existing "type" argument allows specifying a type constraint that
allows for some basic validation, but often there are more constraints on
a variable value than just its type.
This new feature (requiring an experiment opt-in for now, while we refine
it) allows specifying arbitrary validation rules for any variable which
can then cause custom error messages to be returned when a caller provides
an inappropriate value.
variable "example" {
validation {
condition = var.example != "nope"
error_message = "Example value must not be \"nope\"."
}
}
The core parts of this are designed to do as little new work as possible
when no validations are specified, and thus the main new checking codepath
here can therefore only run when the experiment is enabled in order to
permit having validations.
While the NodeDestroyResource type should not be a
GraphNodeProviderConsumer, we're going to avoid uncovering more hidden
behavior by explicitly skipping provider creation and connections in the
provider transformers.
This should be removed when more in-depth testing can be done during a
major release cycle.
The change in #23696 removed the NodeAbstractResource methods from the
NodeDestroyResource type, in order to prevent other resource behaviors,
like requesting a provider.
While this node type is not directly referenced, it was implicitly
ordered against the module cleanup by virtue of being a resource node.
Since there's no good entry point to test this ordering at the moment,
This is a "should never happen" case, but we have reports of it actually
happening. In order to try to collect a bit more data about what's going
on here, we're changing what was previously a hard panic into a normal
error message that can include the address of the instance we were working
on and the action we were trying to do to it at the time.
The hope is to narrow down what situations can trigger this in order to
find a reliable reproduction case in order to debug further. This also
means that for those who _do_ encounter this problem in the meantime
Terraform will have a chance to shut down cleanly and therefore be more
likely to be able to recover on a subsequent plan/apply cycle.
Further investigation of this will follow once we see a report or two of
this updated error message.
