From 538302f143d531922a30561bc00df1d771f7668f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Nov 2016 15:38:28 -0800 Subject: [PATCH] terraform: resources nested within a module must also be depended on For example: A => B => C (modules). If A depends on module B, then it also must depend on everything in module C. --- terraform/context_apply_test.go | 108 ++++++++++++++++++ terraform/terraform_test.go | 25 ++++ .../child/child/main.tf | 1 + .../child/main.tf | 3 + .../main.tf | 7 ++ .../child/child/main.tf | 2 + .../child/main.tf | 7 ++ .../main.tf | 3 + terraform/transform_reference.go | 22 +++- 9 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 terraform/test-fixtures/apply-resource-depends-on-module-deep/child/child/main.tf create mode 100644 terraform/test-fixtures/apply-resource-depends-on-module-deep/child/main.tf create mode 100644 terraform/test-fixtures/apply-resource-depends-on-module-deep/main.tf create mode 100644 terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/child/main.tf create mode 100644 terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/main.tf create mode 100644 terraform/test-fixtures/apply-resource-depends-on-module-in-module/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 00be062b8..50c201258 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -335,6 +335,114 @@ module.child: } } +func TestContext2Apply_resourceDependsOnModuleGrandchild(t *testing.T) { + m := testModule(t, "apply-resource-depends-on-module-deep") + p := testProvider("aws") + p.DiffFn = testDiffFn + + { + // Wait for the dependency, sleep, and verify the graph never + // called a child. + var called int32 + var checked bool + p.ApplyFn = func( + info *InstanceInfo, + is *InstanceState, + id *InstanceDiff) (*InstanceState, error) { + if info.HumanId() == "module.child.grandchild.aws_instance.c" { + checked = true + + // Sleep to allow parallel execution + time.Sleep(50 * time.Millisecond) + + // Verify that called is 0 (dep not called) + if atomic.LoadInt32(&called) != 0 { + return nil, fmt.Errorf("aws_instance.a should not be called") + } + } + + atomic.AddInt32(&called, 1) + return testApplyFn(info, is, id) + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + if !checked { + t.Fatal("should check") + } + + checkStateString(t, state, testTerraformApplyResourceDependsOnModuleDeepStr) + } +} + +func TestContext2Apply_resourceDependsOnModuleInModule(t *testing.T) { + m := testModule(t, "apply-resource-depends-on-module-in-module") + p := testProvider("aws") + p.DiffFn = testDiffFn + + { + // Wait for the dependency, sleep, and verify the graph never + // called a child. + var called int32 + var checked bool + p.ApplyFn = func( + info *InstanceInfo, + is *InstanceState, + id *InstanceDiff) (*InstanceState, error) { + if info.HumanId() == "module.child.grandchild.aws_instance.c" { + checked = true + + // Sleep to allow parallel execution + time.Sleep(50 * time.Millisecond) + + // Verify that called is 0 (dep not called) + if atomic.LoadInt32(&called) != 0 { + return nil, fmt.Errorf("nothing else should not be called") + } + } + + atomic.AddInt32(&called, 1) + return testApplyFn(info, is, id) + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + if !checked { + t.Fatal("should check") + } + + checkStateString(t, state, testTerraformApplyResourceDependsOnModuleInModuleStr) + } +} + func TestContext2Apply_mapVarBetweenModules(t *testing.T) { m := testModule(t, "apply-map-var-through-module") p := testProvider("null") diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index cbe59abf1..453e8b79f 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -665,6 +665,31 @@ module.child: ID = foo ` +const testTerraformApplyResourceDependsOnModuleDeepStr = ` +aws_instance.a: + ID = foo + + Dependencies: + module.child + +module.child.grandchild: + aws_instance.c: + ID = foo +` + +const testTerraformApplyResourceDependsOnModuleInModuleStr = ` + +module.child: + aws_instance.b: + ID = foo + + Dependencies: + module.grandchild +module.child.grandchild: + aws_instance.c: + ID = foo +` + const testTerraformApplyTaintStr = ` aws_instance.bar: ID = foo diff --git a/terraform/test-fixtures/apply-resource-depends-on-module-deep/child/child/main.tf b/terraform/test-fixtures/apply-resource-depends-on-module-deep/child/child/main.tf new file mode 100644 index 000000000..716e64b1a --- /dev/null +++ b/terraform/test-fixtures/apply-resource-depends-on-module-deep/child/child/main.tf @@ -0,0 +1 @@ +resource "aws_instance" "c" {} diff --git a/terraform/test-fixtures/apply-resource-depends-on-module-deep/child/main.tf b/terraform/test-fixtures/apply-resource-depends-on-module-deep/child/main.tf new file mode 100644 index 000000000..6cbe350a7 --- /dev/null +++ b/terraform/test-fixtures/apply-resource-depends-on-module-deep/child/main.tf @@ -0,0 +1,3 @@ +module "grandchild" { + source = "./child" +} diff --git a/terraform/test-fixtures/apply-resource-depends-on-module-deep/main.tf b/terraform/test-fixtures/apply-resource-depends-on-module-deep/main.tf new file mode 100644 index 000000000..a0859c0fd --- /dev/null +++ b/terraform/test-fixtures/apply-resource-depends-on-module-deep/main.tf @@ -0,0 +1,7 @@ +module "child" { + source = "./child" +} + +resource "aws_instance" "a" { + depends_on = ["module.child"] +} diff --git a/terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/child/main.tf b/terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/child/main.tf new file mode 100644 index 000000000..3794f75c0 --- /dev/null +++ b/terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/child/main.tf @@ -0,0 +1,2 @@ +resource "aws_instance" "c" {} + diff --git a/terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/main.tf b/terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/main.tf new file mode 100644 index 000000000..62fb42cfd --- /dev/null +++ b/terraform/test-fixtures/apply-resource-depends-on-module-in-module/child/main.tf @@ -0,0 +1,7 @@ +module "grandchild" { + source = "./child" +} + +resource "aws_instance" "b" { + depends_on = ["module.grandchild"] +} diff --git a/terraform/test-fixtures/apply-resource-depends-on-module-in-module/main.tf b/terraform/test-fixtures/apply-resource-depends-on-module-in-module/main.tf new file mode 100644 index 000000000..0f6991c53 --- /dev/null +++ b/terraform/test-fixtures/apply-resource-depends-on-module-in-module/main.tf @@ -0,0 +1,3 @@ +module "child" { + source = "./child" +} diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index ac6703298..7d8a5445d 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -11,6 +11,11 @@ import ( // GraphNodeReferenceable must be implemented by any node that represents // a Terraform thing that can be referenced (resource, module, etc.). +// +// Even if the thing has no name, this should return an empty list. By +// implementing this and returning a non-nil result, you say that this CAN +// be referenced and other methods of referencing may still be possible (such +// as by path!) type GraphNodeReferenceable interface { // ReferenceableName is the name by which this can be referenced. // This can be either just the type, or include the field. Example: @@ -205,7 +210,7 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { // example, if this is a referenceable thing at path []string{"foo"}, // then it can be referenced at "module.foo" if pn, ok := v.(GraphNodeSubPath); ok { - if p := ReferenceModulePath(pn.Path()); p != "" { + for _, p := range ReferenceModulePath(pn.Path()) { refMap[p] = append(refMap[p], v) } } @@ -234,15 +239,22 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { } // Returns the reference name for a module path. The path "foo" would return -// "module.foo" -func ReferenceModulePath(p []string) string { +// "module.foo". If this is a deeply nested module, it will be every parent +// as well. For example: ["foo", "bar"] would return both "module.foo" and +// "module.foo.module.bar" +func ReferenceModulePath(p []string) []string { p = normalizeModulePath(p) if len(p) == 1 { // Root, no name - return "" + return nil } - return fmt.Sprintf("module.%s", strings.Join(p[1:], ".")) + result := make([]string, 0, len(p)-1) + for i := len(p); i > 1; i-- { + result = append(result, modulePrefixStr(p[:i])) + } + + return result } // ReferencesFromConfig returns the references that a configuration has