This commit extracts the remaining UI logic from the local backend,
and removes access to the direct CLI output. This is replaced with an
instance of a `views.Operation` interface, which codifies the current
requirements for the local backend to interact with the user.
The exception to this at present is interactivity: approving a plan
still depends on the `UIIn` field for the backend. This is out of scope
for this commit and can be revisited separately, at which time the
`UIOut` field can also be removed.
Changes in support of this:
- Some instances of direct error output have been replaced with
diagnostics, most notably in the emergency state backup handler. This
requires reformatting the error messages to allow the diagnostic
renderer to line-wrap them;
- The "in-automation" logic has moved out of the backend and into the
view implementation;
- The plan, apply, refresh, and import commands instantiate a view and
set it on the `backend.Operation` struct, as these are the only code
paths which call the `local.Operation()` method that requires it;
- The show command requires the plan rendering code which is now in the
views package, so there is a stub implementation of a `views.Show`
interface there.
Other refactoring work in support of migrating these commands to the
common views code structure will come in follow-up PRs, at which point
we will be able to remove the UI instances from the unit tests for those
commands.
The clistate package includes a Locker interface which provides a simple
way for the local backend to lock and unlock state, while providing
feedback to the user if there is a delay while waiting for the lock.
Prior to this commit, the backend was responsible for initializing the
Locker, passing through direct access to the cli.Ui instance.
This structure prevented commands from implementing different
implementations of the state locker UI. In this commit, we:
- Move the responsibility of creating the appropriate Locker to the
source of the Operation;
- Add the ability to set the context for a Locker via a WithContext
method;
- Replace the Locker's cli.Ui and Colorize members with a StateLocker
view;
- Implement views.StateLocker for human-readable UI;
- Update the Locker interface to return detailed diagnostics instead of
errors, reducing its direct interactions with UI;
- Add a Timeout() method on Locker to allow the remote backend to
continue to misuse the -lock-timeout flag to cancel pending runs.
When an Operation is created, the StateLocker field must now be
populated with an implementation of Locker. For situations where locking
is disabled, this can be a no-op locker.
This change has no significant effect on the operation of Terraform,
with the exception of slightly different formatting of errors when state
locking or unlocking fails.
The enhanced backends (local and remote) need to be able to render
diagnostics during operations. Prior to this commit, this functionality
was supported with a per-backend `ShowDiagnostics` function pointer.
In order to allow users of these backends to control how diagnostics are
rendered, this commit moves that function pointer to the `Operation`
type. This means that a diagnostic renderer is configured for each
operation, rather than once per backend initialization.
Some secondary consequences of this change:
- The `ReportResult` method on the backend is now moved to the
`Operation` type, as it needs to access the `ShowDiagnostics` callback
(and nothing else from the backend);
- Tests which assumed that diagnostics would be written to the backend's
`cli.Ui` instance are migrated to using a new record/playback diags
helper function;
- Apply, plan, and refresh commands now pass a pointer to the `Meta`
struct's `showDiagnostics` method.
This commit should not change how Terraform works, and is refactoring in
preparation for more changes which move UI code out of the backend.
CountHook is an implementation of terraform.Hook which is used to
calculate how many resources were added, changed, or destroyed during an
apply. This hook was previously injected in the local backend code,
which means that the apply command code has no access to these counts.
This commit moves the CountHook code into the command package, and
removes an unused instance of the hook in the plan code path. The goal
here is moving UI code into the command package.
We now require the output to accept UTF-8 and we can determine how wide
the terminal (if any) is, so here we begin to make use of that for the
"terraform plan" command.
The horizontal rule is now made of box drawing characters instead of
hyphens and fills the whole terminal width.
The paragraphs of text in the output are now also wrapped to fill the
terminal width, instead of the hard-wrapping we did before.
This is just a start down the road of making better use of the terminal
capabilities. Lots of other commands could benefit from updates like these
too.
When rendering a diff between current state and projected state, we only
show resources and outputs which have changes. However, we show a full
structural diff for these values, which includes all attributes and
blocks for a changed resource or output. The result can be a very long
diff, which makes it difficult to verify what the changed fields are.
This commit adds an experimental concise diff renderer, which suppresses
most unchanged fields, only displaying the most relevant changes and
some identifying context. This means:
- Always show all identifying attributes, initially defined as `id`,
`name`, and `tags`, even if unchanged;
- Only show changed, added, or removed primitive values: `string`,
`number`, or `bool`;
- Only show added or removed elements in unordered collections and
structural types: `map`, `set`, and `object`;
- Show added or removed elements with any surrounding unchanged elements
for sequence types: `list` and `tuple`;
- Only show added or removed nested blocks, or blocks with changed
attributes.
If any attributes, collection elements, or blocks are hidden, a count
is kept and displayed at the end of the parent scope. This ensures that
it is clear that the diff is only displaying a subset of the resource.
The experiment is currently enabled by default, but can be disabled by
setting the TF_X_CONCISE_DIFF environment variable to 0.
* unlock the state if Context() has an error, exactly as backend/remote does today
* terraform console and terraform import will exit before unlocking state in case of error in Context()
* responsibility for unlocking state in the local backend is pushed down the stack, out of backend.go and into each individual state operation
* add tests confirming that state is not locked after apply and plan
* backend/local: add checks that the state is unlocked after operations
This adds tests to plan, apply and refresh which validate that the state
is unlocked after all operations, regardless of exit status. I've also
added specific tests that force Context() to fail during each operation
to verify that locking behavior specifically.
This is a baby-step towards an intended future where all Terraform actions
which have side-effects in either remote objects or the Terraform state
can go through the plan+apply workflow.
This initial change is focused only on allowing plan+apply for changes to
root module output values, so that these can be written into a new state
snapshot (for consumption by terraform_remote_state elsewhere) without
having to go outside of the primary workflow by running
"terraform refresh".
This is also better than "terraform refresh" because it gives an
opportunity to review the proposed changes before applying them, as we're
accustomed to with resource changes.
The downside here is that Terraform Core was not designed to produce
accurate changesets for root module outputs. Although we added a place for
it in the plan model in Terraform 0.12, Terraform Core currently produces
inaccurate changesets there which don't properly track the prior values.
We're planning to rework Terraform Core's evaluation approach in a
forthcoming release so it would itself be able to distinguish between the
prior state and the planned new state to produce an accurate changeset,
but this commit introduces a temporary stop-gap solution of implementing
the logic up in the local backend code, where we can freeze a snapshot of
the prior state before we take any other actions and then use that to
produce an accurate output changeset to decide whether the plan has
externally-visible side-effects and render any changes to output values.
This temporary approach should be replaced by a more appropriately-placed
solution in Terraform Core in a release, which should then allow further
behaviors in similar vein, such as user-visible drift detection for
resource instances.
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>
* huge change to weave new addrs.Provider into addrs.ProviderConfig
* terraform: do not include an empty string in the returned Providers /
Provisioners
- Fixed a minor bug where results included an extra empty string
This unusual situation isn't supposed to arise in normal use, but it can
come up in practice in some edge-case scenarios where Terraform fails in
a severe way during a create_before_destroy.
Some earlier versions of Terraform also had bugs in their handling of
deposed objects, so this may also arise if upgrading from one of those
older versions with some leftover deposed objects in the state.
If an instance object in state has an earlier schema version number then
it is likely that the schema we're holding won't be able to decode the
raw data that is stored. Instead, we must ask the provider to upgrade it
for us first, which might also include translating it from flatmap form
if it was last updated with a Terraform version earlier than v0.12.
This ends up being a "seam" between our use of int64 for schema versions
in the providers package and uint64 everywhere else. We intend to
standardize on int64 everywhere eventually, but for now this remains
consistent with existing usage in each layer to keep the type conversion
noise contained here and avoid mass-updates to other Terraform components
at this time.
This also includes a minor change to the test helpers for the
backend/local package, which were inexplicably setting a SchemaVersion of
1 on the basic test state but setting the mock schema version to zero,
creating an invalid situation where the state would need to be downgraded.
We can't generate a valid plan file without a backend configuration to
write into it, but it's the responsibility of the caller (the command
package) to manage the backend configuration mechanism, so we require it
to tell us what to write here.
This feels a little strange because the backend in principle knows its
own config, but in practice the backend only knows the _processed_ version
of the config, not the raw configuration value that was used to configure
it.
converted the existing testPlanState() from terraform.State to
states.State to fix various plan tests.
reverted the "bandaid" in plans/planfile/tfplan.go - at this moment the
backend tests do not include backend configuration, and so the planfile
package can write the plan file but not read it back in. That will be
revisted in a separate track of work.
I have no confidence in the change to plans/planfile/tfplan.go. The
tests were passing an empty backend config, which planfile was able to
write to a file but not read from the same file. This change let me move
past that and it did not break any tests in the planfile package, but I
am concerned that it introduces undesired behavior.
incoming values
Addresses an odd state where the priorV of an object to be changed is
known but null.
While this situation should not happen, it seemed prudent to ensure that
core is resilient to providers sending incorrect values (which might
also occur with manually edited state).
The "config" package is no longer used and will be removed as part
of the 0.12 release cleanup. Since configschema is part of the
"new world" of configuration modelling, it makes more sense for
it to live as a subdirectory of the newer "configs" package.
The new config loader requires some steps to happen in a different
order, particularly in regard to knowing the schema in order to
decode the configuration.
Here we lean directly on the configschema package, rather than
on helper/schema.Backend as before, because it's generally
sufficient for our needs here and this prepares us for the
helper/schema package later moving out into its own repository
to seed a "plugin SDK".
In #15884 we adjusted the plan output to give an explicit command to run
to apply a plan, whereas before this command was just alluded to in the
prose.
Since releasing that, we've got good feedback that it's confusing to
include such instructions when Terraform is running in a workflow
automation tool, because such tools usually abstract away exactly what
commands are run and require users to take different actions to
proceed through the workflow.
To accommodate such environments while retaining helpful messages for
normal CLI usage, here we introduce a new environment variable
TF_IN_AUTOMATION which, when set to a non-empty value, is a hint to
Terraform that it isn't being run in an interactive command shell and
it should thus tone down the "next steps" messaging.
The documentation for this setting is included as part of the "...in
automation" guide since it's not generally useful in other cases. We also
intentionally disclaim comprehensive support for this since we want to
avoid creating an extreme number of "if running in automation..."
codepaths that would increase the testing matrix and hurt maintainability.
The focus is specifically on the output of the three commands we give in
the automation guide, which at present means the following two situations:
* "terraform init" does not include the final paragraphs that suggest
running "terraform plan" and tell you in what situations you might need
to re-run "terraform init".
* "terraform plan" does not include the final paragraphs that either
warn about not specifying "-out=..." or instruct to run
"terraform apply" with the generated plan file.
We are changing the behaviour of the "stub" diff operation to just have
the pre/post-diff hooks skipped on eval, meaning that the test against
CountHook will ultimately be meaningless and fail, hence we need a
different test here that tests it on a more general level.
Fixes#11504
The local backend should error if `terraform plan` is called in a
directory with no Terraform config files (same behavior as 0.8.x).
**New behavior:** We now allow `terraform plan -destroy` with no
configuration files since that seems reasonable.
The local backend implementation is an implementation of
backend.Enhanced that recreates all the behavior of the CLI but through
the backend interface.
Alisdair McDiarmid