Next to adding the locking for the `state push` command, this commit also fixes a small bug where the lock would not be propertly released when running the `state show` command.
And finally it renames some variables in the `[un]taint` code in order to try to standardize the var names of a few frequently used variables (e.g. statemgr.Full, states.State, states.SyncState).
It's possible that a computed collection could be handled by the
attribute name, rather than the index count value.
Use a new testDiffFn for some tests, which don't work with the old
function that can't determine `computed` without the schema.
In a couple places in tests we execute a child "go build" to make a helper
program. Now that we're running in module mode, "go build" will normally
default to downloading and caching dependencies, which we don't want
because we're still using vendoring for the moment.
Therefore we need to instruct these child builds to use vendoring too,
avoiding the need to download all of the dependencies and ensuring that
we'll be building with the same dependencies that we'd use for a normal
build.
The main significant change here is that the package name for the proto
definition is "tfplugin5", which is important because this name is part
of the wire protocol for references to types defined in our package.
Along with that, we also move the generated package into "internal" to
make it explicit that importing the generated Go package from elsewhere is
not the right approach for externally-implemented SDKs, which should
instead vendor the proto definition they are using and generate their
own stubs to ensure that the wire protocol is the only hard dependency
between Terraform Core and plugins.
After this is merged, any provider binaries built against our
helper/schema package will need to be rebuilt so that they use the new
"tfplugin5" package name instead of "proto".
In a future commit we will include more elaborate and organized
documentation on how an external codebase might make use of our RPC
interface definition to implement an SDK, but the primary concern here
is to ensure we have the right wire package name before release.
There are some known broken links right now because this repository has
been updated ahead of some necessary changes in the terraform-website
repository. We'll need to wait until the end of the v0.12 release process
to re-enable this because the terraform-website repository is currently
set up for the v0.11 content and will continue to be until v0.12.0 final
is ready for release.
We're still using vendoring for now until we get _all_ of our tooling
updated, so the main idea here is to force use of the vendor directory
when running tests and building for development so we can quickly find
situations where we forget to run "go mod vendor".
We also setting GO111MODULE=off for installation of tools. Right now this
is the best way to install a tool in GOBIN without also interfering with
go.mod and go.sum, until a better pattern for managing tool dependencies
is devised by the Go team.
Finally, we run "go mod download" before launching "gox" in the main
build process, to prime the local module cache once so that the concurrent
"go build" processes won't race to populate it redundantly. This means
that we'll be producing final builds from the module cache rather than
from vendor as with everything else -- there's currently no way to tell
gox to use -mod=vendor -- but that should be fine in practice since
our go.sum file will ensure that we get the exact sources we expect in
the module cache before building.
Several of these tests rely on external services (e.g. Terraform Registry)
that have not yet been updated to support the needs of Terraform v0.12.0,
so for now we'll skip all of these tests and wait until those systems have
been updated.
This should be removed before Terraform v0.12.0 final to enable these
tests to be used as part of pre-release smoke testing.
This was broken by an earlier change to verify the Terraform version
number when reading a state file. To fix it, we'll use our current version
in our constructed file which should then match when it's read back in.
This was a mistake while adapting this code from the old state.LocalState.
Since the lock is held on the output file (s.path) the metadata should
live adjacent to that rather than being built from the read path
(s.readPath) that is used only as the initial snapshot on first
instantiation.
This also includes more logging, continuing the trend of other recent
commits in these files. The local state behavior is sufficiently complex
that these trace logs are a great help in debugging issues such as this
one with the wrong files being used or actions being taken in the wrong
order.
The local filesystem state manager no longer creates backup files eagerly,
instead creating them only if on first write there is already a snapshot
present in the target file.
Therefore for this test to exercise the codepaths it intends to we must
create an initial state snapshot for it to overwrite, creating the backup
in the process.
There are several other tests for this behavior elsewhere, so this test
is primarily to verify that the refresh command is configuring the backend
appropriately to get the backups written in the desired location.
We now only create a backup state file if the given output file already
exists, which it does not in this test.
(The behavior of creating the backup files is already covered by other
tests, so no need for this one go out of its way to do it.)
We now don't create a local state backup until the first snapshot write,
so we don't expect there to be a backup file until the end of the test.
(There is already a check at the end there, unmodified by this change.)
The filesystem backend has the option of using a different file for its
initial read.
Previously we were incorrectly writing the contents of that file out into
the backup file, rather than the prior contents of the output file. Now
we will always read the output file in RefreshState in order to decide
what we will back up but then we will optionally additionally read the
input file and prefer its content as the "current" state snapshot.
This is verified by command.TestMetaBackend_planLocalStatePath and
TestMetaBackend_configureNew, which are both now passing.
The changes to how we handle setting the state path on the local backend
broke the heuristic we were using here for detecting migration from one
local backend to another with the same state path, which would by default
end up deleting the state altogether after migration.
We now use the StatePaths method to do this, which takes into account
both the default values and any settings that have been set.
Additionally this addresses a flaw in the old method which could
potentially have deleted all non-default workspace state files if the
"path" setting were changed without also changing the "workspace_dir"
setting. This new approach is conservative because it will preserve all
of the files if any one overlaps.
In an earlier change we fixed the "backendFromConfig" codepath to be
able to properly detect changes to the -backend-config arguments during
"terraform init", but this detection is too strict for the normal case
of running an operation in a previously-initialized directory.
Before any of the recent changes, the logic here was to selectively update
the hash to include -backend-config settings in the init case. Since
that late hash recalculation was confusing, here we take the alternative
path of using the hash only in the normal case and full value comparison
in the init case. Treating both of these cases separately makes things
marginally easier to follow here.
The import command was imposing the default state path at the CLI level,
rather than leaving that to be handled by the backend. As a result, the
output state was always forced to be terraform.tfstate, regardless of
the backend settings.
This test is testing some strange implementation details of the old
local backend which do not hold with the new filesystem state manager.
Specifically, it was expecting state to be read from the stateOutPath
rather than the statePath, which makes no sense here because the backend
is configured to read from the default terraform.tfstate file (which does
not exist.)
There is another problem with this test which will be addressed in a
subsequent commit.
As part of integrating the new "remote" backend we relaxed the requirement
that a "default" workspace must exist in all backends and now skip
migrating empty workspace states to avoid creating unnecessary "default"
workspaces when switching between backends that require it and backends
that don't, such as when switching from the local backend (which always
has a "default" workspace) to Terraform Enterprise.
This was failing because we now handle the settings for the local backend
a little differently as a result of decoding it with the HCL2 machinery.
Specifically, the backend.State* fields are now assumed to be what is
given in configuration, and any CLI overrides are maintained separately
in OverrideState* fields so that they can be imposed "just in time" in
StatePaths.
This is particularly important because OverrideStatePath (when set) is
used regardless of workspace name, while StatePath is a suitable value
only for the "default" workspace, with others needing to be constructed
from StateWorkspaceDir instead.
Our new state model has a different implementation of "empty" that doesn't
consider lineage/serial, so we need to have some actual content in these
state fixtures to avoid them being skipped during state migrations.
We previously hacked around the import/export functionality being missing
in the statemgr layer after refactoring, but now it's been reintroduced
to fix functionality elsewhere we should use the centralized Import and
Export functions to ensure consistent behavior.
In particular, this pushes the logic for checking lineage and serial
during push down into the state manager itself, which is better because
all other details about lineage and serial are managed within the state
managers.
This test was initially failing because its fixture had a state which our
new state models consider to be "empty", and thus it was not migrated.
After fixing that (by adding an output to the fixture), this revealed a
bug that the lineage was not being persisted through the migration. This
is fixed by using the statemgr.Migrate method instead of writing via the
normal Writer interface, which allows two cooperating state managers to
properly transfer the lineage and serial along with the state snapshot.
In our recent refactoring of the state manager interfaces we made serial
and lineage management the responsibility of the state managers
themselves, not exposing them at all to most callers, and allowing for
simple state managers that don't implement them at all.
However, we do have some specific cases where we need to preserve these
properly when available, such as migration between backends, and the
"terraform state push" and "terraform state pull" commands.
These new functions and their associated optional interface allow the
logic here to be captured in one place and access via some simple
calls. Separating this from the main interface leaves things simple for
the normal uses of state managers.
Since these functions are mostly just thin wrappers around other
functionality, they are not yet well-tested directly, but will be
indirectly tested through the tests of their callers. A subsequent commit
will add more unit tests here.
This test was incorrectly updated in a previous iteration, with it
creating a modified state to write but then not actually writing it,
writing an empty test state instead.
This made the test fail because a backup state file is created only if
the new state snapshot is different to the old when written.
Terraform used to provide empty diffs to the provider when calculating
`ignore_changes`, which would cause some DiffSuppressFunc to fail, as
can be seen in #18209.
Verify that this is no longer the case in 0.12
In order to prevent mismatched states between read/plan/apply, we need
to ensure that the attributes are generated consistently each time.
Because of the various ways in which helper/schema and the hcl2 shims
interpret empty values, the only way to ensure consistency is to always
remove them altogether.
This makes sure the diff is generated with the matching set ids from
helper/schema.
Update the tests to add ID fields to the state, which will exists in
practice, since any state traversing through the shims will have the ID
inserted.
Sander van Harmelen