Skip to content

Commit

Permalink
core: fix provider config inheritence for deeply nested modules
Browse files Browse the repository at this point in the history
The flattening process was not properly drawing dependencies between provider
nodes in modules and their parent provider nodes.

Fixes #2832
Fixes #4443
Fixes #4865
  • Loading branch information
phinze committed Apr 14, 2016
1 parent e5713ca commit 52a5d6c
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 17 deletions.
41 changes: 41 additions & 0 deletions terraform/context_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,47 @@ func TestContext2Apply_moduleOrphanProvider(t *testing.T) {
}
}

func TestContext2Apply_moduleGrandchildProvider(t *testing.T) {
m := testModule(t, "apply-module-grandchild-provider-inherit")
p := testProvider("aws")
p.ApplyFn = testApplyFn
p.DiffFn = testDiffFn

var callLock sync.Mutex
called := false
p.ConfigureFn = func(c *ResourceConfig) error {
if _, ok := c.Get("value"); !ok {
return fmt.Errorf("value is not found")
}
callLock.Lock()
called = true
callLock.Unlock()

return nil
}

ctx := testContext2(t, &ContextOpts{
Module: m,
Providers: map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p),
},
})

if _, err := ctx.Plan(); err != nil {
t.Fatalf("err: %s", err)
}

if _, err := ctx.Apply(); err != nil {
t.Fatalf("err: %s", err)
}

callLock.Lock()
defer callLock.Unlock()
if called != true {
t.Fatalf("err: configure never called")
}
}

// This tests an issue where all the providers in a module but not
// in the root weren't being added to the root properly. In this test
// case: aws is explicitly added to root, but "test" should be added to.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
resource "aws_instance" "foo" {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module "grandchild" {
source = "./grandchild"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
provider "aws" {
value = "foo"
}

module "child" {
source = "./child"
}
27 changes: 10 additions & 17 deletions terraform/transform_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package terraform

import (
"fmt"
"log"
"strings"

"github.com/hashicorp/go-multierror"
Expand Down Expand Up @@ -214,9 +215,11 @@ func (t *PruneProviderTransformer) Transform(g *Graph) error {
if pn, ok := v.(GraphNodeProvider); !ok || pn.ProviderName() == "" {
continue
}

// Does anything depend on this? If not, then prune it.
if s := g.UpEdges(v); s.Len() == 0 {
if nv, ok := v.(dag.NamedVertex); ok {
log.Printf("[DEBUG] Pruning provider with no dependencies: %s", nv.Name())
}
g.Remove(v)
}
}
Expand Down Expand Up @@ -340,22 +343,18 @@ func (n *graphNodeDisabledProviderFlat) ProviderName() string {

// GraphNodeDependable impl.
func (n *graphNodeDisabledProviderFlat) DependableName() []string {
return []string{n.Name()}
return modulePrefixList(
n.graphNodeDisabledProvider.DependableName(),
modulePrefixStr(n.PathValue))
}

func (n *graphNodeDisabledProviderFlat) DependentOn() []string {
var result []string

// If we're in a module, then depend on our parent's provider
if len(n.PathValue) > 1 {
prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1])
if prefix != "" {
prefix += "."
}

result = append(result, fmt.Sprintf(
"%s%s",
prefix, n.graphNodeDisabledProvider.Name()))
result = modulePrefixList(
n.graphNodeDisabledProvider.DependableName(), prefix)
}

return result
Expand Down Expand Up @@ -474,13 +473,7 @@ func (n *graphNodeProviderFlat) DependentOn() []string {
// If we're in a module, then depend on our parent's provider
if len(n.PathValue) > 1 {
prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1])
if prefix != "" {
prefix += "."
}

result = append(result, fmt.Sprintf(
"%s%s",
prefix, n.graphNodeProvider.Name()))
result = modulePrefixList(n.graphNodeProvider.DependableName(), prefix)
}

return result
Expand Down

0 comments on commit 52a5d6c

Please sign in to comment.