command: "terraform show" to render plans like "terraform plan" #23292
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
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.
This "Plan" type, along with the other types it directly or indirectly embeds and the associated functions, are adaptations of the flatmap-oriented plan renderer logic from Terraform 0.11 and prior. The current diff rendering logic is in diff.go, and so the contents of the plan.go file are defunct apart from the DiffActionSymbol function that both implementations share. Therefore here we move DiffActionSymbol into diff.go and then remove plan.go entirely, in the interests of dead code removal.
apparentlymart
added
bug
cli
v0.12
ghost
added
the
sdkv1
jbardin
approved these changes
Nov 6, 2019
pselle
reviewed
Nov 6, 2019
| // can call into it, but we're leaving it here for now in order to avoid | ||
| // disruptive refactoring. | ||
| // | ||
| // If you find yourself wanting to call this function from a third callsite, |
|
Thank you ! That was quick, hats off to you. |
This was referenced Nov 6, 2019
|
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.
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 showcommand 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 plancommand, making the output of both identical.This fixes #23106.
Unfortunately, due to the current backend architecture that logic lives inside the
backend/local, and it contains some business logic around state and schema wrangling that would make it inappropriate to move wholesale into thecommand/formatpackage. To allow for a low-risk fix to theterraform showoutput, here we avoid some more severe refactoring by just exporting the rendering functionality in a way that allows theterraform showcommand to call into it.In future we'd like to move all of the code that actually writes to the output into the
commandpackage so that the roles of these components are better segregated, but that is too big a change to block fixing this issue.This also left lots of dead code in
command/formatfor the old and mostly-broken plan renderer. In order to reduce naming confusion in thecommand/formatpackage I've removedplan.goentirely after removing the one function that was still used from there (DiffActionSymbol) intodiff.go.