fix: Make state show command exit with code 1 if the stored state cannot be marshalled for rendering#37933
Conversation
…annot be marshalled for rendering Prior to this change, when running a `state show` command with an address for a resource that existed in state but didn't match the current schema of the resource, users would see the "Failed to marshal" error but would also see output saying "The state file is empty. No resources are represented". This is misleading, as the state file isn't empty. Also, the command would exit with code 0 despite the error.
9accd41 to
1f6b657
Compare
| @@ -0,0 +1,5 @@ | |||
| kind: BUG FIXES | |||
There was a problem hiding this comment.
Could this be considered a breaking change?
I'd argue not, as the old behaviour was misleading and incorrect, though someone could have built a system that relied on the command not failing somewhere.
There was a problem hiding this comment.
On some level every fix in terraform has the potential to be a breaking change - the longer the behavior is present, the more likely people have already accounted for it in their workflows.
This is about the least-concerning breaking change I can think of since it's not a change in anything that modifies state and even if someone's automation for e.g. carefully checks for that error message if the show command succeeds (to understand that it actually failed), changing the exit code to properly return 1 probably isn't going to break anything.
I'd still call it a bug fix, but not a bad idea to get some other opinions! My process instincts are outdated
There was a problem hiding this comment.
Yeah agreed, previous projects I've worked on have been strict about breaking changes but in those cases state was always being modified. I don't think we have a blanket block on breaking changes here in Core as there's more grey area, but figured I'd flag to the reviewer anyways.
I'm happy to go ahead with this as a bug fix.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
(Potential) Fix following changes in #33116
Prior to this change, if a user runs a
state showcommand with an address for a resource that existed in state but didn't match the current schema of the resource then the user would see"Failed to marshal state to json: unsupported attribute "foobar"""The state file is empty. No resources are represented"This is misleading because the state file used for
state showisn't empty, its just that the command hasn't been able to assemble ajsonformat.Statecorrectly for output. And that suggests that we should return with code 1 before that is attempted.Target Release
1.15.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry