builtin/providers/terraform: Disable remote state file version checks #27011
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
Instead of always checking the Terraform version associated with a state file when reading it, we add a CheckTerraformVersion method and call it from locations where we care about enforcing this check. For this commit, the check has been retained at all call sites for states/statefile.Read, with these exceptions: - Unit tests, which shouldn't care about the state file version; - E2E test runner which should always be using valid state files; - terraform.ShimLegacyState, where the check is pointless as the state file was just created by the current Terraform version.
Add RefreshStateWithoutCheckVersion method to the statemgr.Persistent interface, allowing callers to refresh state from the backend without raising errors if the state's Terraform version is thought to be not fully compatible. This enables use cases where we can be extremely confident that any state file we can read is suitable, such as the Terraform provider's remote state data source, which only reads outputs.
Allow users of backends to initialize a state manager instance without checking the Terraform version of any state files which are retrieved during this process. Many backends call RefreshState as part of initialization, and this new method instead calls the new RefreshStateWithoutCheckVersion method to prevent version checking.
The builtin Terraform provider's remote state data source uses a configured backend to fetch a given state, in order to allow access to its root module outputs. Until this commit, this was only possible with remote states which are from the current Terraform version or older, forcing multi-state users to carefully orchestrate Terraform upgrades. Building on previous commits in this branch, we now disable this version check, and allow any Terraform state file that the current Terraform version can parse. Since we are only ever accessing root module outputs, this is very likely to be safe for the foreseeable future.
Codecov Report
|
pselle
approved these changes
Nov 30, 2020
There was a problem hiding this comment.
As usual, I really appreciate the attention and detail you put into your commit text/summaries that made walking through this PR commit-by-commit indeed easier to read (and thanks for giving that recommendation!). Thank you for the effort you put in there, it always inspires me to boost the quality of my own!
alisdair
deleted the
alisdair/builtin-terraform-provider-disable-version-checks-012
branch
|
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 as resolved 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.
Backport of #26692 to v0.12. This was not a clean cherry-pick, so I've pushed each commit individually to show passing tests. From the original PR description:
The builtin Terraform provider's remote state data source uses a configured backend to fetch a given state, in order to allow access to its root module outputs. Until this change, this was only possible with remote states which are from the current Terraform version or older, forcing multi-state users to carefully orchestrate Terraform upgrades.
We can now disable this version check, and allow any Terraform state file that the current Terraform version can parse. Since we are only ever accessing root module outputs, this is very likely to be safe for the foreseeable future.
Notes to reviewers 📝
I believe this PR will be easier to review one commit at a time. There is some detail in each commit message which may be worth reading. Each commit except the last one should be a no-op, and existing tests pass.
This specific implementation of this idea was intended to be as mechanically simple as possible, because it covers so many files which are difficult to test. The ambition is for reviewers to be confident enough in this change to approve back-porting it to earlier Terraform versions.