* backend/remote: Filter environment variables when loading context
Following up on #23122, the remote system (Terraform Cloud or
Enterprise) serves environment and Terraform variables using a single
type of object. We only should load Terraform variables into the
Terraform context.
Fixes https://github.com/hashicorp/terraform/issues/23283.
During the Terraform 0.12 work we briefly had a partial update of the old
Terraform 0.11 (and prior) diff renderer that could work with the new
plan structure, but could produce only partial results.
We switched to the new plan implementation prior to release, but the
"terraform show" command was left calling into the old partial
implementation, and thus produced incomplete results when rendering a
saved plan.
Here we instead use the plan rendering logic from the "terraform plan"
command, making the output of both identical.
Unfortunately, due to the current backend architecture that logic lives
inside the local backend package, and it contains some business logic
around state and schema wrangling that would make it inappropriate to move
wholesale into the command/format package. To allow for a low-risk fix to
the "terraform show" output, here we avoid some more severe refactoring by
just exporting the rendering functionality in a way that allows the
"terraform show" command to call into it.
In future we'd like to move all of the code that actually writes to the
output into the "command" package so that the roles of these components
are better segregated, but that is too big a change to block fixing this
issue.
For remote operations, the remote system (Terraform Cloud or Enterprise)
writes the stored variable values into a .tfvars file before running the
remote copy of Terraform CLI.
By contrast, for operations that only run locally (like
"terraform import"), we fetch the stored variable values from the remote
API and add them into the set of available variables directly as part
of creating the local execution context.
Previously in the local-only case we were assuming that all stored
variables are strings, which isn't true: the Terraform Cloud/Enterprise UI
allows users to specify that a particular variable is given as an HCL
expression, in which case the correct behavior is to parse and evaluate
the expression to obtain the final value.
This also addresses a related issue whereby previously we were forcing
all sensitive values to be represented as a special string "<sensitive>".
That leads to type checking errors for any variable specified as having
a type other than string, so instead here we use an unknown value as a
placeholder so that type checking can pass.
Unpopulated sensitive values may cause errors downstream though, so we'll
also produce a warning for each of them to let the user know that those
variables are not available for local-only operations. It's a warning
rather than an error so that operations that don't rely on known values
for those variables can potentially complete successfully.
This can potentially produce errors in situations that would've been
silently ignored before: if a remote variable is marked as being HCL
syntax but is not valid HCL then it will now fail parsing at this early
stage, whereas previously it would've just passed through as a string
and failed only if the operation tried to interpret it as a non-string.
However, in situations like these the remote operations like
"terraform plan" would already have been failing with an equivalent
error message anyway, so it's unlikely that any existing workspace that
is being used for routine operations would have such a broken
configuration.
Some commands don't use variables at all or use them in a way that doesn't
require them to all be fully valid and consistent. For those, we don't
want to fetch variable values from the remote system and try to validate
them because that's wasteful and likely to cause unnecessary error
messages.
Furthermore, the variables endpoint in Terraform Cloud and Enterprise only
works for personal access tokens, so it's important that we don't assume
we can _always_ use it. If we do, then we'll see problems when commands
are run inside Terraform Cloud and Enterprise remote execution contexts,
where the variables map always comes back as empty.
The remote backend uses backend.ParseVariableValues locally only to decide
if the user seems to be trying to use -var or -var-file options locally,
since those are not supported for the remote backend.
Other than detecting those, we don't actually have any need to use the
results of backend.ParseVariableValues, and so it's better for us to
ignore any errors it produces itself and prefer to just send a
potentially-invalid request to the remote system and let the remote system
be responsible for validating it.
This then avoids issues caused by the fact that when remote operations are
in use the local system does not have all of the required context: it
can't see which environment variables will be set in the remote execution
context nor which variables the remote system will set using its own
generated -var-file based on the workspace stored variables.
Terraform Core expects all variables to be set, but for some ancillary
commands it's fine for them to just be set to placeholders because the
variable values themselves are not key to the command's functionality
as long as the terraform.Context is still self-consistent.
For such commands, rather than prompting for interactive input for
required variables we'll just stub them out as unknowns to reflect that
they are placeholders for values that a user would normally need to
provide.
This achieves a similar effect to how these commands behaved before, but
without the tendency to produce a slightly invalid terraform.Context that
would fail in strange ways when asked to run certain operations.
During the 0.12 work we intended to move all of the variable value
collection logic into the UI layer (command package and backend packages)
and present them all together as a unified data structure to Terraform
Core. However, we didn't quite succeed because the interactive prompts
for unset required variables were still being handled _after_ calling
into Terraform Core.
Here we complete that earlier work by moving the interactive prompts for
variables out into the UI layer too, thus allowing us to handle final
validation of the variables all together in one place and do so in the UI
layer where we have the most context still available about where all of
these values are coming from.
This allows us to fix a problem where previously disabling input with
-input=false on the command line could cause Terraform Core to receive an
incomplete set of variable values, and fail with a bad error message.
As a consequence of this refactoring, the scope of terraform.Context.Input
is now reduced to only gathering provider configuration arguments. Ideally
that too would move into the UI layer somehow in a future commit, but
that's a problem for another day.
Previously we were using the experimental HCL 2 repository, but now we'll
shift over to the v2 import path within the main HCL repository as part of
actually releasing HCL 2.0 as stable.
This is a mechanical search/replace to the new import paths. It also
switches to the v2.0.0 release of HCL, which includes some new code that
Terraform didn't previously have but should not change any behavior that
matters for Terraform's purposes.
For the moment the experimental HCL2 repository is still an indirect
dependency via terraform-config-inspect, so it remains in our go.sum and
vendor directories for the moment. Because terraform-config-inspect uses
a much smaller subset of the HCL2 functionality, this does still manage
to prune the vendor directory a little. A subsequent release of
terraform-config-inspect should allow us to completely remove that old
repository in a future commit.
* backend/remote-state/s3/backend_state.go: Prior to this commit, the terraform s3 backend did
not paginate calls to s3 when finding workspaces, which resulted in workspaces 'disappearing'
once they are switched away from, even though the state file still exists. This is due to the
ListBucket operation defaulting MaxItems to 1000, so terraform s3 backends that contained
more then 1000 workspaces did not function as expected. This rectifies this situation by
paginating calls to s3 when finding workspaces.
Signed-off-by: Collin J. Doering <collin@rekahsoft.ca>
Properly wait for cost estimation to finish running before outputting
the results. Waits 500 milliseconds between checks, rather than backing
off exponentially, because we are not in a run queue. At the point we're
waiting, we expect cost estimation to be run in a timely manner.
faster
The acceptance tests for etcdv3, oss and manta were not validating
required env variablea, chosing to assume that if one was running
acceptance tests they had already configured the credentials.
It was not always clear if this was a bug in the tests or the provider,
so I opted to make the tests fail faster when required attributes were
unset (or "").
The documentation for the -target option warns that it's intended for
exceptional circumstances only and not for routine use, but that's not a
very prominent location for that warning and so some users miss it.
Here we make the warning more prominent by including it directly in the
Terraform output when -target is in use. We first warn during planning
that the plan might be incomplete, and then warn again after apply
concludes and direct the user to run "terraform plan" to make sure that
there are no further changes outstanding. The latter message is intended
to reinforce that -target should only be a one-off operation and that you
should always run without it soon after to ensure that the workspace is
left in a consistent, converged state.
Previously, terraform was returning a potentially-misleading error
message in response to anything other than a 404 from the
b.client.Workspaces.Read operation. This PR simplifies Terraform's error
message with the intent of encouraging those who encounter it to focus
on the error message returned from the tfe client.
The added test is odd, and a bit hacky, and possibly overkill.
When a TFC workspace is configured without a VCS root, and with a
working directory, and a user is running `terraform init` from that same
directory, TFC uploads the entire configuration directory, not only the
user's cwd. This is not obvious to the user, so we are adding a descriptive
message explaining what is being uploaded, and why.
* backend/enhanced: start with absolute config path
We recently started normalizing the config path before all "command"
operations, which was necessary for consistency but had unexpected
consequences for remote backend operations, specifically when a vcs root
with a working directory are configured.
This PR de-normalizes the path back to an absolute path.
* Check the error and add a test
It turned out all required logic was already present, so I just needed to add a test for this specific use case.
Support for cross-domain authentication has been added and mapping
environment variables to the correct domain settings has been
fixed.
In addition, support for clouds.yaml files has been added.
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.
When changes are made and we failed to upload the state, we should not
try to unlock the workspace. Leaving the workspace locked is a good
indication something went wrong and also prevents other changes from
being applied before the newest state is properly uploaded.
Additionally we now output the lock ID when a lock or force-unlock
action failed.
When failing to write the state, the local backend writes the state to a local file called `errrored.tfstate`. Previously it would do so by creating a new state file which would use a new serial and lineage. By exorting the existing state file and directly assigning the new state, the serial and lineage are preserved.
For users who in previous versions have relied on our lack of checking for
whether variables are declared, they may previously have seen an
overwhelming number of warnings when running Terraform v0.12.
Here we cap that number at three specific warnings and then one general
warning, so we can still give a specific source location for the first
couple (for users who have genuinely made a typo) but summarize away a
large number for those who are seeing this because they've not yet
migrated to using environment variables.
This mirrors the change made for providers, so that default values can
be inserted into the config by the backend implementation. This is only
the interface and method name changes, it does not yet add any default
values.
Previously we checked can-update in order to determine if a user had the
required permissions to apply a run, but that wasn't sufficient. So we
added a new permission, can-queue-apply, that we now use instead.
The handling of slashes was broken around listing workspaces in
workspace_key_prefix. While it worked in most places by splitting an
extra time around the spurious slashes, it failed in the case that the
prefix ended with a slash of its own.
A test was temporarily added to verify that the backend works with the
unusual keys, but rather than risking silent breakage around prefixes
with trailing slashes, we also add validation to prevent users from
entering keys with trailing slashes at all.
The init error was output deep in the backend by detecting a
special ResourceProviderError and formatted directly to the CLI.
Create some Diagnostics closer to where the problem is detected, and
passed that back through the normal diagnostic flow. While the output
isn't as nice yet, this restores the helpful error message and makes the
code easier to maintain. Better formatting can be handled later.
The API surface area is much smaller when we use the remote backend for remote state only.
So in order to try and prevent any backwards incompatibilities when TF runs inside of TFE, we’ve split up the discovery services into `state.v2` (which can be used for remote state only configurations, so when running in TFE) and `tfe.v2.1` (which can be used for all remote configurations).
This changes the contract for `PlanResourceChange` so that the provider is now responsible
for populating all default values during plan, including inserting any unknown values for
defaults it will fill in at apply time.
We've changed the contract for PlanResourceChange to now require the
provider to populate any default values (including unknowns) it wants to
set for computed arguments, so our mock provider here now needs to be a
little more complex to deal with that.
This fixes several of the tests in this package. A minor change to
TestLocal_applyEmptyDirDestroy was required to make it properly configure
the mock provider so PlanResourceChange can access the schema.
In Terraform 0.11 and earlier we just silently ignored undeclared
variables in -var-file and the automatically-loaded .tfvars files. This
was a bad user experience for anyone who made a typo in a variable name
and got no feedback about it, so we made this an error for 0.12.
However, several users are now relying on the silent-ignore behavior for
automation scenarios where they pass the same .tfvars file to all
configurations in their organization and expect Terraform to ignore any
settings that are not relevant to a specific configuration. We never
intentionally supported that, but we don't want to immediately break that
workflow during 0.12 upgrade.
As a compromise, then, we'll make this a warning for v0.12.0 that contains
a deprecation notice suggesting to move to using environment variables
for this "cross-configuration variables" use-case. We don't produce errors
for undeclared variables in environment variables, even though that
potentially causes the same UX annoyance as ignoring them in vars files,
because environment variables are assumed to live in the user's session
and this it would be very inconvenient to have to unset such variables
when moving between directories. Their "ambientness" makes them a better
fit for these automatically-assigned general variable values that may or
may not be used by a particular configuration.
This can revert to being an error in a future major release, after users
have had the opportunity to migrate their automation solutions over to
use environment variables.
We don't seem to have any tests covering this specific situation right
now. That isn't ideal, but this change is so straightforward that it would
be relatively expensive to build new targeted test cases for it and so
I instead just hand-tested that it is indeed now producing a warning where
we were previously producing an error. Hopefully if there is any more
substantial work done on this codepath in future that will be our prompt
to add some unit tests for this.
The AWS Go SDK automatically provides a default request retryer with exponential backoff that is invoked via setting `MaxRetries` or leaving it `nil` will default to 3. The terraform-aws-provider `config.Client()` sets `MaxRetries` to 0 unless explicitly configured above 0. Previously, we were not overriding this behavior by setting the configuration and therefore not invoking the default request retryer.
The default retryer already handles HTTP error codes above 500, including S3's InternalError response, so the extraneous handling can be removed. This will also start automatically retrying many additional cases, such as temporary networking issues or other retryable AWS service responses.
Changes:
* s3/backend: Add `max_retries` argument
* s3/backend: Enhance S3 NoSuchBucket error to include additional information
* Upgrading to 2.0.0 of github.com/hashicorp/go-azure-helpers
* Support for authenticating using Azure CLI
* backend/azurerm: support for authenticating using the Azure CLI
This PR improves the error handling so we can provide better feedback about any service discovery errors that occured.
Additionally it adds logic to test for specific versions when discovering a service using `service.vN`. This will enable more informational errors which can indicate any version incompatibilities.
Martin Atkins