This is the known case broken by the changes to allow resources pending
destruction to be evaluated from state. When a resource references
another that is create_before_destroy, and that resource is being scaled
in, the first resource will not be updated correctly.
Since we have to allow destroy nodes to be evaluated for providers
during a full destroy, this is adding a transformer to connect temporary
values to any destroy versions of their references when possible. The
ensures that the destroy happens before evaluation, even when there
isn't a full create-then-destroy set of instances.
The cases where the connection can't be made are when the temporary
value has a provider descendant, which means it must evaluate early in
the case of a full destroy. This means the value may contain incorrect
data when referencing resource that are create_before_destroy, or being
scaled-in via count or for_each. That will need to be addressed later by
reevaluating how we handle the full destroy case in terraform.
During a full destroy, providers may reference resources that are going
to be destroyed as well. We currently cannot change this behavior, so we
need to allow the evaluation and try to prevent it from leaking into as
many other places as possible. Another transformer to try and protect
the values in locals, variables and outputs will be added to enforce
destroy ordering when possible.
Outputs and locals cannot refer to destroy nodes. Since those nodes
types do not have different ordering for create and destroy operations,
connecting them directly to destroy nodes can cause cycles.
The destroy graph builder test requires state in order to be correct,
which it didn't have. The other tests hits the edge case where a planned
destroy cannot remove outputs, because the apply phase does not know it
was created from a destroy.
Since data source destruction is only state removal, and other resources
cannot depend on them creating any physical resources, the destroy
dependencies were not tracked in the state. It turns out that there is a
special case which requires this; running terraform destroy where the
provider depends on a data source. In that case the resources using that
provider need to record their indirect dependence on the data source, so
that they can be deleted before the data source is removed from the
state.
Our reference transformer analyses and our destroy transformer analyses
are built around static (not-yet-expanded) addresses so that they can
correctly handle mixtures of expanded and not-yet-expanded objects in the
same graph.
However, this characteristic also makes them unnecessarily conservative
in their handling of references between resources within different
instances of the same module: we know they can never interact with each
other in practice because the dependencies for all instances of a module
are the same and so one instance cannot possibly depend on another.
As a compromise then, here we introduce a new helper function that can
recognize when a proposed edge is between two resource instances that
belong to different instances of the same module, and thus allow us to
skip actually creating those edges even though our imprecise analyses
believe them to be needed.
As well as significantly reducing the number of edges in situations where
multi-instance resources appear inside multi-instance modules, this also
fixes some potential cycles in situations where a single plan includes
both destroying an instance of a module and creating a new instance of the
same module: the dependencies between the objects in the instance being
destroyed and the objects in the instance being created can, if allowed
to connect, cause Terraform to believe that the create and the destroy
both depend on one another even though there is no need for that to be
true in practice.
This involves a very specialized helper function to encode the situation
where this exception applies. This function has an ugly name to reflect
how specialized it is; it's not intended to be of any use outside of these
three situations in particular.
A lingering FIXME caused missing configuration from provider config
blocks in the json output of terraform plan. This fixes the regression
and adds a test. For the sake of testing, I added an optional attribute
to the show test provider, which resulted in the providers schema test
getting an update - not a bad addition, but we can always add a
test-specific provider schema as needed.
Part of the upgrade process happens in the first "terraform apply" after
adding explicit source addresses in the configuration. Previously we just
left that implied under the assumption that everyone would run
"terraform apply" shortly after anyway, but there is a specific tricky
situation where the first change after upgrading is to remove a resource
from the configuration, leaving Terraform unable to complete the upgrade.
Because of that, we'll now explicitly direct users to run
"terraform apply" after upgrading. Along with that, there's a reminder to
make sure that "terraform plan" indicates no changes before upgrading, so
that completing the upgrade doesn't involve also applying changes to
remote objects.
An invalid type name in a resource (or data source) could cause a panic
when determining the implied provider for the resource. This commit adds
verification that the type name is valid. It does not add a diagnostic,
since the invalid type name would have already been caught by the
parser.
Fixes#25560
The AbstractResourceInstance type was storing the entire Resource from
the state, when it only needs the actual instance state. This would
cause resources to consume memory on the order of n^2, where n in the
number of instances of the resource.
Rather than attaching the entire resource state, which includes copying
each individual instance, only attach the ResourceInstance state, and
extract out the provider address from the Resource.
The pruneUnusedNodes transformer was skipping root level locals and
variables, causing them to be left in the graph during a full destroy.
Use the return value from temporaryValue to indicate if the node is
truly temporary or not, rather then keeping the entire root module.
The main motivation here is to produce a helpful error if a user
incorrectly uses the terraform-provider- prefix (which we see on provider
VCS repositories and plugin executables) as part of the source address.
However, this also more broadly blocks "terraform-" as a prefix in
anticipation of whatever instinct causes the phenomenon where e.g.
Python's PyPI has thousands of packages whose names start with "python-",
even though everything on PyPI is for Python by definition. This is
definitely not _necessary_, but it's better to be restrictive at first
and weaken later as needed.
If the user specifies a host that isn't a provider registry in a provider
source address then we'll print out some specialized error messages for
different variants of that situation.
In particular, this includes a special case for when the error is on the
hostname "github.com", in anticipation of folks incorrectly attempting to
use GitHub repository URLs (or Go-style module paths that happen to be
on GitHub) to specify providers, so we can give a more specific hint about
that.
This is just a different presentation of an existing error case that we
are already covering in the installer tests, so there are no new tests
here. We could in principle have a test covering the exact text of these
error messages, but we don't have much precedent for command package tests
covering that level of cosmetic detail.
For Terraform v0.12 we introduced a special loading mode where we would
use the 0.11-syntax-compatible "earlyconfig" package as a heuristic to
identify situations where it was likely that the user was trying to use
0.11-only syntax that the upgrade tool might help with.
However, as the language has moved on that is no longer a suitable
heuristic in Terraform 0.13 and later: other new additions to the
language can cause the main loader to disagree with earlyconfig, which
would lead us to give poor advice about how to respond.
Instead, we'll now return the same generic "there are errors" message in
all syntax error cases. We have an extra message for errors in this
case (as compared to other commands) because "terraform init" is usually
the first command a new user interacts with and so this message gives some
extra explanation about what "terraform init" will do with the
configuration once it's valid.
This also includes a reset control character in the output of the message
as part of our ongoing mission to stop Terraform printing out whole
paragraphs of colored text, which can often be hard to read for various
reasons.
Matthew Sanabria