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 code does not appear to have any effect. The operation request has
its PlanOutBackend field populated directly in the Meta.Operation
method, by calling m.backendForState.ForPlan().
I tested a trivial null-resource config with a Consul backend, and the
saved plans with and without this code present were identical.
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.
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.
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.
Errors encountered when parsing flags for apply, plan, and refresh were
being suppressed. This resulted in a generic usage error when using an
invalid `-target` flag.
This commit makes several changes to address this. First, these commands
now output the flag parse error before exiting, leaving at least some
hint about the error. You can verify this manually with something like:
terraform apply -invalid-flag
We also change how target attributes are parsed, moving the
responsibility from the flags instance to the command. This allows us to
customize the diagnostic output to be more user friendly. The
diagnostics now look like:
```shellsession
$ terraform apply -no-color -target=foo
Error: Invalid target "foo"
Resource specification must include a resource type and name.
```
Finally, we add test coverage for both parsing of target flags, and at
the command level for successful use of resource targeting. These tests
focus on the UI output (via the change summary and refresh logs), as the
functionality of targeting is covered by the context tests in the
terraform package.
Several commands continued to support the legacy positional path
argument to specify a working directory. This functionality has been
replaced with the global -chdir flag, which is specified before any
other arguments, including the sub-command name.
This commit removes support for the trailing path parameter from
most commands. The only command which still supports a path argument is
fmt, which also supports "-" to indicate receiving configuration from
standard input.
Any invocation of a command with an invalid trailing path parameter will
result in a short error message, pointing at the -chdir alternative.
There are many test updates in this commit, almost all of which are
migrations from using positional arguments to specify a working
directory. Because of the layer at which these tests run, we are unable
to use the -chdir argument, so the churn in test files is larger than
ideal. Sorry!
The short description of our commands (as shown in the main help output
from "terraform") was previously very inconsistent, using different
tense/mood for different commands. Some of the commands were also using
some terminology choices inconsistent with how we currently talk about
the related ideas in our documentation.
Here I've tried to add some consistency by first rewriting them all in
the imperative mood (except the ones that just are just subcommand
groupings), and tweaking some of the terminology to hopefully gel better
with how we present similar ideas in our recently-updated docs.
While working on this I inevitably spotted some similar inconsistencies
in the longer-form help output of some of the commands. I've not reviewed
all of these for consistency, but I did update some where the wording
was either left inconsstent with the short form changes I'd made or
where the prose stood out to me as particularly inconsistent with our
current usual documentation language style.
All of this is subjective, so I expect we'll continue to tweak these over
time as we continue to develop our documentation writing style based on
user questions and feedback.
The workspace name can be overridden by setting a TF_WORKSPACE
environment variable. If this is done, we should still validate the
resulting workspace name; otherwise, we could end up with an invalid and
unselectable workspace.
This change updates the Meta.Workspace function to return an error, and
handles that error wherever necessary.
When warnings appear in isolation (not accompanied by an error) it's
reasonable to want to defer resolving them for a while because they are
not actually blocking immediate work.
However, our warning messages tend to be long by default in order to
include all of the necessary context to understand the implications of
the warning, and that can make them overwhelming when combined with other
output.
As a compromise, this adds a new CLI option -compact-warnings which is
supported for all the main operation commands and which uses a more
compact format to print out warnings as long as they aren't also
accompanied by errors.
The default remains unchanged except that the threshold for consolidating
warning messages is reduced to one so that we'll now only show one of
each distinct warning summary.
Full warning messages are always shown if there's at least one error
included in the diagnostic set too, because in that case the warning
message could contain additional context to help understand the error.
* docs: update plan command documentation. Fixes#19235
* docs: added a missing reserved variable name. Fixes#19159.
* website: add note that resource names cannot start with a number
* website: add some notes to the 0.12 upgrade guide
A lot of commands used `c.Meta.flagSet()` to create the initial flagset for the command, while quite a few of them didn’t actually use or support the flags that are then added.
So I updated a few commands to use `flag.NewFlagSet()` instead to only add the flags that are actually needed/supported.
Additionally this prevents a few commands from using locking while they actually don’t need locking (as locking is enabled as a default in `c.Meta.flagSet()`.
This connects a missing link left by earlier refactoring: the command
package is responsible for gathering up variable values provided by the
user and passing them through to the backend to use in operations.
In previous work we didn't quite connect these dots. The connection here
is sub-awesome since the existing interfaces here had some unfortunate
assumptions that we'd like to move away from (like the idea of a "nil
backend" implying the local backend) but we're accepting this for now to
avoid another big round of refactoring.
The main implication of this is that we will now always include a backend
configuration in the plan, though it might just be a placeholder config
for the local backend in the remaining cases where that's still implicitly
set. Later we will change this so that there is no implicit local backend
at all (terraform init is always required, _it_ will deal with setting
implicitly setting the local backend when appropriate), which will allow
us to rework this to be more straightforward and less "spooky".
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.
This is a rather-messy, complex change to get the "command" package
building again against the new backend API that was updated for
the new configuration loader.
A lot of this is mechanical rewriting to the new API, but
meta_config.go and meta_backend.go in particular saw some major
changes to interface with the new loader APIs and to deal with
the change in order of steps in the backend API.
In some cases this is needed to keep the UX clean and to make sure any remote exit codes are passed through to the local process.
The most obvious example for this is when using the "remote" backend. This backend runs Terraform remotely and stream the output back to the local terminal.
When an error occurs during the remote execution, all the needed error information will already be in the streamed output. So if we then return an error ourselves, users will get the same errors twice.
By allowing the backend to specify the correct exit code, the UX remains the same while preserving the correct exit codes.
If the user wishes to interrupt the running operation, only the first
interrupt was communicated to the operation by canceling the provided
context. A second interrupt would start the shutdown process, but not
communicate this to the running operation. This order of event could
cause partial writes of state.
What would happen is that once the command returns, the plugin system
would stop the provider processes. Once the provider processes dies, all
pending Eval operations would return return with an error, and quickly
cause the operation to complete. Since the backend code didn't know that
the process was shutting down imminently, it would continue by
attempting to write out the last known state. Under the right
conditions, the process would exit part way through the writing of the
state file.
Add Stop and Cancel CancelFuncs to the RunningOperation, to allow it to
easily differentiate between the two signals. The backend will then be
able to detect a shutdown and abort more gracefully.
In order to ensure that the backend is not in the process of writing the
state out, the command will always attempt to wait for the process to
complete after cancellation.
Previously we required callers to separately call .Validate on the root
module to determine if there were any value errors, but we did that
inconsistently and would thus see crashes in some cases where later code
would try to use invalid configuration as if it were valid.
Now we run .Validate automatically after config loading, returning the
resulting diagnostics. Since we return a diagnostics here, it's possible
to return both warnings and errors.
We return the loaded module even if it's invalid, so callers are free to
ignore returned errors and try to work with the config anyway, though they
will need to be defensive against invalid configuration themselves in
that case.
As a result of this, all of the commands that load configuration now need
to use diagnostic printing to signal errors. For the moment this just
allows us to return potentially-multiple config errors/warnings in full
fidelity, but also sets us up for later when more subsystems are able
to produce rich diagnostics so we can show them all together.
Finally, this commit also removes some stale, commented-out code for the
"legacy" (pre-0.8) graph implementation, which has not been available
for some time.
Now that the local backend can be cancelled during plan and refresh, we
don't really need the testShutdownHook. Simplify the tests by just
checking for Stop being called on the provider.
Add a shutdown hook to verify that a context has been correctly
cancelled, so we can remove the sleep and stop guessing.
Add a plan version of the shutdown test as well.
There was no cancellation context for a plan, so it would always have to
run to completion as SIGINT was being swallowed.
Move the shutdown channel to the command Meta since it's used in
multiple commands.
This uses the new diagnostics printer for config-related errors in the
main five commands that deal with config.
The immediate motivation for this is to allow HCL2-produced diagnostics
to be printed out in their full fidelity, though it also slightly changes
the presentation of other errors so that they are not presented in all
red text, which can be hard to read on some terminals.
Instead of providing the a path in BackendOpts, provide a loaded
*config.Config instead. This reduces the number of places where
configuration is loaded.
Add the -lock-timeout flag to the appropriate commands.
Add the -lock flag to `init` and `import` which were missing it.
Set both stateLock and stateLockTimeout in Meta.flagsSet, and remove the
extra references for clarity.
Implement debugInfo and the DebugGraph
DebugInfo will be a global variable through which graph debug
information can we written to a compressed archive. The DebugInfo
methods are all safe for concurrent use, and noop with a nil receiver.
The API outside of the terraform package will be to call SetDebugInfo
to create the archive, and CloseDebugInfo() to properly close the file.
Each write to the archive will be flushed and sync'ed individually, so
in the event of a crash or a missing call to Close, the archive can
still be recovered.
The DebugGraph is a representation of a terraform Graph to be written to
the debug archive, currently in dot format. The DebugGraph also contains
an internal buffer with Printf and Write methods to add to this buffer.
The buffer will be written to an accompanying file in the debug archive
along with the graph.
This also adds a GraphNodeDebugger interface. Any node implementing
`NodeDebug() string` can output information to annotate the debug graph
node, and add the data to the log. This interface may change or be
removed to provide richer options for debugging graph nodes.
The new graph builders all delegate the build to the BasicGraphBuilder.
Having a Name field lets us differentiate the actual builder
implementation in the debug graphs.
Alisdair McDiarmid