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 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.
It's pretty common to want to apply the various fmt.Fprint... functions
to our two output streams, and so to make that much less noisy at the
callsite here we have a small number of very thin wrappers around the
underlying fmt package functionality.
Although we're aiming to not have too much abstraction in this "terminal"
package, this seems justified in that it is only a very thin wrapper
around functionality that most Go programmers are already familiar with,
and so the risk of this causing any surprises is low and the improvement
to readability of callers seems worth it.
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.
Add validation which was removed from the configload package, along with
additional validation checks. The output is slightly different, as
instead of validating whether the modules are allowed to have provider
configurations, we validate the various combinations of provider
structures themselves.
This is to allow convenient testing of functions that are designed to work
directly with *terminal.Streams or the individual stream objects inside.
Because the InputStream and OutputStream APIs expose directly an *os.File,
this does some extra work to set up OS-level pipes so we can capture the
output into local buffers to make test assertions against. The idea here
is to keep the tricky stuff we need for testing confined to the test
codepaths, so that the "real" codepaths don't end up needing to work
around abstractions that are otherwise unnecessary.
The configload package should only be responsible for locating and
loading the configuration, and not be further inspecting the config
source itself. Moving the validating into the configs package.
Add support for parsing configuration_aliases in required_providers
entries. The decoder needed to be re-written here in order to support
the bare reference style usage of provider names so that they match the
usage in other location within configuration. The only change to
existing handling of the required_providers block is more precise error
locations in a couple cases.
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.
Right now, there's a bug that if a diagnostic comes back from the
provider with an AttributePath set, but no steps in the AttributePath,
Terraform _thinks_ it's an attribute-specific diagnostic and not a
whole-resource diagnostic, but then doesn't associate it with any
specific attribute, meaning the diagnostic doesn't get associated with
the config at all.
This PR changes things to check if there are any steps in the
AttributePath before deciding this isn't a whole-resource diagnostic,
and if there aren't, treats it as a whole-resource diagnostic, instead.
See hashicorp/terraform-plugin-sdk#561 for more details on how this
surfaces in the wild.
The previous changes removing support for using the trailing positional
argument as a working directory missed a spot in the apply/destroy
command implementation. We still support this argument for applying a
saved plan:
terraform apply foo.tfplan
However, if you pass a positional path which doesn't "look like" a plan
(for example, the path to a configuration directory), Terraform would
silently ignore it and continue.
This commit fixes that by adding an error message if the user specifies
a path which the plan loader rejects as not "looking like" a plan. This
message includes a reference to the `-chdir` flag as a pointer about
what to do next.
We also rearrange the error message when calling `terraform destroy`
with a plan file argument, and add test coverage for the above. While
we're here, update the destroy tests to copy the fixture directory,
chdir, and defer cleanup.
This dramatically simplifies the logic around auto-approve, which is
nice.
Also add test coverage for the manual approve step, for both apply and
destroy, answering both yes and no.
To make the command arguments easier to understand and extend, we are
moving away from positional arguments. This commit changes the graph
command to take a `-plan` flag instead of an optional trailing path.
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!
Alisdair McDiarmid