From 0b7be2d0e3f0feeb7b3ef48c14f6a09abc30e101 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Nov 2018 16:59:27 -0500 Subject: [PATCH 1/2] fixes for the remaining tests It's possible that a computed collection could be handled by the attribute name, rather than the index count value. Use a new testDiffFn for some tests, which don't work with the old function that can't determine `computed` without the schema. --- terraform/context_plan_test.go | 100 ++++++++++++++++++++++++++++++++- terraform/diff.go | 2 +- terraform/provider_mock.go | 2 + 3 files changed, 100 insertions(+), 4 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index e7f0e7ab2..623c93d32 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2075,7 +2075,56 @@ func TestContext2Plan_computedList(t *testing.T) { }, }, } - p.DiffFn = testDiffFn + p.DiffFn = func(info *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) { + diff := &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{}, + } + + computedKeys := map[string]bool{} + for _, k := range c.ComputedKeys { + computedKeys[k] = true + } + + compute, _ := c.Raw["compute"].(string) + if compute != "" { + diff.Attributes[compute] = &ResourceAttrDiff{ + Old: "", + New: "", + NewComputed: true, + } + } + + fooOld := s.Attributes["foo"] + fooNew, _ := c.Raw["foo"].(string) + if fooOld != fooNew { + diff.Attributes["foo"] = &ResourceAttrDiff{ + Old: fooOld, + New: fooNew, + NewComputed: computedKeys["foo"], + } + } + + numOld := s.Attributes["num"] + numNew, _ := c.Raw["num"].(string) + if numOld != numNew { + diff.Attributes["num"] = &ResourceAttrDiff{ + Old: numOld, + New: numNew, + NewComputed: computedKeys["num"], + } + } + + listOld := s.Attributes["list.#"] + if listOld == "" { + diff.Attributes["list.#"] = &ResourceAttrDiff{ + Old: "", + New: "", + NewComputed: true, + } + } + + return diff, nil + } ctx := testContext2(t, &ContextOpts{ Config: m, @@ -2129,6 +2178,7 @@ func TestContext2Plan_computedMultiIndex(t *testing.T) { m := testModule(t, "plan-computed-multi-index") p := testProvider("aws") p.DiffFn = testDiffFn + p.GetSchemaReturn = &ProviderSchema{ ResourceTypes: map[string]*configschema.Block{ "aws_instance": { @@ -2141,6 +2191,47 @@ func TestContext2Plan_computedMultiIndex(t *testing.T) { }, } + p.DiffFn = func(info *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) { + diff := &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{}, + } + + compute, _ := c.Raw["compute"].(string) + if compute != "" { + diff.Attributes[compute] = &ResourceAttrDiff{ + Old: "", + New: "", + NewComputed: true, + } + } + + fooOld := s.Attributes["foo"] + fooNew, _ := c.Raw["foo"].(string) + fooComputed := false + for _, k := range c.ComputedKeys { + if k == "foo" { + fooComputed = true + } + } + if fooNew != "" { + diff.Attributes["foo"] = &ResourceAttrDiff{ + Old: fooOld, + New: fooNew, + NewComputed: fooComputed, + } + } + + ipOld := s.Attributes["ip"] + ipComputed := ipOld == "" + diff.Attributes["ip"] = &ResourceAttrDiff{ + Old: ipOld, + New: "", + NewComputed: ipComputed, + } + + return diff, nil + } + ctx := testContext2(t, &ContextOpts{ Config: m, ProviderResolver: providers.ResolverFixed( @@ -2174,14 +2265,17 @@ func TestContext2Plan_computedMultiIndex(t *testing.T) { switch i := ric.Addr.String(); i { case "aws_instance.foo[0]": checkVals(t, objectVal(t, schema, map[string]cty.Value{ - "ip": cty.UnknownVal(cty.List(cty.String)), + "ip": cty.UnknownVal(cty.List(cty.String)), + "foo": cty.ListValEmpty(cty.String), }), ric.After) case "aws_instance.foo[1]": checkVals(t, objectVal(t, schema, map[string]cty.Value{ - "ip": cty.UnknownVal(cty.List(cty.String)), + "ip": cty.UnknownVal(cty.List(cty.String)), + "foo": cty.ListValEmpty(cty.String), }), ric.After) case "aws_instance.bar[0]": checkVals(t, objectVal(t, schema, map[string]cty.Value{ + "ip": cty.UnknownVal(cty.List(cty.String)), "foo": cty.UnknownVal(cty.List(cty.String)), }), ric.After) default: diff --git a/terraform/diff.go b/terraform/diff.go index c8e660ad5..8e7a8d9c5 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -563,7 +563,7 @@ func (d *InstanceDiff) applyCollectionDiff(attrName string, oldAttrs map[string] // check the index first for special handling for k, diff := range d.Attributes { // check the index value, which can be set, and 0 - if k == attrName+".#" || k == attrName+".%" { + if k == attrName+".#" || k == attrName+".%" || k == attrName { if diff.NewRemoved { return result, nil } diff --git a/terraform/provider_mock.go b/terraform/provider_mock.go index 73d26f6dc..a2c5de337 100644 --- a/terraform/provider_mock.go +++ b/terraform/provider_mock.go @@ -282,6 +282,7 @@ func (p *MockProvider) PlanResourceChange(r providers.PlanResourceChangeRequest) } priorState := NewInstanceStateShimmedFromValue(r.PriorState, 0) cfg := NewResourceConfigShimmed(r.Config, schema) + legacyDiff, err := p.DiffFn(info, priorState, cfg) var res providers.PlanResourceChangeResponse @@ -294,6 +295,7 @@ func (p *MockProvider) PlanResourceChange(r providers.PlanResourceChangeRequest) if err != nil { res.Diagnostics = res.Diagnostics.Append(err) } + res.PlannedState = newVal var requiresNew []string From f375691819547c3309c9819e2452f10cd4e14eb3 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Nov 2018 17:59:38 -0500 Subject: [PATCH 2/2] add missing key-value from test --- helper/plugin/grpc_provider_test.go | 2 +- states/state_string.go | 7 ++++++- terraform/context_plan_test.go | 3 ++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index f27edf399..1dcc04df5 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -662,7 +662,7 @@ func TestNormalizeFlatmapContainers(t *testing.T) { expect: map[string]string{"id": "78629a0f5f3f164f"}, }, { - attrs: map[string]string{"set.2.required": "bar", "set.2.list.#": "1", "set.2.list.0": "x", "set.1.list.#": "0"}, + attrs: map[string]string{"set.2.required": "bar", "set.2.list.#": "1", "set.2.list.0": "x", "set.1.list.#": "0", "set.#": "2"}, expect: map[string]string{"set.2.list.#": "1", "set.2.list.0": "x", "set.2.required": "bar", "set.#": "1"}, }, } { diff --git a/states/state_string.go b/states/state_string.go index 8a81a78f1..bca4581c9 100644 --- a/states/state_string.go +++ b/states/state_string.go @@ -164,11 +164,16 @@ func (m *Module) testString() string { } } attrKeys := make([]string, 0, len(attributes)) - for ak, _ := range attributes { + for ak, val := range attributes { if ak == "id" { continue } + // don't show empty containers in the output + if val == "0" && (strings.HasSuffix(ak, ".#") || strings.HasSuffix(ak, ".%")) { + continue + } + attrKeys = append(attrKeys, ak) } diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 623c93d32..53bd8136c 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2159,7 +2159,8 @@ func TestContext2Plan_computedList(t *testing.T) { switch i := ric.Addr.String(); i { case "aws_instance.bar": checkVals(t, objectVal(t, schema, map[string]cty.Value{ - "foo": cty.UnknownVal(cty.String), + "list": cty.UnknownVal(cty.List(cty.String)), + "foo": cty.UnknownVal(cty.String), }), ric.After) case "aws_instance.foo": checkVals(t, objectVal(t, schema, map[string]cty.Value{