backend/remote: Report invalid variables only remotely #23122
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
apparentlymart
added
bug
backend/remote
v0.12
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.
|
I have tested this manually since we don't seem to have unit test coverage for these particular codepaths. I intend to follow up with some additional test coverage in a subsequent commit, but hope to move forward with this manually-tested change for the moment in the interests of expediting a fixed release. |
mildwonkey
approved these changes
Oct 18, 2019
beekus
added a commit
to beekus/terraform
that referenced
this pull request
Nov 13, 2019
Following up on hashicorp#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 hashicorp#23283.
pselle
pushed a commit
that referenced
this pull request
Nov 13, 2019
…3358) * 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 #23283.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The remote backend uses
backend.ParseVariableValueslocally 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.ParseVariableValueslocally, 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 systembe 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-filebased on the workspace stored variables. This fixes #23115.This does have a minor drawback in that it won't produce the error about variables not being supported if the only
-varargument provided by the user produces a parse error, because we can't currently distinguish that particular error type from other error types. We may wish to revisit this in a later release to improve the precision of this hint, but for the moment the priority is to avoid the incorrect validation messages.