From 470362b0511d44db071f2979009f0e20423f216e Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 26 Sep 2019 12:36:44 -0400 Subject: [PATCH] fix CBD tests to work on real data The CBDEdgeTransformer tests worked on fake data structures, with a synthetic graph, and configs that didn't match. Update them to generate a more complete graph, with real node implementations, from real configs. The output graph is filtered down to instances, and the results still functionally match the original expected test results, with some minor additions due to using the real implementation. --- terraform/graph_builder_apply.go | 15 +- .../main.tf | 11 + .../transform-cbd-destroy-edge-count/main.tf | 10 + .../transform-destroy-cbd-edge-basic/main.tf | 9 + terraform/transform_destroy_cbd.go | 7 + terraform/transform_destroy_cbd_test.go | 324 +++++++++--------- 6 files changed, 206 insertions(+), 170 deletions(-) create mode 100644 terraform/testdata/transform-cbd-destroy-edge-both-count/main.tf create mode 100644 terraform/testdata/transform-cbd-destroy-edge-count/main.tf create mode 100644 terraform/testdata/transform-destroy-cbd-edge-basic/main.tf diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 9382c8aba..73b6b9a56 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -162,14 +162,13 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { State: b.State, Schemas: b.Schemas, }, - GraphTransformIf( - func() bool { return !b.Destroy }, - &CBDEdgeTransformer{ - Config: b.Config, - State: b.State, - Schemas: b.Schemas, - }, - ), + + &CBDEdgeTransformer{ + Config: b.Config, + State: b.State, + Schemas: b.Schemas, + Destroy: b.Destroy, + }, // Handle destroy time transformations for output and local values. // Reverse the edges from outputs and locals, so that diff --git a/terraform/testdata/transform-cbd-destroy-edge-both-count/main.tf b/terraform/testdata/transform-cbd-destroy-edge-both-count/main.tf new file mode 100644 index 000000000..c19e78eaa --- /dev/null +++ b/terraform/testdata/transform-cbd-destroy-edge-both-count/main.tf @@ -0,0 +1,11 @@ +resource "test_object" "A" { + count = 2 + lifecycle { + create_before_destroy = true + } +} + +resource "test_object" "B" { + count = 2 + test_string = test_object.A[*].test_string[count.index] +} diff --git a/terraform/testdata/transform-cbd-destroy-edge-count/main.tf b/terraform/testdata/transform-cbd-destroy-edge-count/main.tf new file mode 100644 index 000000000..775900fcd --- /dev/null +++ b/terraform/testdata/transform-cbd-destroy-edge-count/main.tf @@ -0,0 +1,10 @@ +resource "test_object" "A" { + lifecycle { + create_before_destroy = true + } +} + +resource "test_object" "B" { + count = 2 + test_string = test_object.A.test_string +} diff --git a/terraform/testdata/transform-destroy-cbd-edge-basic/main.tf b/terraform/testdata/transform-destroy-cbd-edge-basic/main.tf new file mode 100644 index 000000000..a17d8b4e3 --- /dev/null +++ b/terraform/testdata/transform-destroy-cbd-edge-basic/main.tf @@ -0,0 +1,9 @@ +resource "test_object" "A" { + lifecycle { + create_before_destroy = true + } +} + +resource "test_object" "B" { + test_string = "${test_object.A.id}" +} diff --git a/terraform/transform_destroy_cbd.go b/terraform/transform_destroy_cbd.go index 4b954facc..21b75c141 100644 --- a/terraform/transform_destroy_cbd.go +++ b/terraform/transform_destroy_cbd.go @@ -134,9 +134,16 @@ type CBDEdgeTransformer struct { // obtain schema information from providers and provisioners so we can // properly resolve implicit dependencies. Schemas *Schemas + + // If the operation is a simple destroy, no transformation is done. + Destroy bool } func (t *CBDEdgeTransformer) Transform(g *Graph) error { + if t.Destroy { + return nil + } + // Go through and reverse any destroy edges destroyMap := make(map[string][]dag.Vertex) for _, v := range g.Vertices() { diff --git a/terraform/transform_destroy_cbd_test.go b/terraform/transform_destroy_cbd_test.go index 665d81647..9d320d776 100644 --- a/terraform/transform_destroy_cbd_test.go +++ b/terraform/transform_destroy_cbd_test.go @@ -1,198 +1,198 @@ package terraform import ( + "regexp" "strings" "testing" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/plans" ) +func cbdTestGraph(t *testing.T, mod string, changes *plans.Changes) *Graph { + module := testModule(t, mod) + + applyBuilder := &ApplyGraphBuilder{ + Config: module, + Changes: changes, + Components: simpleMockComponentFactory(), + Schemas: simpleTestSchemas(), + } + g, err := (&BasicGraphBuilder{ + Steps: cbdTestSteps(applyBuilder.Steps()), + Name: "ApplyGraphBuilder", + }).Build(addrs.RootModuleInstance) + if err != nil { + t.Fatalf("err: %s", err) + } + + return filterInstances(g) +} + +// override the apply graph builder to halt the process after CBD +func cbdTestSteps(steps []GraphTransformer) []GraphTransformer { + found := false + var i int + var t GraphTransformer + for i, t = range steps { + if _, ok := t.(*CBDEdgeTransformer); ok { + found = true + break + } + } + + if !found { + panic("CBDEdgeTransformer not found") + } + + return steps[:i+1] +} + +// remove extra nodes for easier test comparisons +func filterInstances(g *Graph) *Graph { + for _, v := range g.Vertices() { + if _, ok := v.(GraphNodeResourceInstance); !ok { + g.Remove(v) + } + + } + return g +} + func TestCBDEdgeTransformer(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - g.Add(&graphNodeCreatorTest{AddrString: "test_object.A"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.B"}) - g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A", CBD: true}) - - module := testModule(t, "transform-destroy-edge-basic") - - { - tf := &DestroyEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } + changes := &plans.Changes{ + Resources: []*plans.ResourceInstanceChangeSrc{ + { + Addr: mustResourceInstanceAddr("test_object.A"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.CreateThenDelete, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.B"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Update, + }, + }, + }, } - { - tf := &CBDEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } + g := cbdTestGraph(t, "transform-destroy-cbd-edge-basic", changes) + g = filterInstances(g) actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformCBDEdgeBasicStr) - if actual != expected { + expected := regexp.MustCompile(strings.TrimSpace(` +(?m)test_object.A +test_object.A \(destroy deposed \w+\) + test_object.A + test_object.B +test_object.B + test_object.A +`)) + + if !expected.MatchString(actual) { t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected) } } -// FIXME: see if there is a worthwhile test to create from this. -// CBD is marked on created nodes during the plan phase now, and the -// CBDEdgeTransformer only takes care of the final edge reversal. -/* -func TestCBDEdgeTransformer_depNonCBD(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - g.Add(&graphNodeCreatorTest{AddrString: "test_object.A"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.B"}) - g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A"}) - g.Add(&graphNodeDestroyerTest{AddrString: "test_object.B", CBD: true}) - - module := testModule(t, "transform-destroy-edge-basic") - - { - tf := &DestroyEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - { - tf := &CBDEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformCBDEdgeDepNonCBDStr) - if actual != expected { - t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected) - } -} -*/ - func TestCBDEdgeTransformer_depNonCBDCount(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - g.Add(&graphNodeCreatorTest{AddrString: "test_object.A"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[0]"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[1]"}) - g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A", CBD: true}) - - module := testModule(t, "transform-destroy-edge-splat") - - { - tf := &DestroyEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } + changes := &plans.Changes{ + Resources: []*plans.ResourceInstanceChangeSrc{ + { + Addr: mustResourceInstanceAddr("test_object.A"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.CreateThenDelete, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.B[0]"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Update, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.B[1]"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Update, + }, + }, + }, } - { - tf := &CBDEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } + g := cbdTestGraph(t, "transform-cbd-destroy-edge-count", changes) actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(` -test_object.A -test_object.A (destroy) + expected := regexp.MustCompile(strings.TrimSpace(` +(?m)test_object.A +test_object.A \(destroy deposed \w+\) test_object.A - test_object.B[0] - test_object.B[1] -test_object.B[0] -test_object.B[1] - `) - if actual != expected { + test_object.B\[0\] + test_object.B\[1\] +test_object.B\[0\] + test_object.A +test_object.B\[1\] + test_object.A`)) + + if !expected.MatchString(actual) { t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected) } } func TestCBDEdgeTransformer_depNonCBDCountBoth(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - g.Add(&graphNodeCreatorTest{AddrString: "test_object.A[0]"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.A[1]"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[0]"}) - g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[1]"}) - g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A[0]", CBD: true}) - g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A[1]", CBD: true}) - - module := testModule(t, "transform-destroy-edge-splat") - - { - tf := &DestroyEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } + changes := &plans.Changes{ + Resources: []*plans.ResourceInstanceChangeSrc{ + { + Addr: mustResourceInstanceAddr("test_object.A[0]"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.CreateThenDelete, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.A[1]"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.CreateThenDelete, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.B[0]"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Update, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.B[1]"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Update, + }, + }, + }, } - { - tf := &CBDEdgeTransformer{ - Config: module, - Schemas: simpleTestSchemas(), - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } + g := cbdTestGraph(t, "transform-cbd-destroy-edge-both-count", changes) actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(` -test_object.A[0] -test_object.A[0] (destroy) - test_object.A[0] - test_object.B[0] - test_object.B[1] -test_object.A[1] -test_object.A[1] (destroy) - test_object.A[1] - test_object.B[0] - test_object.B[1] -test_object.B[0] -test_object.B[1] - `) - if actual != expected { + expected := regexp.MustCompile(strings.TrimSpace(` +test_object.A \(destroy deposed \w+\) + test_object.A\[0\] + test_object.A\[1\] + test_object.B\[0\] + test_object.B\[1\] +test_object.A \(destroy deposed \w+\) + test_object.A\[0\] + test_object.A\[1\] + test_object.B\[0\] + test_object.B\[1\] +test_object.A\[0\] +test_object.A\[1\] +test_object.B\[0\] + test_object.A\[0\] + test_object.A\[1\] +test_object.B\[1\] + test_object.A\[0\] + test_object.A\[1\] +`)) + + if !expected.MatchString(actual) { t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected) } } - -const testTransformCBDEdgeBasicStr = ` -test_object.A -test_object.A (destroy) - test_object.A - test_object.B -test_object.B -` - -const testTransformCBDEdgeDepNonCBDStr = ` -test_object.A -test_object.A (destroy) (modified) - test_object.A - test_object.B - test_object.B (destroy) -test_object.B -test_object.B (destroy) - test_object.B -`