Previously in refresh-only mode we were skipping making any updates to the
working state at all. That's not correct, though: if the state upgrade or
refresh steps detected changes then we need to at least commit _those_ to
the working state, because those can then be detected by downstream
objects like output values.
If we don't do this then we can create a situation where refresh detects
that an object already doesn't exist but we plan to destroy it anyway,
rather than returning "no changes" as expected.
The "previous run state" is our record of what the previous run of
Terraform considered to be its outcome, but in order to do anything useful
with that we must ensure that the data inside conforms to the current
resource type schemas, which might be different than the schemas that were
current during the previous run if the relevant provider has since been
upgraded.
For that reason then, we'll start off with the previous run state set
exactly equal to what was saved in the prior snapshot (modulo any changes
that happened during a state file format upgrade) but then during our
planning operation we'll overwrite individual resource instance objects
with the result of upgrading, so that in a situation where we successfully
run plan to completion the previous run state should always have a
compatible schema with the "prior state" (the result of refreshing) for
managed resources, and thus the caller can meaningfully compare the two
in order to detect and describe any out-of-band changes that occurred
since the previous run.
Until now we've not really cared much about the state snapshot produced
by the previous Terraform operation, except to use it as a jumping-off
point for our refresh step.
However, we'd like to be able to report to an end-user whenever Terraform
detects a change that occurred outside of Terraform, because that's often
helpful context for understanding why a plan contains changes that don't
seem to have corresponding changes in the configuration.
As part of reporting that we'll need to keep track of the state as it
was before we did any refreshing work, so we can then compare that against
the state after refreshing. To retain enough data to achieve that, the
existing Plan field State is now two fields: PrevRunState and PriorState.
This also includes a very shallow change in the core package to make it
populate something somewhat-reasonable into this field so that integration
tests can function reasonably. However, this shallow implementation isn't
really sufficient for real-world use of PrevRunState because we'll
actually need to update PrevRunState as part of planning in order to
incorporate the results of any provider-specific state upgrades to make
the PrevRunState objects compatible with the current provider schema, or
else our diffs won't be valid. This deeper awareness of PrevRunState in
Terraform Core will follow in a subsequent commit, prior to anything else
making use of Plan.PrevRunState.
If the user gives an index-less address for a resource that expects
instance keys then previously we would've emitted one error per declared
instance of the resource, which is overwhelming and not especially
helpful.
Instead, we'll deal with that check prior to expanding resources into
resource instances, and thus we can report a single error which talks
about all of the instances at once.
This does unfortunately come at the expense of splitting the logic for
dealing with the "force replace" addresses into two places, which will
likely make later maintenance harder. In an attempt to mitigate that,
I've included a comment in each place that mentions the other place, which
hopefully future maintainers will keep up-to-date if that situation
changes.
If a root modules declares a required_provider but has no configuration,
add a graph node for the provider as if there were an empty
configuration. This will allow the provider to be referenced by name in
module call provider maps, so that a module can pass a default provider
by name to a submodule.
Normally these nodes are added by the MissingProviderTransformer, but
they need to be in place earlier to resolve any possible "proxy provider
nodes" within modules.
This is to make it more obvious at all uses of this field that it's not
something to be used for anything other than UI decisions, hopefully
prompting a reader of code elsewhere to refer to the comments to
understand why it has this unusual prefix and thus see what its intended
purpose is.
This only includes the internal mechanisms to make it work, and not any
of the necessary UI changes to "terraform plan" and "terraform apply" to
activate it yet.
The force-replace options are ultimately handled inside the
NodeAbstractResourceInstance.plan method, at the same place we handle the
similar situation of the provider indicating that replacement is needed,
and so the rest of the changes here are just to propagate the settings
through all of the layers in order to reach that point.
This only includes the core mechanisms to make it work. There's not yet
any way to turn this mode on as an end-user, because we have to do some
more work at the UI layer to present this well before we could include it
as an end-user-visible feature in a release.
At the lowest level of abstraction inside the graph nodes themselves, this
effectively mirrors the existing option to disable refreshing with a new
option to disable change-planning, so that either "half" of the process
can be disabled. As far as the nodes are concerned it would be possible
in principle to disable _both_, but the higher-level representation of
these modes prevents that combination from reaching Terraform Core in
practice, because we block using -refresh-only and -refresh=false at the
same time.
Previously we were repeating some logic in the UI layer in order to
recover relevant additional context about a change to report to a user.
In order to help keep things consistent, and to have a clearer path for
adding more such things in the future, here we capture this user-facing
idea of an "action reason" within the plan model, and then use that
directly in order to decide how to describe the change to the user.
For the moment the "tainted" situation is the only one that gets a special
message, matching what we had before, but we can expand on this in future
in order to give better feedback about the other replace situations too.
This also preemptively includes the "replacing by request" reason, which
is currently not reachable but will be used in the near future as part of
implementing the -replace=... plan command line option to allow forcing
a particular object to be replaced.
So far we don't have any special reasons for anything other than replacing,
which makes sense because replacing is the only one that is in a sense
a special case of another action (Update), but this could expand to
other kinds of reasons in the future, such as explaining which of the
few different reasons a data source read might be deferred until the
apply step.
There were some remaining calls to provider where configuration could be
added to diagnostics, where warnings would not get config annotations,
or the diagnostics were skipped entirely.
Previously there were only two planning modes: normal mode and destroy
mode. In that context it made sense for these to be distinguished only by
a boolean flag.
We're now getting ready to add our third mode, "refresh only". This
establishes the idea that planning can be done in one of a number of
mutually-exclusive "modes", which are related to but separate from the
various other options that serve as modifiers for the plan operation.
This commit only introduces the new plans.Mode type and replaces the
existing "destroy" flag with a variable of that type. This doesn't cause
any change in effective behavior because Terraform Core still supports
only NormalMode and DestroyMode, with NewContext rejecting an attempt to
create a RefreshMode context for now.
It is in retrospect a little odd that the "destroy" flag was part of
ContextOpts rather than just an argument to the Plan method, but
refactoring that would be too invasive a change for right now so we'll
leave this as a field of the context for now and save revisiting that for
another day.
When rendering a stored plan file as JSON, we include a data structure
representing the sensitivity of the changed resource values. Prior to
this commit, this was a direct representation of the sensitivity marks
applied to values via mechanisms such as sensitive variables, sensitive
outputs, and the `sensitive` function.
This commit extends this to include sensitivity based on the provider
schema. This is in line with the UI rendering of the plan, which
considers these two different types of sensitivity to be equivalent.
Co-authored-by: Kristin Laemmert <mildwonkey@users.noreply.github.com>
Non-root module outputs no longer strip sensitivity marks from their
values, allowing dynamically sensitive values to propagate through the
configuration. We also remove the requirement for non-root module
outputs to be defined as sensitive if the value is marked as sensitive.
This avoids a static/dynamic clash when using shared modules that might
unknowingly receive sensitive values via input variables.
Co-authored-by: Martin Atkins <mart@degeneration.co.uk>
The destroy plan walk was identifying itself as a normal plan, and
causing providers to be configured when they were not needed. Since the
provider configuration may not be complete during the minimal destroy
plan walk, validation or configuration may fail.
Calling the nonsensitive function with values which are not sensitive
will result in an error. This restriction was added with the goal of
preventing confusingly redundant use of this function.
Unfortunately, this breaks when using nonsensitive to reveal the value of
sensitive resource attributes. This is because the validate walk does
not (and cannot) mark attributes as sensitive based on the schema,
because the resource value itself is unknown.
This commit therefore alters this restriction such that it permits
nonsensitive unknown values, and adds a test case to cover this specific
scenario.
Dependencies are tracked via configuration addresses, but when dealing
with depends_on references they can only apply to resources within the
same module instance. When determining if a data source can be read
during planning, verify that the dependency change is coming from the
same module instance.
When applying sensitivity marks to resources, we previously would first
mark any provider-denoted sensitive attributes, then apply the set of
planned-change sensitive value marks. This would cause a panic if a
provider marked an iterable value as sensitive, because it is invalid to
call `MarkWithPaths` against a marked iterable value.
Instead, we now merge the marks from the provider schema and the planned
change into a single set, and apply them with one call. The included
test panics without this change.
The resource configuration was always being used to determine
dependencies during refresh, because if there were no changes to a
resource, there was no chance to replace any incorrect stored
dependencies. Now that we are refreshing during plan, we can determine
that a resource has no changes and opt to store the new dependencies
immediately.
Here we clean up the writeResourceInstanceState calls to no longer
modify the resource instance state, removing the `dependencies`
argument. Callers are now expected to set the Dependencies field as
needed.
When we map schema sensitivity to resource values, there may be unknowns
when dealing with planned objects. Check for unknowns before iterating
over block values.
* checkpoint save: update InternalValidate tests to compare exact error
* configschema: extract and extend attribute validation
This commit adds an attribute-specific InternalValidate which was extracted directly from the block.InternalValidate logic and extended to verify any NestedTypes inside an Attribute. Only one error message changed, since it is now valid to have a cty.NilType for Attribute.Type as long as NestedType is set.
* terraform: validate provider schema's during NewContext
We haven't been able to guarantee that providers are validating their own schemas using (some version of) InternalValidate since providers were split out of the main codebase. This PR adds a call to InternalValidate when provider schemas are initially loaded by NewContext, which required a few other changes:
InternalValidate's handling of errors vs multierrors was a little weird - before this PR, it was occasionally returning a non-nil error which only stated "0 errors occurred" - so I addressed that in InternalValidate. I then tested this with a configuration that was using all of our most popular providers, and found that at least on provider had some invalid attribute names, so I commented that particular validation out. Adding that in would be a breaking change which we would have to coordinate with enablement and providers and (especially in this case) make sure it's well communicated to external provider developers.
I ran a few very unscientific tests comparing the timing with and without this validation, and it appeared to only cause a sub-second increase.
* refactor validate error message to closer match the sdk's message
* better error message
* tweak error message: move the instruction to run init to the end of the message, after the specific error.
We currently count on interconnecting destroy nodes to handle the
create->destroy dependency edge for replacement, but when the create
node is only an update we don't connect that edge directly.
Lookup all creators that are dependencies of the destory node and ensure
they are connected.
The provider transformers remove extra provider nodes when they are
initially setup, but it may turn out that they are not used later on.
The pruneUnusedNodesTransformer takes care of removing unused expansion
nodes, which originally required a provider, and hence may cause some
provider nodes to no longer be needed. We can also detect these and
remove them during the pruneUnusedNodesTransformer process.
* providers.Interface: rename ValidateDataSourceConfig to
ValidateDataResourceConfig
This PR came about after renaming ValidateResourceTypeConfig to
ValidateResourceConfig: I now understand that we'd called it the former
instead of the latter to indicate that the function wasn't necessarily
operating on a resource that actually exists. A possibly-more-accurate
renaming of both functions might then be ValidateManagedResourceConfig
and ValidateDataResourceConfig.
The next commit will update the protocol (v6 only) as well; these are in
separate commits for reviewers and will get squashed together before
merging.
* extend renaming to protov6
Now that the possibility of extra provider nodes being added is gone, we
can skip the separate handling of configuration_aliases in the
ProviderConfigTransformer. Instead of adding concrete provider nodes for
configuration_aliases (which could be surprising later as the code
evolves), we implicitly add them based on the providers being passed in
by the parent module, using the same mechanism as non-aliased providers.
We can know that the `providers` map will be populated, because the
provider structures are validated while loading the configuration.
Martin Atkins