From bb62aba65116540e260e8e36d7f194a4acbd0c56 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 22 Mar 2019 15:30:51 -0400 Subject: [PATCH] add (forces new resource) to provider test diffs Add the (forces new resource) annotation to the diff output for provider tests failures when we can. This helps providers narrow down what might be triggering changes when encountering test failures with the new SDK. --- config/hcl2shim/paths.go | 23 +++++++++++++++++ config/hcl2shim/paths_test.go | 41 ++++++++++++++++++++++++++++++- helper/resource/testing_config.go | 21 +++++++++++++++- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/config/hcl2shim/paths.go b/config/hcl2shim/paths.go index 99437cbb1..3403c026b 100644 --- a/config/hcl2shim/paths.go +++ b/config/hcl2shim/paths.go @@ -251,3 +251,26 @@ func pathFromFlatmapKeySet(key string, ty cty.Type) (cty.Path, error) { // set as a whole changed. return nil, nil } + +// FlatmapKeyFromPath returns the flatmap equivalent of the given cty.Path for +// use in generating legacy style diffs. +func FlatmapKeyFromPath(path cty.Path) string { + var parts []string + + for _, step := range path { + switch step := step.(type) { + case cty.GetAttrStep: + parts = append(parts, step.Name) + case cty.IndexStep: + switch ty := step.Key.Type(); { + case ty == cty.String: + parts = append(parts, step.Key.AsString()) + case ty == cty.Number: + i, _ := step.Key.AsBigFloat().Int64() + parts = append(parts, strconv.Itoa(int(i))) + } + } + } + + return strings.Join(parts, ".") +} diff --git a/config/hcl2shim/paths_test.go b/config/hcl2shim/paths_test.go index cffbe6b5a..ab166e6f8 100644 --- a/config/hcl2shim/paths_test.go +++ b/config/hcl2shim/paths_test.go @@ -3,6 +3,7 @@ package hcl2shim import ( "fmt" "reflect" + "strconv" "strings" "testing" @@ -365,5 +366,43 @@ func TestRequiresReplace(t *testing.T) { }) } - +} + +func TestFlatmapKeyFromPath(t *testing.T) { + for i, tc := range []struct { + path cty.Path + attr string + }{ + { + path: cty.Path{ + cty.GetAttrStep{Name: "force_new"}, + }, + attr: "force_new", + }, + { + path: cty.Path{ + cty.GetAttrStep{Name: "attr"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "force_new"}, + }, + attr: "attr.0.force_new", + }, + { + path: cty.Path{ + cty.GetAttrStep{Name: "attr"}, + cty.IndexStep{Key: cty.StringVal("key")}, + cty.GetAttrStep{Name: "obj_attr"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "force_new"}, + }, + attr: "attr.key.obj_attr.0.force_new", + }, + } { + t.Run(strconv.Itoa(i), func(t *testing.T) { + attr := FlatmapKeyFromPath(tc.path) + if attr != tc.attr { + t.Fatalf("expected:%q got:%q", tc.attr, attr) + } + }) + } } diff --git a/helper/resource/testing_config.go b/helper/resource/testing_config.go index 7635a7057..311fdb6ef 100644 --- a/helper/resource/testing_config.go +++ b/helper/resource/testing_config.go @@ -214,6 +214,7 @@ func legacyDiffComparisonString(changes *plans.Changes) string { } byModule := map[string]map[string]*ResourceChanges{} resourceKeys := map[string][]string{} + requiresReplace := map[string][]string{} var moduleKeys []string for _, rc := range changes.Resources { if rc.Action == plans.NoOp { @@ -239,6 +240,12 @@ func legacyDiffComparisonString(changes *plans.Changes) string { } else { byModule[moduleKey][resourceKey].Deposed[rc.DeposedKey] = rc } + + rr := []string{} + for _, p := range rc.RequiredReplace.List() { + rr = append(rr, hcl2shim.FlatmapKeyFromPath(p)) + } + requiresReplace[resourceKey] = rr } sort.Strings(moduleKeys) for _, ks := range resourceKeys { @@ -254,6 +261,8 @@ func legacyDiffComparisonString(changes *plans.Changes) string { for _, resourceKey := range resourceKeys[moduleKey] { rc := rcs[resourceKey] + forceNewAttrs := requiresReplace[resourceKey] + crud := "UPDATE" if rc.Current != nil { switch rc.Current.Action { @@ -341,7 +350,17 @@ func legacyDiffComparisonString(changes *plans.Changes) string { // at the core layer. updateMsg := "" - // TODO: Mark " (forces new resource)" in updateMsg when appropriate. + + // This may not be as precise as in the old diff, as it matches + // everything under the attribute that was originally marked as + // ForceNew, but should help make it easier to determine what + // caused replacement here. + for _, k := range forceNewAttrs { + if strings.HasPrefix(attrK, k) { + updateMsg = " (forces new resource)" + break + } + } fmt.Fprintf( &mBuf, " %s:%s %#v => %#v%s\n",