Skip to content

fix: Make state show command exit with code 1 if the stored state cannot be marshalled for rendering #37933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 24, 2025
+69 −0
Merged

fix: Make state show command exit with code 1 if the stored state cannot be marshalled for rendering #37933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/v1.15/BUG FIXES-20251121-183045.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BUG FIXES
Copy link
Member Author

@SarahFrench SarahFrench Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@mildwonkey mildwonkey Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@SarahFrench SarahFrench Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

body: '`state show`: The `state show` command will now explicitly fail and return code 1 when it fails to render the named resources state'
time: 2025-11-21T18:30:45.571448Z
custom:
Issue: "37933"
1 change: 1 addition & 0 deletions internal/command/state_show.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ func (c *StateShowCommand) Run(args []string) int {
root, outputs, err := jsonstate.MarshalForRenderer(statefile.New(singleInstance, "", 0), schemas)
if err != nil {
c.Streams.Eprintf("Failed to marshal state to json: %s", err)
return 1
}

jstate := jsonformat.State{
Expand Down
63 changes: 63 additions & 0 deletions internal/command/state_show_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,69 @@ func TestStateShow(t *testing.T) {
}
}

func TestStateShow_errorMarshallingState(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_instance",
Name: "foo_invalid",
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
&states.ResourceInstanceObjectSrc{
// The error is caused by the state containing attributes that don't
// match the schema for the resource.
AttrsJSON: []byte(`{"non_existent_attr":"I'm gonna cause an error!"}`),
Status: states.ObjectReady,
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
Module: addrs.RootModule,
},
)
})
statePath := testStateFile(t, state)

p := testProvider()
p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
ResourceTypes: map[string]providers.Schema{
"test_instance": {
Body: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
"foo": {Type: cty.String, Optional: true},
"bar": {Type: cty.String, Optional: true},
},
},
},
},
}

streams, done := terminal.StreamsForTesting(t)
c := &StateShowCommand{
Meta: Meta{
testingOverrides: metaOverridesForProvider(p),
Streams: streams,
},
}

args := []string{
"-state", statePath,
"test_instance.foo_invalid",
}
code := c.Run(args)
output := done(t)
if code != 1 {
t.Fatalf("unexpected code: %d\n\n%s", code, output.Stdout())
}

// Test that error outputs were displayed
expected := "unsupported attribute \"non_existent_attr\""
actual := output.Stderr()
if !strings.Contains(actual, expected) {
t.Fatalf("Expected stderr output to include:\n%q\n\n Instead got:\n%q", expected, actual)
}
}

func TestStateShow_multi(t *testing.T) {
submod, _ := addrs.ParseModuleInstanceStr("module.sub")
state := states.BuildState(func(s *states.SyncState) {
Expand Down