Something non-deterministic in the test suite is causing a coverage blip
in a line in the filesystem state manager. This commit adds a test which
specifically covers that line, which hopefully pleases the Codecov robot.
a large refactor to addrs.AbsProviderConfig, embedding the addrs.Provider instead of a Type string. I've added and updated tests, added some Legacy functions to support older state formats and shims, and added a normalization step when reading v4 (current) state files (not the added tests under states/statefile/roundtrip which work with both current and legacy-style AbsProviderConfig strings).
The remaining 'fixme' and 'todo' comments are mostly going to be addressed in a subsequent PR and involve looking up a given local provider config's FQN. This is fine for now as we are only working with default assumption.
* Introduce "Local" terminology for non-absolute provider config addresses
In a future change AbsProviderConfig and LocalProviderConfig are going to
become two entirely distinct types, rather than Abs embedding Local as
written here. This naming change is in preparation for that subsequent
work, which will also include introducing a new "ProviderConfig" type
that is an interface that AbsProviderConfig and LocalProviderConfig both
implement.
This is intended to be largely just a naming change to get started, so
we can deal with all of the messy renaming. However, this did also require
a slight change in modeling where the Resource.DefaultProviderConfig
method has become Resource.DefaultProvider returning a Provider address
directly, because this method doesn't have enough information to construct
a true and accurate LocalProviderConfig -- it would need to refer to the
configuration to know what this module is calling the provider it has
selected.
In order to leave a trail to follow for subsequent work, all of the
changes here are intended to ensure that remaining work will become
obvious via compile-time errors when all of the following changes happen:
- The concept of "legacy" provider addresses is removed from the addrs
package, including removing addrs.NewLegacyProvider and
addrs.Provider.LegacyString.
- addrs.AbsProviderConfig stops having addrs.LocalProviderConfig embedded
in it and has an addrs.Provider and a string alias directly instead.
- The provider-schema-handling parts of Terraform core are updated to
work with addrs.Provider to identify providers, rather than legacy
strings.
In particular, there are still several codepaths here making legacy
provider address assumptions (in order to limit the scope of this change)
but I've made sure each one is doing something that relies on at least
one of the above changes not having been made yet.
* addrs: ProviderConfig interface
In a (very) few special situations in the main "terraform" package we need
to make runtime decisions about whether a provider config is absolute
or local.
We currently do that by exploiting the fact that AbsProviderConfig has
LocalProviderConfig nested inside of it and so in the local case we can
just ignore the wrapping AbsProviderConfig and use the embedded value.
In a future change we'll be moving away from that embedding and making
these two types distinct in order to represent that mapping between them
requires consulting a lookup table in the configuration, and so here we
introduce a new interface type ProviderConfig that can represent either
AbsProviderConfig or LocalProviderConfig decided dynamically at runtime.
This also includes the Config.ResolveAbsProviderAddr method that will
eventually be responsible for that local-to-absolute translation, so
that callers with access to the configuration can normalize to an
addrs.AbsProviderConfig given a non-nil addrs.ProviderConfig. That's
currently unused because existing callers are still relying on the
simplistic structural transform, but we'll switch them over in a later
commit.
* rename LocalType to LocalName
Co-authored-by: Kristin Laemmert <mildwonkey@users.noreply.github.com>
When failing to write the state, the local backend writes the state to a local file called `errrored.tfstate`. Previously it would do so by creating a new state file which would use a new serial and lineage. By exorting the existing state file and directly assigning the new state, the serial and lineage are preserved.
This ensures that we test using the same source as we're using everywhere
else, and more tactically also ensures that when running in Travis-CI we
won't try to download all of the dependencies of Terraform during this
test.
In the long run we will look for a more global solution to this, rather
than adding this to all of our embedded "go" command calls directly, but
this is intended as a low-risk solution to get the build working again in
the mean time.
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 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.
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.
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.
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.
Due to how often the state and plan types are referenced throughout
Terraform, there isn't a great way to switch them out gradually. As a
consequence, this huge commit gets us from the old world to a _compilable_
new world, but still has a large number of known test failures due to
key functionality being stubbed out.
The stubs here are for anything that interacts with providers, since we
now need to do the follow-up work to similarly replace the old
terraform.ResourceProvider interface with its replacement in the new
"providers" package. That work, along with work to fix the remaining
failing tests, will follow in subsequent commits.
The aim here was to replace all references to terraform.State and its
downstream types with states.State, terraform.Plan with plans.Plan,
state.State with statemgr.State, and switch to the new implementations of
the state and plan file formats. However, due to the number of times those
types are used, this also ended up affecting numerous other parts of core
such as terraform.Hook, the backend.Backend interface, and most of the CLI
commands.
Just as with 5861dbf3fc49b19587a31816eb06f511ab861bb4 before, I apologize
in advance to the person who inevitably just found this huge commit while
spelunking through the commit history.
In practice these pairs of functions are often used together when working
with a "full" statemgr, so these helper wrappers allow us to do that more
conveniently.
This also introduces a new interface statemgr.Storage, which represents
a state manager that has all of the storage capabilities but does not
necessarily support locking. In practice callers will usually just use
statemgr.Full, but these more-specific interfaces allow us to reflect
in APIs which subset of the statemgr functionality each function depends
on.
In the old state package we had this as a separate manager
state.BackupState, but that doesn't work with our new interfaces because
we handle lineage and serial within the state managers themselves and
don't expose them to callers anymore.
In practice it being built in to the filesystem manager is not a problem
because we only use the backup functionality for local state anyway.
This also slightly adjusts the behavior to be more intuitive. The old
BackupState relied on the implementation detail that Terraform re-persists
the original state early in an apply operation, which meant that by
coincidence it would back up the right snapshot. In this new approach,
we instead take an in-memory copy during State and then write _that_ to
disk in WriteState if the new state seems different, so we're guaranteed
that we'll always write out what we read before any changes were made.
In future we may improve this further, such as keeping multiple
generations of backups, etc. But for now this is intended to preserve the
goals of the original implementation while making its behavior
self-contained and not dependent on coincidences.
This idea of a "state manager" was previously modelled via the
confusingly-named state.State interface, which we've been calling a "state
manager" only in some local variable names in situations where there were
also *terraform.State variables.
As part of reworking our state models to make room for the new type
system, we also need to change what was previously the state.StateReader
interface. Since we've found the previous organization confusing anyway,
here we just copy all of those interfaces over into statemgr where we can
make the relationship to states.State hopefully a little clearer.
This is not yet a complete move of the functionality from "state", since
we're not yet ready to break existing callers. In a future commit we'll
turn the interfaces in the old "state" package into aliases of the
interfaces in this package, and update all the implementers of what will
by then be statemgr.Reader to use *states.State instead of
*terraform.State.
This also includes an adaptation of what was previously state.LocalState
into statemgr.FileSystem, using the new state serialization functionality
from package statefile instead of the old terraform.ReadState and
terraform.WriteState.
Alisdair McDiarmid