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.
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.
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!
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.
The -module flag to terraform output has been unimplemented since 0.12.
This commit removes some dead code and the specific error message for
this flag.
The website documentation for output does not mention this flag, so it
is unchanged.
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.
For normal provider installation we want to associate each provider with
a selected version number and find a suitable package for that version
that conforms to the official hashes for that release.
Those requirements are very onerous for a provider developer currently
testing a not-yet-released build, though. To allow for that case this new
CLI configuration feature allows overriding specific providers to refer
to give local filesystem directories.
Any provider overridden in this way is not subject to the usual
restrictions about selected versions or checksum conformance, and
activating an override won't cause any changes to the selections recorded
in the lock file because it's intended to be a temporary setting for one
developer only.
This is, in a sense, a spiritual successor of an old capability we had to
override specific plugins in the CLI configuration file. There were
some vestiges of that left in the main package and CLI config package
but nothing has actually been honoring them for several versions now and
so this commit removes them to avoid confusion with the new mechanism.
Use a slightly modified value renderer from terraform-provider-testing
to display values in the console REPL, as well as outputs from the apply
and outputs subcommands.
Derived from code in this repository, MIT licensed:
https://github.com/apparentlymart/terraform-provider-testing
Note that this is technically a breaking change for the console
subcommand, which would previously error if the user attempted to render
an unknown value (such as an unset variable). This was marked as an
unintentional side effect, with the goal being the new behaviour of
rendering "(unknown)", which is why I changed the behaviour in this
commit.
The positional argument passed to apply was once used to specify a
source for a Terraform module to fetch and initialize (#337). This
functionality was removed from the init command later (#15032) but not
completely removed from apply.
This code was non-functional but largely not harmful, except for a very
specific case: when passing an absolute path to a plan file as the
positional argument on Windows, the getter.Detect code would incorrectly
interpret the path as a URL. This caused init to fail and the apply
command would exit with code 1 but without diagnostics.
This commit removes this codepath, which fixes this bug, and should
otherwise have no effect on the supported behaviour of apply.
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.
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 reinstates an old behavior that was lost in the reorganization of how
we deal with the -var and -var-file options.
This fix is verified by TestApply_planVars now passing.
We temporarily disabled this because it needed some further work to update
it for the new state models, which has now been done.
We no longer need the configuration objects for the outputs because the
state itself contains all of the information needed for displaying these.
We previously stubbed most of this out because it hadn't yet been updated
to support the new state types, etc.
This restores all of the previous behavior as covered by the tests.
We intentionally remove one behavior that was not covered by the tests:
we used to allow retrieval of outputs from non-root modules using the
-module option, but since we no longer persist non-root outputs in the
state we can no longer support this without a full expression evaluation
walk, and that'd be overkill for this otherwise-simple command. Descendant
module outputs are not part of the public interface of a configuration
anyway, so accessing them from outside in this way is an anti-pattern.
(For debugging scenarios it is still possible to access these from
"terraform console", which _does_ do a full evaluation graph walk to
prepare its evaluation scope.)
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.
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.
Due to how deeply the configuration types go into Terraform Core, there
isn't a great way to switch out to HCL2 gradually. As a consequence, this
huge commit gets us from the old state to a _compilable_ new state, but
does not yet attempt to fix any tests and has a number of known missing
parts and bugs. We will continue to iterate on this in forthcoming
commits, heading back towards passing tests and making Terraform
fully-functional again.
The three main goals here are:
- Use the configuration models from the "configs" package instead of the
older models in the "config" package, which is now deprecated and
preserved only to help us write our migration tool.
- Do expression inspection and evaluation using the functionality of the
new "lang" package, instead of the Interpolator type and related
functionality in the main "terraform" package.
- Represent addresses of various objects using types in the addrs package,
rather than hand-constructed strings. This is not critical to support
the above, but was a big help during the implementation of these other
points since it made it much more explicit what kind of address is
expected in each context.
Since our new packages are built to accommodate some future planned
features that are not yet implemented (e.g. the "for_each" argument on
resources, "count"/"for_each" on modules), and since there's still a fair
amount of functionality still using old-style APIs, there is a moderate
amount of shimming here to connect new assumptions with old, hopefully in
a way that makes it easier to find and eliminate these shims later.
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.
Since an early version of Terraform, the `destroy` command has always
had the `-force` flag to allow an auto approval of the interactive
prompt. 0.11 introduced `-auto-approve` as default to `false` when using
the `apply` command.
The `-auto-approve` flag was introduced to reduce ambiguity of it's
function, but the `-force` flag was never updated for a destroy.
People often use wrappers when automating commands in Terraform, and the
inconsistency between `apply` and `destroy` means that additional logic
must be added to the wrappers to do similar functions. Both commands are
more or less able to run with similar syntax, and also heavily share
their code.
This commit updates the command in `destroy` to use the `-auto-approve` flag
making working with the Terraform CLI a more consistent experience.
We leave in `-force` in `destroy` for the time-being and flag it as
deprecated to ensure a safe switchover period.
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.
In the 0.10 release we added an opt-in mode where Terraform would prompt
interactively for confirmation during apply. We made this opt-in to give
those who wrap Terraform in automation some time to update their scripts
to explicitly opt out of this behavior where appropriate.
Here we switch the default so that a "terraform apply" with no arguments
will -- if it computes a non-empty diff -- display the diff and wait for
the user to type "yes" in similar vein to the "terraform destroy" command.
This makes the commonly-used "terraform apply" a safe workflow for
interactive use, so "terraform plan" is now mainly for use in automation
where a separate planning step is used. The apply command remains
non-interactive when given an explicit plan file.
The previous behavior -- though not recommended -- can be obtained by
explicitly setting the -auto-approve option on the apply command line,
and indeed that is how all of the tests are updated here so that they can
continue to run non-interactively.
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.
A common reason to want to use `terraform plan` is to have a chance to
review and confirm a plan before running it. If in fact that is the
only reason you are running plan, this new `terraform apply -auto-approve=false`
flag provides an easier alternative to
P=$(mktemp -t plan)
terraform refresh
terraform plan -refresh=false -out=$P
terraform apply $P
rm $P
The flag defaults to true for now, but in a future version of Terraform it will
default to false.
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.
Alisdair McDiarmid