Previously we were repeating some logic in the UI layer in order to
recover relevant additional context about a change to report to a user.
In order to help keep things consistent, and to have a clearer path for
adding more such things in the future, here we capture this user-facing
idea of an "action reason" within the plan model, and then use that
directly in order to decide how to describe the change to the user.
For the moment the "tainted" situation is the only one that gets a special
message, matching what we had before, but we can expand on this in future
in order to give better feedback about the other replace situations too.
This also preemptively includes the "replacing by request" reason, which
is currently not reachable but will be used in the near future as part of
implementing the -replace=... plan command line option to allow forcing
a particular object to be replaced.
So far we don't have any special reasons for anything other than replacing,
which makes sense because replacing is the only one that is in a sense
a special case of another action (Update), but this could expand to
other kinds of reasons in the future, such as explaining which of the
few different reasons a data source read might be deferred until the
apply step.
So far we've only had "normal mode" and "destroy mode", where the latter
is activated either by "terraform plan -destroy" or "terraform destroy".
In preparation for introducing a third mode "refresh only" this
generalizes how we handle modes so we can potentially deal with an
arbitrary number of modes, although for now we only intend to have three.
Mostly this is just a different implementation of the same old behavior,
but there is one small user-visible difference here: the "terraform apply"
command now accepts a -destroy option, mirroring the option of the same
name on "terraform plan", which in turn makes "terraform destroy"
effectively a shorthand for "terraform apply -destroy".
This is intended to make us consistent that "terraform apply" without a
plan file argument accepts all of the same plan-customization options that
"terraform plan" does, which will in turn avoid us having to add a new
alias of "terraform plan" for each new plan mode we might add. The -help
output is changed in that vein here, although we'll wait for subsequent
commit to make a similar change to the website documentation just so we
can deal with the "refresh only mode" docs at the same time.
We previously had a shallow IsMarked call in compactValueStr's caller but
then a more-conservative deep ContainsMarked call inside compactValueStr
with a different resulting message. As well as causing an inconsistency
in messages, this was also a bit confusing because it made it seem like
a non-sensitive collection containing a sensitive element value was wholly
sensitive, making the debug information in the diagnostic messages not
trustworthy for debugging certain varieties of problem.
I originally considered just removing the redundant check in
compactValueStr here, but ultimately I decided to keep it as a sort of
defense in depth in case a future refactoring disconnects these two
checks. This should also serve as a prompt to someone making later changes
to compactValueStr to think about the implications of sensitive values
in there, which otherwise wouldn't be mentioned at all.
Disclosing information about a collection containing sensitive values is
safe here because compactValueStr only discloses information about the
value's type and element keys, and neither of those can be sensitive in
isolation. (Constructing a map with sensitive keys reduces to a sensitive
overall map.)
This commit adds a comprehensive JSON format for diagnostics, which
ensures that all current diagnostic output can be semantically
represented in a machine-readable format. The diagnostic formatter
interface remains unchanged, but it first transforms its input via the
JSON format to ensure that there is only one code path for creating the
diagnostic data.
The JSON diagnostic renderer extracts the non-presentational logic from
the format package, and returns a structure which can either be
marshaled into JSON or rendered as text. The resulting text diagnostic
output is unchanged for all cases covered by unit tests and my own
manual testing.
Included in this commit are a number of golden reference files for the
marshaled JSON output of a diagnostic. This format should change rarely
if at all, and these are in place to ensure that any changes to the
format are intentional and considered.
The previous implementation of views was copying and embedding the base
View struct in each individual view. While this allowed for easy access
to the interface of that struct (both in the view and externally), it more
importantly completely broke the ability of the diagnostic printer to
output source code snippets.
This is because the `configSources` field on the base view is lazily set
after the config loader is initialized. In the commands ported to use
views, this happens after the base View struct is copied, so we are
updating the wrong copy of the struct.
This commit fixes this with a simple mechanical refactor: keep a pointer
to the base View struct instead, and update all of the individual views
to explicitly refer to that struct to access its fields and methods.
This is not a particularly satisfying solution, but I can't find
anything clearly better. It might be worth exploring the alternative
approach in the view for the new test command, which explicitly pulls
its dependencies out of the base view, rather than retaining a full
reference. Maybe there's a third way which is better still.
This is just a prototype to gather some feedback in our ongoing research
on integration testing of Terraform modules. The hope is that by having a
command integrated into Terraform itself it'll be easier for interested
module authors to give it a try, and also easier for us to iterate quickly
based on feedback without having to coordinate across multiple codebases.
Everything about this is subject to change even in future patch releases.
Since it's a CLI command rather than a configuration language feature it's
not using the language experiments mechanism, but generates a warning
similar to the one language experiments generate in order to be clear that
backward compatibility is not guaranteed.
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.
Due to calling the Colorize function with the full string instead of the
format string, plan/apply logs which include resource instance keys or
IDs which happen to match color formatting would be rendered
incorrectly.
This commit fixes this by only colorizing the known-safe format string.
We also add full test coverage for the UI hook, although only one of the
hooks is tested for this color bugfix due to verbosity of the test.
We also add the bold coloring to the provisioner output prefix, which
seems to have been an oversight.
Now that the view code is separated, we can increase test coverage in
unit tests. This commit moves some tests from the command package which
were testing only view code, and adds more new test cases.
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.
Move the code which renders Terraform hook callbacks as UI into the
views package, backed by a views.View instead of a cli.Ui. Update test
setup accordingly.
To allow commands to control this hook, we add a hooks member on the
backend Operation struct. This supersedes the hooks in the Terraform
context, which is not directly controlled by the command logic.
This commit should not change how Terraform works, and is refactoring in
preparation for more changes which move UI code out of the backend.
Rather than modifying and relying on the existing Meta.process
argument extractor, we can more clearly handle global CLI flags using
a separate parser step. This allows us to explicitly configure the view
in the command.
Terraform supports multiple output formats for several sub-commands.
The default format is user-readable text, but many sub-commands support
a `-json` flag to output a machine-readable format for the result. The
output command also supports a `-raw` flag for a simpler, scripting-
focused machine readable format.
This commit adds a "views" abstraction, intended to help ensure
consistency between the various output formats. This extracts the render
specific code from the command package, and moves it into a views
package. Each command is expected to create an interface for its view,
and one or more implementations of that interface.
By doing so, we separate the concerns of generating the sub-command
result from rendering the result in the specified output format. This
should make it easier to ensure that all output formats will be updated
together when changes occur in the result-generating phase.
There are some other consequences of this restructuring:
- Views now directly access the terminal streams, rather than the
now-redundant cli.Ui instance;
- With the reorganization of commands, parsing CLI arguments is now the
responsibility of a separate "arguments" package.
For now, views are added only for the output sub-command, as an example.
Because this command uses code which is shared with the apply and
refresh commands, those are also partially updated.
Alisdair McDiarmid