From 3d0a25c65df992544ee6f6b1d293b8f52f35b0a1 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 15 Jan 2019 15:37:03 +0000 Subject: [PATCH] command/format: Fix nested (JSON) object formatting --- command/format/diff.go | 123 +++++++++++++-- command/format/diff_test.go | 305 +++++++++++++++++++++++++++++++++++- 2 files changed, 407 insertions(+), 21 deletions(-) diff --git a/command/format/diff.go b/command/format/diff.go index d88a80147..24026db0e 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -570,8 +570,6 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa // values are known and non-null. if old.IsKnown() && new.IsKnown() && !old.IsNull() && !new.IsNull() { switch { - // TODO: object diffs that behave a bit like the map diffs, including if the two object types don't exactly match - case ty == cty.String: // We have special behavior for both multi-line strings in general // and for strings that can parse as JSON. For the JSON handling @@ -747,7 +745,6 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString("]") return - case ty.IsListType() || ty.IsTupleType(): p.buf.WriteString("[") if p.pathForcesNewResource(path) { @@ -755,12 +752,27 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa } p.buf.WriteString("\n") - elemDiffs := ctySequenceDiff(old.AsValueSlice(), new.AsValueSlice()) - for _, elemDiff := range elemDiffs { - p.buf.WriteString(strings.Repeat(" ", indent+2)) - p.writeActionSymbol(elemDiff.Action) - p.writeValue(elemDiff.Value, elemDiff.Action, indent+4) - p.buf.WriteString(",\n") + if ty.IsTupleType() && ty.Length() > 0 && ty.TupleElementType(0).IsObjectType() { + elemDiffs := ctyObjectSequenceDiff(old.AsValueSlice(), new.AsValueSlice(), 0) + for _, elemDiff := range elemDiffs { + p.buf.WriteString(strings.Repeat(" ", indent+2)) + p.writeActionSymbol(elemDiff.Action) + if elemDiff.Action == plans.NoOp { + p.writeValue(elemDiff.Before, elemDiff.Action, indent+4) + } else { + p.writeValueDiff(elemDiff.Before, elemDiff.After, indent+4, path) + } + + p.buf.WriteString(",\n") + } + } else { + elemDiffs := ctySequenceDiff(old.AsValueSlice(), new.AsValueSlice()) + for _, elemDiff := range elemDiffs { + p.buf.WriteString(strings.Repeat(" ", indent+2)) + p.writeActionSymbol(elemDiff.Action) + p.writeValue(elemDiff.Value, elemDiff.Action, indent+4) + p.buf.WriteString(",\n") + } } p.buf.WriteString(strings.Repeat(" ", indent)) @@ -841,15 +853,96 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString("}") return + case ty.IsObjectType(): + p.buf.WriteString("{") + p.buf.WriteString("\n") + + forcesNewResource := p.pathForcesNewResource(path) + + var allKeys []string + keyLen := 0 + for it := old.ElementIterator(); it.Next(); { + k, _ := it.Element() + keyStr := k.AsString() + allKeys = append(allKeys, keyStr) + if len(keyStr) > keyLen { + keyLen = len(keyStr) + } + } + for it := new.ElementIterator(); it.Next(); { + k, _ := it.Element() + keyStr := k.AsString() + allKeys = append(allKeys, keyStr) + if len(keyStr) > keyLen { + keyLen = len(keyStr) + } + } + + sort.Strings(allKeys) + + lastK := "" + for i, k := range allKeys { + if i > 0 && lastK == k { + continue // skip duplicates (list is sorted) + } + lastK = k + + p.buf.WriteString(strings.Repeat(" ", indent+2)) + kV := k + var action plans.Action + if !old.Type().HasAttribute(kV) { + action = plans.Create + } else if !new.Type().HasAttribute(kV) { + action = plans.Delete + } else if eqV := old.GetAttr(kV).Equals(new.GetAttr(kV)); eqV.IsKnown() && eqV.True() { + action = plans.NoOp + } else { + action = plans.Update + } + + path := append(path, cty.GetAttrStep{Name: kV}) + + p.writeActionSymbol(action) + p.buf.WriteString(k) + p.buf.WriteString(strings.Repeat(" ", keyLen-len(k))) + p.buf.WriteString(" = ") + + switch action { + case plans.Create, plans.NoOp: + v := new.GetAttr(kV) + p.writeValue(v, action, indent+4) + case plans.Delete: + oldV := old.GetAttr(kV) + newV := cty.NullVal(oldV.Type()) + p.writeValueDiff(oldV, newV, indent+4, path) + default: + oldV := old.GetAttr(kV) + newV := new.GetAttr(kV) + p.writeValueDiff(oldV, newV, indent+4, path) + } + + p.buf.WriteString("\n") + } + + p.buf.WriteString(strings.Repeat(" ", indent)) + p.buf.WriteString("}") + + if forcesNewResource { + p.buf.WriteString(p.color.Color(forcesNewResourceCaption)) + } + return } } - // In all other cases, we just show the new and old values as-is - p.writeValue(old, plans.Delete, indent) - if new.IsNull() { - p.buf.WriteString(p.color.Color(" [dark_gray]->[reset] ")) - } else { - p.buf.WriteString(p.color.Color(" [yellow]->[reset] ")) + // Avoid printing null -> "known" + if !old.IsNull() { + // In all other cases, we just show the new and old values as-is + p.writeValue(old, plans.Delete, indent) + if new.IsNull() { + p.buf.WriteString(p.color.Color(" [dark_gray]->[reset] ")) + } else { + p.buf.WriteString(p.color.Color(" [yellow]->[reset] ")) + } } p.writeValue(new, plans.Create, indent) if p.pathForcesNewResource(path) { diff --git a/command/format/diff_test.go b/command/format/diff_test.go index bc0a3ad82..43d2002bb 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -309,13 +309,73 @@ func TestResourceChange_JSON(t *testing.T) { ~ id = "i-02ae66f368e8518a9" -> (known after apply) ~ json_field = jsonencode( ~ { - - aaa = "value" - } -> { - + aaa = "value" + aaa = "value" + bbb = "new_value" } ) } +`, + }, + "in-place update (from empty tuple)": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`{"aaa": []}`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`{"aaa": ["value"]}`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ { + ~ aaa = [ + + "value", + ] + } + ) + } +`, + }, + "in-place update (to empty tuple)": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`{"aaa": ["value"]}`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`{"aaa": []}`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ { + ~ aaa = [ + - "value", + ] + } + ) + } `, }, "force-new update": { @@ -343,9 +403,7 @@ func TestResourceChange_JSON(t *testing.T) { ~ id = "i-02ae66f368e8518a9" -> (known after apply) ~ json_field = jsonencode( ~ { - - aaa = "value" - } -> { - + aaa = "value" + aaa = "value" + bbb = "new_value" } # forces replacement ) @@ -438,6 +496,241 @@ func TestResourceChange_JSON(t *testing.T) { } `, }, + "JSON list item removal": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`["first","second","third"]`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`["first","second"]`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ [ + "first", + "second", + - "third", + ] + ) + } +`, + }, + "JSON list item addition": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`["first","second"]`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`["first","second","third"]`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ [ + "first", + "second", + + "third", + ] + ) + } +`, + }, + "JSON list object addition": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`{"first":"111"}`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`{"first":"111","second":"222"}`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ { + first = "111" + + second = "222" + } + ) + } +`, + }, + "JSON object with nested list": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`{ + "Statement": ["first"] + }`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`{ + "Statement": ["first", "second"] + }`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ { + ~ Statement = [ + "first", + + "second", + ] + } + ) + } +`, + }, + "JSON list of objects": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`[{"one": "111"}]`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`[{"one": "111"}, {"two": "222"}]`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ [ + { + one = "111" + }, + + { + + two = "222" + }, + ] + ) + } +`, + }, + "JSON object with list of objects": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`{"parent":[{"one": "111"}]}`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`{"parent":[{"one": "111"}, {"two": "222"}]}`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ { + ~ parent = [ + { + one = "111" + }, + + { + + two = "222" + }, + ] + } + ) + } +`, + }, + "JSON object double nested lists": { + Action: plans.Update, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "json_field": cty.StringVal(`{"parent":[{"another_list": ["111"]}]}`), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "json_field": cty.StringVal(`{"parent":[{"another_list": ["111", "222"]}]}`), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "json_field": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(), + ExpectedOutput: ` # test_instance.example will be updated in-place + ~ resource "test_instance" "example" { + ~ id = "i-02ae66f368e8518a9" -> (known after apply) + ~ json_field = jsonencode( + ~ { + ~ parent = [ + ~ { + ~ another_list = [ + "111", + + "222", + ] + }, + ] + } + ) + } +`, + }, + // TODO: JSON with unknown values inside } runTestCases(t, testCases) }