From 0b7be2d0e3f0feeb7b3ef48c14f6a09abc30e101 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Nov 2018 16:59:27 -0500 Subject: [PATCH] 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 e7f0e7ab2b..623c93d324 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 c8e660ad5e..8e7a8d9c5b 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 73d26f6dc7..a2c5de3377 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