Skip to content

Commit

Permalink
backport of commit d1473d5
Browse files Browse the repository at this point in the history
  • Loading branch information
apparentlymart committed May 20, 2022
1 parent 07555a4 commit c1c4d05
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 0 deletions.
74 changes: 74 additions & 0 deletions internal/configs/configload/loader_load_test.go
Expand Up @@ -104,3 +104,77 @@ func TestLoaderLoadConfig_loadDiags(t *testing.T) {
t.Fatal("expected config module")
}
}

func TestLoaderLoadConfig_childProviderGrandchildCount(t *testing.T) {
// This test is focused on the specific situation where:
// - A child module contains a nested provider block, which is no longer
// recommended but supported for backward-compatibility.
// - A child of that child does _not_ contain a nested provider block,
// and is called with "count" (would also apply to "for_each" and
// "depends_on").
// It isn't valid to use "count" with a module that _itself_ contains
// a provider configuration, but it _is_ valid for a module with a
// provider configuration to call another module with count. We previously
// botched this rule and so this is a regression test to cover the
// solution to that mistake:
// https://github.com/hashicorp/terraform/issues/31081

// Since this test is based on success rather than failure and it's
// covering a relatively large set of code where only a small part
// contributes to the test, we'll make sure to test both the success and
// failure cases here so that we'll have a better chance of noticing if a
// future change makes this succeed only because we've reorganized the code
// so that the check isn't happening at all anymore.
//
// If the "not okay" subtest fails, you should also be skeptical about
// whether the "okay" subtest is still valid, even if it happens to
// still be passing.
t.Run("okay", func(t *testing.T) {
fixtureDir := filepath.Clean("testdata/child-provider-grandchild-count")
loader, err := NewLoader(&Config{
ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
})
if err != nil {
t.Fatalf("unexpected error from NewLoader: %s", err)
}

cfg, diags := loader.LoadConfig(fixtureDir)
assertNoDiagnostics(t, diags)
if cfg == nil {
t.Fatalf("config is nil; want non-nil")
}

var gotPaths []string
cfg.DeepEach(func(c *configs.Config) {
gotPaths = append(gotPaths, strings.Join(c.Path, "."))
})
sort.Strings(gotPaths)
wantPaths := []string{
"", // root module
"child",
"child.grandchild",
}

if !reflect.DeepEqual(gotPaths, wantPaths) {
t.Fatalf("wrong module paths\ngot: %swant %s", spew.Sdump(gotPaths), spew.Sdump(wantPaths))
}
})
t.Run("not okay", func(t *testing.T) {
fixtureDir := filepath.Clean("testdata/child-provider-child-count")
loader, err := NewLoader(&Config{
ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
})
if err != nil {
t.Fatalf("unexpected error from NewLoader: %s", err)
}

_, diags := loader.LoadConfig(fixtureDir)
if !diags.HasErrors() {
t.Fatalf("loading succeeded; want an error")
}
if got, want := diags.Error(), "Module is incompatible with count, for_each, and depends_on"; !strings.Contains(got, want) {
t.Errorf("missing expected error\nwant substring: %s\ngot: %s", want, got)
}
})

}
@@ -0,0 +1,19 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "."
},
{
"Key": "child",
"Source": "./child",
"Dir": "testdata/child-provider-child-count/child"
},
{
"Key": "child.grandchild",
"Source": "../grandchild",
"Dir": "testdata/child-provider-child-count/grandchild"
}
]
}
@@ -0,0 +1,4 @@
module "child" {
source = "./child"
count = 1
}
@@ -0,0 +1,7 @@
provider "boop" {
blah = true
}

module "grandchild" {
source = "../grandchild"
}
@@ -0,0 +1 @@
# Intentionally blank
@@ -0,0 +1,19 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "."
},
{
"Key": "child",
"Source": "./child",
"Dir": "testdata/child-provider-grandchild-count/child"
},
{
"Key": "child.grandchild",
"Source": "../grandchild",
"Dir": "testdata/child-provider-grandchild-count/grandchild"
}
]
}
@@ -0,0 +1,3 @@
module "child" {
source = "./child"
}
@@ -0,0 +1,12 @@
provider "boop" {
blah = true
}

module "grandchild" {
source = "../grandchild"

# grandchild's caller (this file) has a legacy nested provider block, but
# grandchild itself does not and so it's valid to use "count" here even
# though it wouldn't be valid to call "child" (this file) with "count".
count = 2
}
@@ -0,0 +1 @@
# Intentionally blank

0 comments on commit c1c4d05

Please sign in to comment.