* states: fix RootOutputValuesEqual comparing s2 to itself
RootOutputValuesEqual had a copy-paste bug where it iterated over
s2.RootOutputValues instead of s.RootOutputValues, effectively
comparing s2 against itself rather than comparing the receiver (s)
against the argument (s2). This meant the function would always
return true as long as both states had the same number of output
values, regardless of whether the actual values differed.
This bug was introduced in #37886 and affects refresh-only plan mode,
where RootOutputValuesEqual is used to determine if root output values
changed during refresh, which controls whether the plan is considered
applyable.
* add changelog entry for RootOutputValuesEqual fix
* Update changelog wording per reviewer suggestion
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
the use of reflect.DeepEqual fails for states, because they contain
value which cannot be directly compared for equality. The Equal method
is not used much, except to aid in backend migration, so the failure
there was rarely noticed.
The failure of ManagedResourcesEqual would show up in the CLI after
Terraform reported there were no changes, by exiting with a non-zero
code because the resource states incorrectly reported as being changed.
* Update remote package's Client interface to use diagnostics instead of errors
* Update all implementations of Client interface to match changes, update calling code (incl. tests) to use diags.
* Add code to 'wrap' a provider implementation so that it can behave like a backend.Backend
* Add code for creating a state manager that will interact with a provider via grpc to interact with state
* Remove prototyping code
* Update old implementation of PrepareConfig on Pluggable to match new RPC names
* Implement Configure method on Pluggable
* Implement Workspaces and DeleteWorkspace methods on Pluggable
* Prevent construction of a pluggable with missing data, add godoc comment to NewPluggable
* Add godoc comment to ConfigSchema
* Refactor how we create a state manager for interacting with PSS
Now we reuse the remote.State struct and all the pre-existing logic there. We still wrap the provider interface to enable use of gRPC methods, but that's now done using the remote.Client interface.
* Rename file
* Move file into the remote package
* Rename file and package to `pluggable`
* Add test for the only method implemented fully on `grpcClient` : Delete
* Add tests for `NewPluggable`
* Add tests for (Pluggable).ConfigSchema method, make minor fixes
* Change mocks: hardcoded response or logic should return before default logic in method.
* Add tests for (Pluggable).PrepareConfig method
* Add tests for (Pluggable).Configure method
* Add tests for (Pluggable).Workspaces method
* Add tests for (Pluggable).DeleteWorkspace method
* Fix rebase
* Run `make syncdeps`
* Add headers
* Add missing comments
* No need to implement ClientForcePusher
* Apply feedback from review, make small tweaks to test failure messages
* configschema: Add identity attribute to import block
* Mark import target ID as legacy
* Add test with import identity
* Use ID or identity when importing via configuration
* Add plan import tests
* Review Feedback
* Make sure to copy identity for ResourceInstanceObjects
* Add helper for converting cty.Objects to string
* Replace getProvider calls
* Improve unknown object check
* Fix copy-pasta
* Update some comments clarifying backend-related interfaces and "enhanced" versus "operations"
* Fix more comments that refer to types and interfaces that have moved into the backendrun package
* jsonstate: Marshal identity values
* jsonstate: Test identity marshalling
* Add identity to prepareStateV4
* Check identity schema version when marshaling state
* Marshal identity for deposed resources
* Marshal identity version if `0`
* Check for missing resource identity schema
* check root output values when determining plan applicability in refresh mode
In refresh-only mode, we do not anticipate proposing any actions; however, a plan is marked as “applyable” if there are changes in the state between runs. Currently, a plan is considered “applyable” only when there are differences in managed resources. This approach seems to overlook changes in root output values. As a result, a plan can be marked as non-applyable, even when there are changes in the root output value, due to the lack of detected changes since only managed resources were checked.
* set 'applyable' to true
* Remove handling of ephemeral root outputs
This is effectively reverting ~99% of https://github.com/hashicorp/terraform/pull/35676
The only changes not being reverted are some formatting and deprecation fixes which remain relevant.
The code being removed is basically dead code now in the context of root ephemeral outputs being rejected per https://github.com/hashicorp/terraform/pull/35791
* Remove unrelated changes
Ephemeral root output values must be kept in the in-memory state representation, but not written to the state file. To achieve this, we store ephemeral root outputs separately from non-ephemeral root outputs, so Terraform can access them during a single plan or apply phase.
Ephemeral root outputs always have a value of null in the state file. This means that the "terraform output" command, that reads the state file, reports null values for these outputs. Consumers of 'terraform output -json' should use the presence of '"ephemeral": true' in such output to interpret the value correctly.
Simple state cache after encoding so we have it for immediate decode if
needed. Making state work entire with decoded state would be
preferable, but that is a really heavy lift right now.
In the very first implementation of "sensitive values" we were
unfortunately not disciplined about separating the idea of "marked value"
from the idea of "sensitive value" (where the latter is a subset of the
former). The first implementation just assumed that any marking whatsoever
meant "sensitive".
We later improved that by adding the marks package and the marks.Sensitive
value to standardize on the representation of "sensitive value" as being
a value marked with _that specific mark_.
However, we did not perform a thorough review of all of the mark-handling
codepaths to make sure they all agreed on that definition. In particular,
the state and plan models were both designed as if they supported arbitrary
marks but then in practice marks other than marks.Sensitive would be
handled in various inconsistent ways: dropped entirely, or interpreted as
if marks.Sensitive, and possibly do so inconsistently when a value is
used only in memory vs. round-tripped through a wire/file format.
The goal of this commit is to resolve those oddities so that there are now
two possible situations:
- General mark handling: some codepaths genuinely handle marks
generically, by transporting them from input value to output value in
a way consistent with how cty itself deals with marks. This is the
ideal case because it means we can add new marks in future and assume
these codepaths will handle them correctly without any further
modifications.
- Sensitive-only mark preservation: the codepaths that interact with our
wire protocols and file formats typically have only specialized support
for sensitive values in particular, and lack support for any other
marks. Those codepaths are now subject to a new rule where they must
return an error if asked to deal with any other mark, so that if we
introduce new marks in future we'll be forced either to define how we'll
avoid those markings reaching the file/wire formats or extend the
file/wire formats to support the new marks.
Some new helper functions in package marks are intended to standardize how
we deal with the "sensitive values only" situations, in the hope that
this will make it easier to keep things consistent as the codebase evolves
in future.
In practice the modules runtime only ever uses marks.Sensitive as a mark
today, so all of these checks are effectively covering "should never
happen" cases. The only other mark Terraform uses is an implementation
detail of "terraform console" and does not interact with any of the
codepaths that only support sensitive values in particular.
The state serialization only knows how to save and restore the
marks.Sensitive mark in particular, but previously it was just assuming
that any mark it found was sensitive without actually checking.
Now we'll return an error if we're asked to save a mark we don't support.
In practice there are no other marks currently used by the modules runtime
and so this cannot fail, but this is to help notice problems sooner if we
introduce any new marks later.
This interface was originally in the local backend's package because that
backend is the only one that _truly_ runs Terraform operations and thus
needs to negotiate how to store the intermediate state snapshots that
result from them.
However, putting the interface and its supporting types/functions there
means that anything that wants to implement this interface needs to import
the local backend package, and thus indirectly depends on the entire
modules runtime, etc.
We already had package stagemgr as a home for the shared stuff used by most
state storage managers, and so we'll just move these symbols over there
and then we can shed some internal dependencies on package local.
This also severs the last dependency link between the remote state backends
and the modules runtime, allowing us to shed a bunch of indirect
dependencies from those remote state backends.
schemarepo.Schemas used to be terraform.Schemas, but its presence in the
terraform package was the only reason for a state manager to need to
import package terraform, so we've moved it out into its own package in
the parent commit.
This means that none of the state-related packages need to import package
terraform any longer, and so once we've untangled some other dependencies
this will curtail the number of packages in this codebase that the remote
state backends transitively depend on, as part of an ongoing effort to
eventually separate them from this codebase entirely.
As we previously found with "package addrs", any package that contains
types used as identifiers or addresses tends to get imported from all over,
which means that if we mix real logic with those types in the same package
then a lot more packages end up depending indirectly on our external
dependencies.
In this case, the getproviders package depends on various things we need
only during provider installation, such as the library we use for GPG
signature verification. Lots of different packages in Terraform need to
talk about provider requirements, but only the CLI layer actually needs to
do provider installation, and so this commit splits the types we use to
talk about requirements into a new package "providerreqs", which
package getproviders then depends on.
To keep the size of this diff relatively small to start I've left
forwarding aliases in package getproviders, which means that not all of
the callers needed to be updated all at once. This commit does update
all of the packages that the remote state backends depend on though,
because that then allows us to reduce the set of indirect dependencies
for each of those backends.
The nested module go.mod files show that this has disconnected the indirect
dependencies on github.com/ProtonMail/go-crypto and on
github.com/cloudflare/circl , both of which Terraform CLI uses only for
provider installation.
This package was our last non-legacy use of the third-party "copystructure"
library, and that library is far more general than most of this work needs.
Most of this is replacing overly-general code with more specific code,
since it's not exactly hard to shallow-copy a map or a slice of strings.
The 2-to-3 upgrade is a little trickier because we were previously copying
the entire data structure before making light modifications to it, since
the V2 and V3 formats are similar enough. However, we can be a little
naughty and use our mainly-test-oriented copy.DeepCopyValue helper instead
of copystructure, since these structures are simple enough for that to
work and are never going to change because they are here only to support
upgrading from long-obsolete file formats.
This removes Terraform's last non-legacy dependency on copystructure, and
with it the last non-legacy indirect dependency on the sibling module
"reflectwalk".
This functionality is now provided by the "errors" package in the standard
library, so the HashiCorp-specific library is obsolete.
This also removed our one direct use of github.com/pkg/errors, which we
were using in a way that was totally redundant with the stdlib package
errors. However, it lives on as an indirect dependency through the Consul
backend's dependency on the Consul SDK, which uses that library.
Back when we added local values (a long time ago now!) we put their
results in state mainly just because it was the only suitable shared data
structure to keep them in. They are a bit ideosyncratic there because we
intentionally discard them when serializing state to a snapshot, and
that's just fine because they never need to be retained between runs
anyway.
We now have namedvals.State for all of our named value result storage
needs, so we can remove the local-value-related fields of states.Module
and use the relevant map inside the local value state instead.
As of this commit, states.State now tracks only the data that we
actually persist between runs in state snapshots, which will hopefully
avoid future bugs resulting from the former difference in fidelity
between a freshly-created in-memory state vs. one loaded from a
snapshot.
For a very long time we've had an annoying discrepancy between the
in-memory state model and our state snapshot format where the in-memory
format stores output values for all modules whereas the snapshot format
only tracks the root module output values because those are all we
actually need to preserve between runs.
That design wart was a result of us using the state both as an internal
and an external artifact, due to having nowhere else to store the
transient values of non-root module output values while Terraform Core
does its work.
We now have namedvals.State to internally track all of the throwaway
results from named values that don't need to persist between runs, so now
we'll use that for our internal work instead and reserve the states.State
model only for the data that we will preserve between runs in state
snapshots.
The namedvals internal model isn't really designed to support enumerating
all of the output values for a particular module call, but our expression
evaluator currently depends on being able to do that and so we have a
temporary inefficient implementation of that which just scans the entire
table of values as a stopgap just to avoid this commit growing even larger
than it already is. In a future commit we'll rework the evaluator to
support the PartialEval mode and at the same time move the responsiblity
for enumerating all of the output values into the evaluator itself, since
it should be able to determine what it's expecting by analyzing the
configuration rather than just by trusting that earlier evaluation has
completed correctly.
Because our legacy state string serialization previously included output
values for all modules, some of our context tests were accidentally
depending on the implementation detail of how those got stored internally.
Those tests are updated here to test only the data that is a real part
of Terraform Core's result, by ensuring that the relevant data appears
somewhere either in a root output value or in a resource attribute.
Previously a SyncState became completely invalid after a call to Close,
but that's a little too strict because SyncState objects live on as part
of any returned lang.Scope objects that a caller might want to use for
(read-only) expression evaluation.
As a pragmatic compromise then, we'll instead continue to allow read access
through the SyncState object after it's closed, which is safe as long as
the caller does not try to perform those reads concurrently with its own
modifications to the underlying state.
In the previous commit we renamed AllResourceInstaceObjectAddrs to
AllManagedResourceInstanceObjectAddrs to reflect that it filters out any
resource instances that aren't "managed".
This now adds back in a State.AllResourceInstanceObjectAddrs method which
does what the other one sounded like it did before: returns all of the
known addresses without any filtering whatsoever.
This method used the more appropriate name
AllManagedResourceInstanceObjectAddrs in its docstring, but was
incorrectly named just AllResourceInstanceObjectAddrs in its actual
definition, obscuring the fact that it filters out everything except
objects from managed resources.
Now we'll use the originally-documented name to make this clearer. This
also factors out the implementation into a separate unexported function
that takes the filtering criteria as an argument, since in future commits
we're going to add other variations of this which return different subsets
of the resource instance objects in the state.
Originally we had this function returning a slice of an anonymous struct
type because we hadn't yet recognized "resource instance object" as a
first-class address type.
We do now have addrs.AbsResourceInstanceObject to represent that sort of
thing, and so we'll use that here. This also means we can return a set
of addresses rather than a slice, which is semantically better as this
set of addresses is not inherently ordered.
Some existing callers were depending on the sort.SliceStable we were
originally doing to make the result dependable, and so now we have a
generic helper for turning addrs.Set into a sorted slice of the same type
using our address types' "Less" methods that describe their natural or
preferred ordering.
We need to retain the prior state (in a form that Terraform Core's modules
runtime can accept) between the plan and apply phases because Terraform
uses it to verify that the provider's "final plan" is consistent with
the initial plan.
We previously tried to do this by reusing the "old" value from the planned
change, but that's not really possible in practice because of the
historical implementation detail that Terraform wants state information
in a JSON format and plan information in MessagePack.
This also contains the beginnings of handling the "discarding" of
unrecognized keys from the state data structures, although we'll probably
need to do more in that area later since this is just the bare minimum.
Previously we had states.DeposedKey as our representation of the unique
keys used to differentiate between multiple deposed objects on the same
resource instance. That type is useful in various places, but its presence
in the "states" package made it troublesome to import for some callers.
Since its purpose is as an identifier, this type is more at home in the
"addrs" package. The states package retains a small number of aliases to
preserve compatibility with a few existing callers for now, until we have
the stomach for a more invasive change.
This also removes the largely-redundant states.Generation interface type,
which initially was intended as a way to distinguish between "current"
objects and deposed objects, but didn't really add anything because we
already have addrs.NotDeposed to represent that situation. Instead, we
just ended up with various bits of boilerplate adapter code converting
between the two representations. Everything now uses addrs.DeposedKey and
uses addrs.NotDeposed to identify non-deposed objects.
This is a helper for when we're accepting a deposed key from outside of
Terraform, such as in a plan or state file. It doesn't really "parse"
the string in the usual sense -- the result is always just the input
verbatim if successful -- but it's named that way for consistency with
how we usually name functions like this.
