A couple of bugs have been discovered in ResourceDiff.ForceNew:
* NewRemoved is not preserved when a diff for a key is already present.
This is because the second diff that happens after customization
performs a second getChange on not just state and config, but also on
the pre-existing diff. This results in Exists == true, meaning nil is
never returned as a new value.
* ForceNew was doing the work of adding the key to the list of changed
keys by doing a full SetNew on the existing value. This has a side
effect of fetching zero values from what were otherwise undefined values
and creating diffs for these values where there should not have been
(example: "" => "0").
This update fixes these scenarios by:
* Adding a new private function to check the existing diff for
NewRemoved keys. This is included in the check on new values in
diffChange.
* Keys that have been flagged as ForceNew (or parent keys of lists and
sets that have been flagged as ForceNew) are now maintained in a
separate map. UpdatedKeys now returns the results of both of these maps,
but otherwise these keys are ignored by ResourceDiff.
* Pursuant the above, values are no longer pushed into the newDiff
writer by ForceNew. This prevents the zero value problem, and makes for
a cleaner implementation where the provider has to "manually" SetNew to
update the appropriate values in the writer. It also prevents
non-computed keys from winding up in the diff, which ResourceDiff
normally blocks by design.
There are also a couple of tests for cases that should never come up
right now involving Optional/Computed values and NewRemoved, for which
explanations are given in annotations of each test. These are here to
guard against future regressions.
Match the tested behavior, and that of the ssh implementation, where the
communicator automatically connects when starting a command.
Remove unused import from legacy dependency handling.
The error from a remote command is not exported, and only exposed via
the Run method. Otherwise the Run method works exactly like the
runCommand function being removed.
Create-before-destroy dependencies are automatically ordered correctly
by terraform. Remove the old notice about requiring all dependencies to
have the same setting for create_before_destroy.
During a full destroy when outputs are removed, the
NodeDestroyableOutput was preventing it's sibling output from being
destroyed. Prune the output node if it only has its destroy node as a
dependent.
The destroy output test is simply run a second time with no state, which
would cause the output interpolation to fail if it remained in the
graph.
This was already added to triton-go and is now making its way to
the manta backend
```
% acctests backend/remote-state/manta
=== RUN TestBackend_impl
--- PASS: TestBackend_impl (0.00s)
=== RUN TestBackend
--- PASS: TestBackend (27.36s)
=== RUN TestBackendLocked
--- PASS: TestBackendLocked (16.24s)
=== RUN TestRemoteClient_impl
--- PASS: TestRemoteClient_impl (0.00s)
=== RUN TestRemoteClient
--- PASS: TestRemoteClient (3.40s)
=== RUN TestRemoteClientLocks
--- PASS: TestRemoteClientLocks (7.17s)
PASS
ok github.com/hashicorp/terraform/backend/remote-state/manta
```
If an existing resources is scaled back to 0, locals and outputs will
still have a multi-variable reference to evaluate, which should return
an empty list. Due to how the resource is removed, the resource will
still exist in the state but with no primary instance, which needs to be
ignored in the instance count.
Chris Marchesi