Init should only _add_ values, not remove them.
During graph execution, there are steps that expect that a state isn't
being actively pruned out from under it. Namely: writing deposed states.
Writing deposed states has no way to handle if a state changes
underneath it because the only way to uniquely identify a deposed state
is its index in the deposed array. When destroying deposed resources, we
set the value to `<nil>`. If the array is pruned before the next deposed
destroy, then the indexes have changed, and this can cause a crash.
This PR does the following (with more details below):
* `init()` no longer prunes.
* `ReadState()` always prunes before returning. I can't think of a
scenario where this is unsafe since generally we can always START
from a pruned state, its just causing problems to prune
mid-execution.
* Exported State APIs updated to be robust against nil ModuleStates.
Instead, I think we should adopt the following semantics for init/prune
in our structures that support it (Diff, for example). By having
consistent semantics around these functions, we can avoid this in the
future and have set expectations working with them.
* `init()` (in anything) will only ever be additive, and won't change
ordering or existing values. It won't remove values.
* `prune()` is destructive, expectedly.
* Functions on a structure must not assume a pruned structure 100% of
the time. They must be robust to handle nils. This is especially
important because in many cases values such as `Modules` in state
are exported so end users can simply modify them outside of the
exported APIs.
This PR may expose us to unknown crashes but I've tried to cover our
cases in exposed APIs by checking for nil.
It makes for sense for this to happen in State.prune(). Also move a
redundant pruning from ResourceState.init, and make sure
ResourceState.prune is called from the parent's prune method.
The Deposed slice wasn't being normalized and nil values could be read
in from a state file. Filter out the nils during init. There is
still a bug in copystructure, but that will be addressed separately.
We con no longer copy an InstanceState via a simple
dereference+assignment because of the mutex which can't be copied. This
adds a set method to properly set all field from another InstanceState,
and take the appropriate locks while doing so.
Add locks to the state structs to handle concurrency during the graph
walks. We can't embed the mutexes due to serialization constraints when
communicating with providers, so expose the Lock/Unlock methods
manually.
Use copystructure.LockedCopy to ensure locks are honored.
Fix checksum issue with remote state
If we read a state file with "null" objects in a module and they become
initialized to an empty map the state file may be written out with empty
objects rather than "null", changing the checksum. If we can detect
this, increment the serial number to prevent a conflict in atlas.
Our fakeAtlas test server now needs to decode the state directly rather
than using the ReadState function, so as to be able to read the state
unaltered.
The terraform.State data structures have initialization spread out
throughout the package. More thoroughly initialize State during
ReadState, and add a call to init() during WriteState as another
normalization safeguard.
Expose State.init through an exported Init() method, so that a new State
can be completely realized outside of the terraform package.
Additionally, the internal init now completely walks all internal state
structures ensuring that all maps and slices are initialized. While it
was mentioned before that the `init()` methods are problematic with too
many call sites, expanding this out better exposes the entry points that
will need to be refactored later for improved concurrency handling.
The State structures had a mix of `omitempty` fields. Remove omitempty
for all maps and slices as part of this normalization process. Make
Lineage mandatory, which is now explicitly set in some tests.
The behaviour whereby outputs for a particular nested module can be
output was broken by the changes for lists and maps. This commit
restores the previous behaviour by passing the module path into the
outputsAsString function.
We also add a new test of this since the code path for indivdual output
vs all outputs for a module has diverged.
Always increment the state serial whenever we upgrade the state version.
This prevents possible version conflicts between local and remote state
when one has been upgraded, but the serial numbers match.
The implementation of Stringer on OutputState previously assumed outputs
may only be strings - we now no longer cast to string, instead using the
built in formatting directives.
The previous mechanism for testing state threw away the mutation made on
the state by calling State() twice - this commit corrects the test to
match the comment.
In addition, we replace the custom copying logic with the copystructure
library to simplify the code.
The lineage of a state is an identifier shared by a set of states whose
serials are meaningfully comparable because they are produced by
progressive Refresh/Apply operations from the same initial empty state.
This is initialized as a type-4 (random) UUID when a new state is
initialized and then preserved on all other changes.
Since states before this change will not have lineage but users may wish
to set a lineage for an existing state in order to get the safety
benefits it will grow to imply, an empty lineage is considered to be
compatible with all lineages.
This commit makes the current Terraform state version 3 (previously 2),
and a migration process as part of reading v2 state. For the most part
this is unnecessary: helper/schema will deal with upgrading state for
providers written with that framework. However, for providers which
implemented the resource model directly, this gives a best-efforts
attempt at lossless upgrade.
The heuristics used to change the count of a map from the .# key to the
.% key are as follows:
- if the flat map contains any non-numeric keys, we treat it as a
map
- if the map is empty it must be computed or optional, so we remove
it from state
There is a known edge condition: maps with all-numeric keys are
indistinguishable from sets without access to the schema. They will need
manual conversion or may result in spurious diffs.
This removes support for the V0 binary state format which was present in
Terraform prior to 0.3. We still check for the file type and present an
error message explaining to the user that they can upgrade it using a
prior version of Terraform.
This means it’s shown correctly in a plan and takes into account any
actions that are dependant on the tainted resource and, vice verse, any
actions that the tainted resource depends on.
So this changes the behaviour from saying this resource is tainted so
just forget about it and make sure it gets deleted in the background,
to saying I want that resource to be recreated (taking into account the
existing resource and it’s place in the graph).
This commit forward ports the changes made for 0.6.17, in order to store
the type and sensitive flag against outputs.
It also refactors the logic of the import for V0 to V1 state, and
fixes up the call sites of the new format for outputs in V2 state.
Finally we fix up tests which did not previously set a state version
where one is required.
Once a data resource gets into the state, the state system needs to be
able to parse its id to match it with resources in the configuration.
Since data resources live in a separate namespace than managed resources,
the extra "mode" discriminator is required to specify which namespace
we're talking about, just like we do in the resource configuration.
This changes the representation of maps in the interpolator from the
dotted flatmap form of a string variable named "var.variablename.key"
per map element to use native HIL maps instead.
This involves porting some of the interpolation functions in order to
keep the tests green, and adding support for map outputs.
There is one backwards incompatibility: as a result of an implementation
detail of maps, one could access an indexed map variable using the
syntax "${var.variablename.key}".
This is no longer possible - instead HIL native syntax -
"${var.variablename["key"]}" must be used. This was previously
documented, (though not heavily used) so it must be noted as a backward
compatibility issue for Terraform 0.7.
This commit adds the groundwork for supporting module outputs of types
other than string. In order to do so, the state version is increased
from 1 to 2 (though the "public-facing" state version is actually as the
first state file was binary).
Tests are added to ensure that V2 (1) state is upgraded to V3 (2) state,
though no separate read path is required since the V2 JSON will
unmarshal correctly into the V3 structure.
Outputs in a ModuleState are now of type map[string]interface{}, and a
test covers round-tripping string, []string and map[string]string, which
should cover all of the types in question.
Type switches have been added where necessary to deal with the
interface{} value, but they currently default to panicking when the input
is not a string.
This commit rectifies the fact that the original binary state is
referred to as V1 in the source code, but the first version of the JSON
state uses StateVersion: 1. We instead make the code refer to V0 as the
binary state, and V1 as the first version of JSON state.
This adds a field terraform_version to the state that represents the
Terraform version that wrote that state. If Terraform encounters a state
written by a future version, it will error. You must use at least the
version that wrote that state.
Internally we have fields to override this behavior (StateFutureAllowed),
but I chose not to expose them as CLI flags, since the user can just
modify the state directly. This is tricky, but should be tricky to
represent the horrible disaster that can happen by enabling it.
We didn't have to bump the state format version since the absense of the
field means it was written by version "0.0.0" which will always be
older. In effect though this change will always apply to version 2 of
the state since it appears in 0.7 which bumped the version for other
purposes.
I decided to split this up from the terraform state rm command to make the diff easier to see. These changes will also be used for terraform state mv.
This adds a `Remove` method to the `*terraform.State` struct. It takes a list of addresses and removes the items matching that list. This leverages the `StateFilter` committed last week to make the view of the world consistent across address lookups.
There is a lot of test duplication here with StateFilter, but in Terraform style: we like it that way.
Mitchell Hashimoto