ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

configload should not be doing validation 

The configload package should only be responsible for locating and
loading the configuration, and not be further inspecting the config
source itself. Moving the validating into the configs package.
This commit is contained in:
James Bardin 2021-02-10 10:20:40 -05:00
parent ac585be079
commit 7aaffac223
21 changed files with 0 additions and 349 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
configs/configload/loader_load.go
View File

@ -100,83 +100,5 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,
}
}
// 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.
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 || mc.DependsOn != nil {
for key, pc := range mod.ProviderConfigs {
// Use these to track if a provider is configured (not allowed),
// or if we've found its matching proxy
var isConfigured bool
var foundMatchingProxy bool
// Validate the config against an empty schema to see if it's empty.
_, pcConfigDiags := pc.Config.Content(&hcl.BodySchema{})
if pcConfigDiags.HasErrors() || pc.Version.Required != nil {
isConfigured = true
}
// If it is empty or only has an alias,
// does this provider exist in our proxy configs?
for _, r := range mc.Providers {
// Must match on name and Alias
if pc.Name == r.InChild.Name && pc.Alias == r.InChild.Alias {
foundMatchingProxy = true
break
}
}
if isConfigured || !foundMatchingProxy {
if mc.Count != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module does not support count",
Detail: fmt.Sprintf(moduleProviderError, mc.Name, "count", key, pc.NameRange),
Subject: mc.Count.Range().Ptr(),
})
}
if mc.ForEach != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module does not support for_each",
Detail: fmt.Sprintf(moduleProviderError, mc.Name, "for_each", key, pc.NameRange),
Subject: mc.ForEach.Range().Ptr(),
})
}
if mc.DependsOn != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module does not support depends_on",
Detail: fmt.Sprintf(moduleProviderError, mc.Name, "depends_on", key, pc.NameRange),
Subject: mc.SourceAddrRange.Ptr(),
})
}
}
}
}
// 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 diags
}
var moduleProviderError = `Module "%s" cannot be used with %s because it contains a nested provider configuration for "%s", at %s.
This module can be made compatible with %[2]s by changing it to receive all of its provider configurations from the calling module, by using the "providers" argument in the calling module block.`

63
configs/configload/loader_load_test.go
View File

@ -1,7 +1,6 @@
package configload
import (
"fmt"
"path/filepath"
"reflect"
"sort"
@ -81,65 +80,3 @@ func TestLoaderLoadConfig_addVersion(t *testing.T) {
t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want)
}
}
func TestLoaderLoadConfig_moduleExpand(t *testing.T) {
// 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,
// but IFF a provider is passed through the module call
paths := []string{"provider-configured", "no-provider-passed", "nested-provider", "more-nested-provider"}
for _, p := range paths {
fixtureDir := filepath.Clean(fmt.Sprintf("testdata/expand-modules/%s", p))
loader, err := NewLoader(&Config{
ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
})
if err != nil {
t.Fatalf("unexpected error from NewLoader at path %s: %s", p, err)
}
_, diags := loader.LoadConfig(fixtureDir)
if !diags.HasErrors() {
t.Fatalf("success; want error at path %s", p)
}
got := diags.Error()
want := "Module does not support count"
if !strings.Contains(got, want) {
t.Fatalf("wrong error at path %s \ngot:\n%s\n\nwant: containing %q", p, got, want)
}
}
}
func TestLoaderLoadConfig_moduleExpandValid(t *testing.T) {
// This tests for when valid configs are passing a provider through as a proxy,
// either with or without an alias present.
fixtureDir := filepath.Clean("testdata/expand-modules/valid")
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)
assertNoDiagnostics(t, diags)
}
func TestLoaderLoadConfig_moduleDependsOnProviders(t *testing.T) {
// We do not allow providers to be configured in module using depends_on.
fixtureDir := filepath.Clean("testdata/module-depends-on")
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.Fatal("success; want error")
}
got := diags.Error()
want := "Module does not support depends_on"
if !strings.Contains(got, want) {
t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want)
}
}

34
configs/configload/testdata/expand-modules/more-nested-provider/.terraform/modules/modules.json vendored
View File

@ -1,34 +0,0 @@
{
"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"
}
]
}

4
configs/configload/testdata/expand-modules/more-nested-provider/child/main.tf vendored
View File

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

4
configs/configload/testdata/expand-modules/more-nested-provider/child2/main.tf vendored
View File

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

7
configs/configload/testdata/expand-modules/more-nested-provider/child3/main.tf vendored
View File

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

4
configs/configload/testdata/expand-modules/more-nested-provider/root.tf vendored
View File

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

24
configs/configload/testdata/expand-modules/nested-provider/.terraform/modules/modules.json vendored
View File

@ -1,24 +0,0 @@
{
"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"
}
]
}

4
configs/configload/testdata/expand-modules/nested-provider/child/main.tf vendored
View File

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

7
configs/configload/testdata/expand-modules/nested-provider/child2/main.tf vendored
View File

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

4
configs/configload/testdata/expand-modules/nested-provider/root.tf vendored
View File

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

14
configs/configload/testdata/expand-modules/no-provider-passed/.terraform/modules/modules.json vendored
View File

@ -1,14 +0,0 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "testdata/expand-modules/no-provider-passed"
},
{
"Key": "child",
"Source": "./child",
"Dir": "testdata/expand-modules/no-provider-passed/child"
}
]
}

7
configs/configload/testdata/expand-modules/no-provider-passed/child/main.tf vendored
View File

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

9
configs/configload/testdata/expand-modules/no-provider-passed/root.tf vendored
View File

@ -1,9 +0,0 @@
provider "aws" {
alias = "usw2"
region = "us-west-2"
}
module "child" {
count = 1
source = "./child"
# To make this test fail, add a valid providers {} block passing "aws" to the child
}

14
configs/configload/testdata/expand-modules/provider-configured/.terraform/modules/modules.json vendored
View File

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

7
configs/configload/testdata/expand-modules/provider-configured/child/main.tf vendored
View File

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

11
configs/configload/testdata/expand-modules/provider-configured/root.tf vendored
View File

@ -1,11 +0,0 @@
provider "aws" {
region = "us-west-2"
}
module "child" {
count = 1
source = "./child"
providers = {
aws = aws.w2
}
}

19
configs/configload/testdata/expand-modules/valid/.terraform/modules/modules.json vendored
View File

@ -1,19 +0,0 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "testdata/expand-modules/valid"
},
{
"Key": "child",
"Source": "./child",
"Dir": "testdata/expand-modules/valid/child"
},
{
"Key": "child_with_alias",
"Source": "./child-with-alias",
"Dir": "testdata/expand-modules/valid/child-with-alias"
}
]
}

8
configs/configload/testdata/expand-modules/valid/child-with-alias/main.tf vendored
View File

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

7
configs/configload/testdata/expand-modules/valid/child/main.tf vendored
View File

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

20
configs/configload/testdata/expand-modules/valid/root.tf vendored
View File

@ -1,20 +0,0 @@
provider "aws" {
region = "us-east-1"
alias = "east"
}
module "child" {
count = 1
source = "./child"
providers = {
aws = aws.east
}
}
module "child_with_alias" {
for_each = toset(["a", "b"])
source = "./child-with-alias"
providers = {
aws.east = aws.east
}
}