diff --git a/internal/command/jsonformat/differ/attribute.go b/internal/command/jsonformat/differ/attribute.go index 017fc02096..698c3a494c 100644 --- a/internal/command/jsonformat/differ/attribute.go +++ b/internal/command/jsonformat/differ/attribute.go @@ -44,6 +44,18 @@ func computeDiffForNestedAttribute(change structured.Change, nested *jsonprovide } func ComputeDiffForType(change structured.Change, ctype cty.Type) computed.Diff { + if !change.NonLegacySchema { + // Empty strings in blocks should be considered null, because the legacy + // SDK can't always differentiate between null and empty strings and may + // return either. + if before, ok := change.Before.(string); ok && len(before) == 0 { + change.Before = nil + } + if after, ok := change.After.(string); ok && len(after) == 0 { + change.After = nil + } + } + if sensitive, ok := checkForSensitiveType(change, ctype); ok { return sensitive } diff --git a/internal/command/jsonformat/differ/block.go b/internal/command/jsonformat/differ/block.go index c6d0510706..a53e11ac50 100644 --- a/internal/command/jsonformat/differ/block.go +++ b/internal/command/jsonformat/differ/block.go @@ -21,6 +21,10 @@ func ComputeDiffForBlock(change structured.Change, block *jsonprovider.Block) co return unknown } + // NonLegacyValue is only ever switched from false to true, since the + // behavior would be for the entire resource. + change.NonLegacySchema = change.NonLegacySchema || containsNonLegacyFeatures(block) + current := change.GetDefaultActionForIteration() blockValue := change.AsMap() @@ -34,15 +38,6 @@ func ComputeDiffForBlock(change structured.Change, block *jsonprovider.Block) co childValue = childValue.AsNoOp() } - // Empty strings in blocks should be considered null for legacy reasons. - // The SDK doesn't support null strings yet, so we work around this now. - if before, ok := childValue.Before.(string); ok && len(before) == 0 { - childValue.Before = nil - } - if after, ok := childValue.After.(string); ok && len(after) == 0 { - childValue.After = nil - } - // Always treat changes to blocks as implicit. childValue.BeforeExplicit = false childValue.AfterExplicit = false @@ -119,3 +114,40 @@ func ComputeDiffForBlock(change structured.Change, block *jsonprovider.Block) co return computed.NewDiff(renderers.Block(attributes, blocks), current, change.ReplacePaths.Matches()) } + +// containsNonLegacyFeatures checks for features not supported by the legacy +// SDK, so that we can skip the empty string -> null fixup for them. +func containsNonLegacyFeatures(block *jsonprovider.Block) bool { + for _, blockType := range block.BlockTypes { + switch NestingMode(blockType.NestingMode) { + case nestingModeMap, nestingModeGroup: + // these block types were not possible in the SDK + return true + } + } + + for _, attribute := range block.Attributes { + //nested object types were not possible in the SDK + if attribute.AttributeNestedType != nil { + return true + } + + ty := unmarshalAttribute(attribute) + // these types were not possible in the SDK + switch { + case ty.HasDynamicTypes(): + return true + case ty.IsTupleType() || ty.IsObjectType(): + return true + case ty.IsCollectionType(): + // Nested collections were not really supported, but could be + // generated with string types (though we conservatively limit this + // to primitive types) + ety := ty.ElementType() + if ety.IsCollectionType() && !ety.ElementType().IsPrimitiveType() { + return true + } + } + } + return false +} diff --git a/internal/command/jsonformat/plan_test.go b/internal/command/jsonformat/plan_test.go index 3668b1f91a..754db9156d 100644 --- a/internal/command/jsonformat/plan_test.go +++ b/internal/command/jsonformat/plan_test.go @@ -565,7 +565,8 @@ func TestResourceChange_primitiveTypes(t *testing.T) { RequiredReplace: cty.NewPathSet(), ExpectedOutput: ` # test_instance.example will be destroyed - resource "test_instance" "example" { - - id = "i-02ae66f368e8518a9" -> null + - id = "i-02ae66f368e8518a9" -> null + # (1 unchanged attribute hidden) }`, }, "forget": { @@ -724,24 +725,95 @@ func TestResourceChange_primitiveTypes(t *testing.T) { "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-BEFORE"), "unchanged": cty.NullVal(cty.String), + "empty": cty.StringVal(""), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-AFTER"), "unchanged": cty.NullVal(cty.String), + "empty": cty.NullVal(cty.String), }), Schema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ "id": {Type: cty.String, Optional: true, Computed: true}, "ami": {Type: cty.String, Optional: true}, "unchanged": {Type: cty.String, Optional: true}, + "empty": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ ami = "ami-BEFORE" -> "ami-AFTER" + id = "i-02ae66f368e8518a9" + # (1 unchanged attribute hidden) + }`, + }, + "string update (non-legacy)": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "from_null": cty.NullVal(cty.String), + "to_null": cty.StringVal(""), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "from_null": cty.StringVal(""), + "to_null": cty.NullVal(cty.String), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "from_null": {Type: cty.DynamicPseudoType, Optional: true}, + "to_null": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + + from_null = "" + id = "i-02ae66f368e8518a9" + - to_null = "" -> null + }`, + }, + "string update (non-legacy nested object)": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "obj": cty.ObjectVal(map[string]cty.Value{ + "from_null": cty.NullVal(cty.String), + "to_null": cty.StringVal(""), + }), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "obj": cty.ObjectVal(map[string]cty.Value{ + "from_null": cty.StringVal(""), + "to_null": cty.NullVal(cty.String), + }), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "obj": {NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "from_null": {Type: cty.DynamicPseudoType, Optional: true}, + "to_null": {Type: cty.String, Optional: true}, + }, + }}, }, }, RequiredReplace: cty.NewPathSet(), ExpectedOutput: ` # test_instance.example will be updated in-place ~ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" id = "i-02ae66f368e8518a9" + ~ obj = { + + from_null = "" + - to_null = "" -> null + } }`, }, "in-place update of multi-line string field": { diff --git a/internal/command/jsonformat/structured/change.go b/internal/command/jsonformat/structured/change.go index 65af43e45c..5a225be842 100644 --- a/internal/command/jsonformat/structured/change.go +++ b/internal/command/jsonformat/structured/change.go @@ -90,6 +90,14 @@ type Change struct { // that we should display. Any element/attribute not matched by this Matcher // should be skipped. RelevantAttributes attribute_path.Matcher + + // NonLegacySchema must only be used when rendering the change to the CLI, + // and is otherwise ignored. This flag is set when we can be sure that the + // change originated from a resource which is not using the legacy SDK, so + // we don't need to hide changes between empty and null strings. + // NonLegacySchema is only switched to true by the renderer, because that is + // where we have most of the schema information to detect the condition. + NonLegacySchema bool } // FromJsonChange unmarshals the raw []byte values in the jsonplan.Change diff --git a/internal/command/jsonformat/structured/map.go b/internal/command/jsonformat/structured/map.go index f1fed0c5bb..6d507b625a 100644 --- a/internal/command/jsonformat/structured/map.go +++ b/internal/command/jsonformat/structured/map.go @@ -33,6 +33,10 @@ type ChangeMap struct { // RelevantAttributes matches the same attributes in Change exactly. RelevantAttributes attribute_path.Matcher + + // this reflects the parent NonLegacyValue, so that any behavior is + // automatically inherited into child changes. + nonLegacySchema bool } // AsMap converts the Change into an object or map representation by converting @@ -47,6 +51,7 @@ func (change Change) AsMap() ChangeMap { AfterSensitive: genericToMap(change.AfterSensitive), ReplacePaths: change.ReplacePaths, RelevantAttributes: change.RelevantAttributes, + nonLegacySchema: change.NonLegacySchema, } } @@ -69,6 +74,7 @@ func (m ChangeMap) GetChild(key string) Change { AfterSensitive: afterSensitive, ReplacePaths: m.ReplacePaths.GetChildWithKey(key), RelevantAttributes: m.RelevantAttributes.GetChildWithKey(key), + NonLegacySchema: m.nonLegacySchema, } }