NoOp changes should not participate in a destroy sequence, but because
they are included as normal update nodes the usual connections were
still being made.
Refreshing for a destroy should use the refresh-only plan to avoid
planning new objects or evaluating conditions. This should also be
skipped if there is no state, since there would be nothing to refresh.
IsFullDestroy was a workaround during apply to detect when the change
set was created by a destroy plan. This no longer works correctly, and
we need to fall back to the UIMode set in the plan.
Not all root output instances were going through proper expansion when
destroy operations were involved, leading to cases where they would be
evaluated even though the expected result was only to remove them from
the state.
Normally destroy nodes stand alone in the graph, and do not produce
references to other nodes. Because root output nodes were replaced by
expansion nodes, these were being connected via normal references, even
in the case where we were working with a destroy graph.
Module output may need to be evaluated during destroy in order to
possibly be used by providers. The final state however is that all
objects are destroyed, so preconditions should not be evaluated.
The graph walking mechanism is specified as requiring a graph with a single
root, which in practice means there's exactly one node in the graph
which doesn't have any dependencies.
However, we previously weren't verifying that invariant is true for
subgraphs returned from DynamicExpand. It was working anyway, but it's not
ideal to be relying on a behavior that isn't guaranteed by our underlying
infrastructure.
We also previously had the RootTransformer being a bit clever and trying
to avoid adding a new node if there is already only a single graph with
no dependencies. That special case isn't particularly valuable since
there's no harm in turning a one-node graph into a two-node graph with
an explicit separate root node, and doing that allows us to assume that
the root node is always present and is always exactly terraform.rootNode.
Many existing DynamicExpand implementations were not producing valid
graphs and were previously getting away with it. All of them now produce
properly-rooted graphs that should pass validation, and we will guarantee
that with an explicit check of the DynamicExpand return value before we
try to walk that subgraph. For good measure we also verify that the root
node is exactly terraform.rootNode, even though that isn't strictly
required by our graph walker, just to help us catch potential future bugs
where a DynamicExpand implementation neglects to add our singleton root
node.
This opts to inline document these intentional design decisions in the protocol definition as a catch-all for it not being documented elsewhere.
Protocol Buffers files updated via:
```shell
make protobuf
```
An orphaned resource which plans as a NoOp change will have no config.
This is not an error, but there is nothing to do since there are also
no checks to validate. We still leave the change in the plan to keep the
plan as complete as possible, noting all possible changes.
Preventing the node from being added to the graph is awkward, because
the config is attached separately from the diff transformer. This should
not pose any problems however, because there is no longer any state or
config linking the instance to any dependencies in the graph.
* Add support for `storage_custom_endpoint` in `gcs` backend
* Add documentation for new `storage_custom_endpoint` endpoint
* Empty commit to trigger Vercel deployment
When reading this code to check Terraform's graph sorting behavior, I got very
confused about the direction of traversal for several methods. Although some of
these methods would also probably benefit from renames, this commit only updates
their doc comments to use the same directional terminology that we use in the
`Edge` interface (source/target).
Because import does not yet plan new instances as part of the import
process, we can end up evaluating references to resources which have no
state at all. The fallback for this situation could result in slightly
better values during import. The count and for_each values were
technically incorrect, since the length is not known to be zero, and the
single instance does have a concrete type which we can return.
When we checked for cycles with destroy edges around providers, it was
only for providers of a different type, but one can do the same thing
with the same provider under different local aliases. Check to see if
the provider also contains an alias, or is defined absolutely in some
other way. The absolute accuracy here isn't critical, since in most
cases these edges are not required for correct results, but finding a
correct and consistent method for determining when these edges are
needed is going to take more research.
There was also an oversight fixed here where the basic
creator->destroyer edges were added _after_ the cycle checks, limiting
their utility. The ordering of the additions was swapped to make sure
all cycles are noticed.
* Add ability to use customer-managed KMS key to encrypt state, add acceptance tests
* Change test names for different encrpytion methods
* Commit files updated by `go mod tidy`
* Add guard against missing ENVs to `setupKmsKey` func
* Update KMS setup function to get credentials from ENVs
* Update tests to not include zero-values in config
This means that default values are supplied later by TF instead of supplied as config from the user
This also avoids issues related to making field conflicts explicit with `ConflictsWith`
* Make `encryption_key` & `kms_encryption_key` conflicting fields
Removing the Default from `encryption_key` does not appear to be a breaking change when tested manually
* Add ability to set `kms_encryption_key` via ENV
* Refactor `encryption_key` to use `DefaultFunc` to access ENV, if set
* Remove comments
* Update `gcs` backend docs & descriptions in schema
* Update `gcs` backend docs to include information on encryption methods
* Apply technical writing suggestions from code review
Co-authored-by: Matthew Garrell <69917312+mgarrell777@users.noreply.github.com>
* Update documentation to remove passive voice
* Change use of context in tests, add inline comment, update logs
* Remove use of `ReadPathOrContents` for new field
Co-authored-by: Matthew Garrell <69917312+mgarrell777@users.noreply.github.com>
If a previously deposed object is deleted outside of Terraform, the
next plan will result in a NoOp change for the deposed object. Fix the
check to verify that the deposed object has an acceptable action rather
than use the `update` flag.
We originally included this warning because the go-cty-yaml module wasn't
yet stable and it was also not extensively tested so it wasn't yet clear
if its behavior would need to change in some less common cases we hadn't
tested so far.
However, go-cty-yaml had its v1.0.0 release some time ago and is now
committed to preserving its current Marshal output unless it is found to
be non-compliant with the YAML 1.2 specification. This doc change means
that Terraform's yamlencode is now adopting a similar posture:
- The exact style details produced by the function for a particular input
are now frozen. It'll change only if we find that the function is
producing output that isn't valid per the YAML spec.
- If someone finds a YAML parser that cannot parse what yamlencode
produces but what it produces is valid per the YAML 1.2 spec, we'll
expect the parser to be corrected to better support the spec rather
than changing the yamlencode output.
There may be pragmatic exceptions if we encounter a situation we cannot
anticipate yet, but the above will be our general rule. This is really
just a specialization of the spirit of the v1.x Compatibility Promises,
tailored specifically to this function.
Legacy providers expect Terraform to be able to clean up invalid plans
and computed attributes. Add a special case for the LegacyTypeSystem to
revert `ignore_changes = all` to the complete prior state.
