Skip to content

Commit 35c19d7

Browse files
command/jsonstate: remove redundant remarking of resource instance (hashicorp#29049)
* command/jsonstate: remove redundant remarking of resource instance ResourceInstanceObjectSrc.Decode already handles marking values with any marks stored in ri.Current.AttrSensitivePaths, so re-applying those marks is not necessary. We've gotten reports of panics coming from this line of code, though I have yet to reproduce the panic in a test. * Implement test to reproduce panic on hashicorp#29042 Co-authored-by: David Alger <davidmalger@gmail.com>
1 parent 512e4e7 commit 35c19d7

2 files changed

Lines changed: 98 additions & 6 deletions

File tree

‎internal/command/jsonstate/state.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,7 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module
330330

331331
current.AttributeValues = marshalAttributeValues(riObj.Value)
332332

333-
// Mark the resource instance value with any marks stored in AttrSensitivePaths so we can build the SensitiveValues object
334-
markedVal := riObj.Value.MarkWithPaths(ri.Current.AttrSensitivePaths)
335-
s := SensitiveAsBool(markedVal)
333+
s := SensitiveAsBool(riObj.Value)
336334
v, err := ctyjson.Marshal(s, s.Type())
337335
if err != nil {
338336
return nil, err
@@ -371,9 +369,7 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module
371369

372370
deposed.AttributeValues = marshalAttributeValues(riObj.Value)
373371

374-
// Mark the resource instance value with any marks stored in AttrSensitivePaths so we can build the SensitiveValues object
375-
markedVal := riObj.Value.MarkWithPaths(rios.AttrSensitivePaths)
376-
s := SensitiveAsBool(markedVal)
372+
s := SensitiveAsBool(riObj.Value)
377373
v, err := ctyjson.Marshal(s, s.Type())
378374
if err != nil {
379375
return nil, err

‎internal/command/jsonstate/state_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,52 @@ func TestMarshalResources(t *testing.T) {
198198
},
199199
false,
200200
},
201+
"resource with marks": {
202+
map[string]*states.Resource{
203+
"test_thing.bar": {
204+
Addr: addrs.AbsResource{
205+
Resource: addrs.Resource{
206+
Mode: addrs.ManagedResourceMode,
207+
Type: "test_thing",
208+
Name: "bar",
209+
},
210+
},
211+
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
212+
addrs.NoKey: {
213+
Current: &states.ResourceInstanceObjectSrc{
214+
Status: states.ObjectReady,
215+
AttrsJSON: []byte(`{"foozles":"confuzles"}`),
216+
AttrSensitivePaths: []cty.PathValueMarks{{
217+
Path: cty.Path{cty.GetAttrStep{Name: "foozles"}},
218+
Marks: cty.NewValueMarks(marks.Sensitive)},
219+
},
220+
},
221+
},
222+
},
223+
ProviderConfig: addrs.AbsProviderConfig{
224+
Provider: addrs.NewDefaultProvider("test"),
225+
Module: addrs.RootModule,
226+
},
227+
},
228+
},
229+
testSchemas(),
230+
[]resource{
231+
{
232+
Address: "test_thing.bar",
233+
Mode: "managed",
234+
Type: "test_thing",
235+
Name: "bar",
236+
Index: addrs.InstanceKey(nil),
237+
ProviderName: "registry.terraform.io/hashicorp/test",
238+
AttributeValues: attributeValues{
239+
"foozles": json.RawMessage(`"confuzles"`),
240+
"woozles": json.RawMessage(`null`),
241+
},
242+
SensitiveValues: json.RawMessage(`{"foozles":true}`),
243+
},
244+
},
245+
false,
246+
},
201247
"single resource wrong schema": {
202248
map[string]*states.Resource{
203249
"test_thing.baz": {
@@ -418,6 +464,51 @@ func TestMarshalResources(t *testing.T) {
418464
},
419465
false,
420466
},
467+
"resource with marked map attr": {
468+
map[string]*states.Resource{
469+
"test_map_attr.bar": {
470+
Addr: addrs.AbsResource{
471+
Resource: addrs.Resource{
472+
Mode: addrs.ManagedResourceMode,
473+
Type: "test_map_attr",
474+
Name: "bar",
475+
},
476+
},
477+
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
478+
addrs.NoKey: {
479+
Current: &states.ResourceInstanceObjectSrc{
480+
Status: states.ObjectReady,
481+
AttrsJSON: []byte(`{"data":{"woozles":"confuzles"}}`),
482+
AttrSensitivePaths: []cty.PathValueMarks{{
483+
Path: cty.Path{cty.GetAttrStep{Name: "data"}},
484+
Marks: cty.NewValueMarks(marks.Sensitive)},
485+
},
486+
},
487+
},
488+
},
489+
ProviderConfig: addrs.AbsProviderConfig{
490+
Provider: addrs.NewDefaultProvider("test"),
491+
Module: addrs.RootModule,
492+
},
493+
},
494+
},
495+
testSchemas(),
496+
[]resource{
497+
{
498+
Address: "test_map_attr.bar",
499+
Mode: "managed",
500+
Type: "test_map_attr",
501+
Name: "bar",
502+
Index: addrs.InstanceKey(nil),
503+
ProviderName: "registry.terraform.io/hashicorp/test",
504+
AttributeValues: attributeValues{
505+
"data": json.RawMessage(`{"woozles":"confuzles"}`),
506+
},
507+
SensitiveValues: json.RawMessage(`{"data":true}`),
508+
},
509+
},
510+
false,
511+
},
421512
}
422513

423514
for name, test := range tests {
@@ -654,6 +745,11 @@ func testSchemas() *terraform.Schemas {
654745
"bar": {Type: cty.String, Optional: true},
655746
},
656747
},
748+
"test_map_attr": {
749+
Attributes: map[string]*configschema.Attribute{
750+
"data": {Type: cty.Map(cty.String), Optional: true, Computed: true, Sensitive: true},
751+
},
752+
},
657753
},
658754
},
659755
},

0 commit comments

Comments
 (0)