diff --git a/internal/configs/configload/loader_load_test.go b/internal/configs/configload/loader_load_test.go index ab8dd5dee630..ee2488c792ba 100644 --- a/internal/configs/configload/loader_load_test.go +++ b/internal/configs/configload/loader_load_test.go @@ -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) + } + }) + +} diff --git a/internal/configs/configload/testdata/child-provider-child-count/.terraform/modules/modules.json b/internal/configs/configload/testdata/child-provider-child-count/.terraform/modules/modules.json new file mode 100644 index 000000000000..a9239e3a8b69 --- /dev/null +++ b/internal/configs/configload/testdata/child-provider-child-count/.terraform/modules/modules.json @@ -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" + } + ] +} diff --git a/internal/configs/configload/testdata/child-provider-child-count/child-provider-child-count.tf b/internal/configs/configload/testdata/child-provider-child-count/child-provider-child-count.tf new file mode 100644 index 000000000000..5b39941a03c6 --- /dev/null +++ b/internal/configs/configload/testdata/child-provider-child-count/child-provider-child-count.tf @@ -0,0 +1,4 @@ +module "child" { + source = "./child" + count = 1 +} diff --git a/internal/configs/configload/testdata/child-provider-child-count/child/child-provider-child-count-child.tf b/internal/configs/configload/testdata/child-provider-child-count/child/child-provider-child-count-child.tf new file mode 100644 index 000000000000..8d3fe1023d2d --- /dev/null +++ b/internal/configs/configload/testdata/child-provider-child-count/child/child-provider-child-count-child.tf @@ -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 +} diff --git a/internal/configs/configload/testdata/child-provider-grandchild-count/.terraform/modules/modules.json b/internal/configs/configload/testdata/child-provider-grandchild-count/.terraform/modules/modules.json new file mode 100644 index 000000000000..a9239e3a8b69 --- /dev/null +++ b/internal/configs/configload/testdata/child-provider-grandchild-count/.terraform/modules/modules.json @@ -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" + } + ] +} diff --git a/internal/configs/configload/testdata/child-provider-grandchild-count/child-provider-grandchild-count.tf b/internal/configs/configload/testdata/child-provider-grandchild-count/child-provider-grandchild-count.tf new file mode 100644 index 000000000000..1f95749fa7ea --- /dev/null +++ b/internal/configs/configload/testdata/child-provider-grandchild-count/child-provider-grandchild-count.tf @@ -0,0 +1,3 @@ +module "child" { + source = "./child" +} diff --git a/internal/configs/configload/testdata/child-provider-grandchild-count/child/child-provider-grandchild-count-child.tf b/internal/configs/configload/testdata/child-provider-grandchild-count/child/child-provider-grandchild-count-child.tf new file mode 100644 index 000000000000..8d3fe1023d2d --- /dev/null +++ b/internal/configs/configload/testdata/child-provider-grandchild-count/child/child-provider-grandchild-count-child.tf @@ -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 +}