Validation for provider blocks in expanding modules (nested) (#25248)

* Refactor provider validation into separate func & recurse

Refactors the validate provider functions into a separate function
that can recursively search above a module to check and see if
any parents of the module contain count/for_each configs to be
considered
This commit is contained in:
Pam Selle 2020-06-16 13:52:41 -04:00 committed by GitHub
parent 3506f159aa
commit 199157a51a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 153 additions and 41 deletions

View File

@ -103,6 +103,14 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,
// The providers associated with expanding modules must be present in the proxy/passed providers // The providers associated with expanding modules must be present in the proxy/passed providers
// block. Guarding here for accessing the module call just in case. // block. Guarding here for accessing the module call just in case.
if mc, exists := req.Parent.Module.ModuleCalls[req.Name]; exists { if mc, exists := req.Parent.Module.ModuleCalls[req.Name]; exists {
var validateDiags hcl.Diagnostics
validateDiags = validateProviderConfigs(mc, mod, req.Parent, validateDiags)
diags = append(diags, validateDiags...)
}
return mod, record.Version, diags
}
func validateProviderConfigs(mc *configs.ModuleCall, mod *configs.Module, parent *configs.Config, diags hcl.Diagnostics) hcl.Diagnostics {
if mc.Count != nil || mc.ForEach != nil { if mc.Count != nil || mc.ForEach != nil {
for key, pc := range mod.ProviderConfigs { for key, pc := range mod.ProviderConfigs {
// Use these to track if a provider is configured (not allowed), // Use these to track if a provider is configured (not allowed),
@ -145,8 +153,20 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,
} }
} }
} }
// If this module has further parents, go through them recursively
if !parent.Path.IsRoot() {
// Use the path to get the name so we can look it up in the parent module calls
path := parent.Path
name := path[len(path)-1]
// This parent's module call, so we can check for count/for_each here,
// guarding with exists just in case. We pass the diags through to the recursive
// call so they will accumulate if needed.
if mc, exists := parent.Parent.Module.ModuleCalls[name]; exists {
return validateProviderConfigs(mc, mod, parent.Parent, diags)
} }
return mod, record.Version, diags }
return diags
} }
var moduleProviderError = `Module "%s" cannot be used with %s because it contains a nested provider configuration for "%s", at %s. var moduleProviderError = `Module "%s" cannot be used with %s because it contains a nested provider configuration for "%s", at %s.

View File

@ -86,24 +86,24 @@ func TestLoaderLoadConfig_moduleExpand(t *testing.T) {
// We do not allow providers to be configured in expanding modules // We do not allow providers to be configured in expanding modules
// In addition, if a provider is present but an empty block, it is allowed, // In addition, if a provider is present but an empty block, it is allowed,
// but IFF a provider is passed through the module call // but IFF a provider is passed through the module call
paths := []string{"provider-configured", "no-provider-passed"} paths := []string{"provider-configured", "no-provider-passed", "nested-provider", "more-nested-provider"}
for _, p := range paths { for _, p := range paths {
fixtureDir := filepath.Clean(fmt.Sprintf("testdata/expand-modules/%s", p)) fixtureDir := filepath.Clean(fmt.Sprintf("testdata/expand-modules/%s", p))
loader, err := NewLoader(&Config{ loader, err := NewLoader(&Config{
ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"), ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
}) })
if err != nil { if err != nil {
t.Fatalf("unexpected error from NewLoader: %s", err) t.Fatalf("unexpected error from NewLoader at path %s: %s", p, err)
} }
_, diags := loader.LoadConfig(fixtureDir) _, diags := loader.LoadConfig(fixtureDir)
if !diags.HasErrors() { if !diags.HasErrors() {
t.Fatalf("success; want error") t.Fatalf("success; want error at path %s", p)
} }
got := diags.Error() got := diags.Error()
want := "Module does not support count" want := "Module does not support count"
if !strings.Contains(got, want) { if !strings.Contains(got, want) {
t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want) t.Fatalf("wrong error at path %s \ngot:\n%s\n\nwant: containing %q", p, got, want)
} }
} }
} }

View File

@ -0,0 +1,34 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "testdata/expand-modules/nested-provider"
},
{
"Key": "child",
"Source": "./child",
"Dir": "testdata/expand-modules/nested-provider/child"
},
{
"Key": "child2",
"Source": "./child2",
"Dir": "testdata/expand-modules/nested-provider/child2"
},
{
"Key": "child3",
"Source": "./child3",
"Dir": "testdata/expand-modules/nested-provider/child3"
},
{
"Key": "child.child2",
"Source": "../child2",
"Dir": "testdata/expand-modules/nested-provider/child2"
},
{
"Key": "child.child2.child3",
"Source": "../child3",
"Dir": "testdata/expand-modules/nested-provider/child3"
}
]
}

View File

@ -0,0 +1,4 @@
module "child2" {
source = "../child2"
}

View File

@ -0,0 +1,4 @@
module "child3" {
source = "../child3"
}

View File

@ -0,0 +1,7 @@
provider "aws" {
}
output "my_output" {
value = "my output"
}

View File

@ -0,0 +1,4 @@
module "child" {
count = 1
source = "./child"
}

View File

@ -0,0 +1,24 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "testdata/expand-modules/nested-provider"
},
{
"Key": "child",
"Source": "./child",
"Dir": "testdata/expand-modules/nested-provider/child"
},
{
"Key": "child2",
"Source": "./child2",
"Dir": "testdata/expand-modules/nested-provider/child2"
},
{
"Key": "child.child2",
"Source": "../child2",
"Dir": "testdata/expand-modules/nested-provider/child2"
}
]
}

View File

@ -0,0 +1,4 @@
module "child2" {
source = "../child2"
}

View File

@ -0,0 +1,7 @@
provider "aws" {
}
output "my_output" {
value = "my output"
}

View File

@ -0,0 +1,4 @@
module "child" {
count = 1
source = "./child"
}