Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate implied provider names in submodules #31573

Merged
merged 1 commit into from Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions internal/configs/configload/loader_load_test.go
Expand Up @@ -105,6 +105,33 @@ func TestLoaderLoadConfig_loadDiags(t *testing.T) {
}
}

func TestLoaderLoadConfig_loadDiagsFromSubmodules(t *testing.T) {
// building a config which didn't load correctly may cause configs to panic
fixtureDir := filepath.Clean("testdata/invalid-names-in-submodules")
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)
if !diags.HasErrors() {
t.Fatalf("loading succeeded; want an error")
}
if got, want := diags.Error(), " Invalid provider local name"; !strings.Contains(got, want) {
t.Errorf("missing expected error\nwant substring: %s\ngot: %s", want, got)
}

if cfg == nil {
t.Fatal("partial config not returned with diagnostics")
}

if cfg.Module == nil {
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
Expand Down
@@ -0,0 +1,14 @@
{
"Modules": [
{
"Key": "test",
"Source": "./sub",
"Dir": "testdata/invalid-names-in-submodules/sub"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was somewhat confused at first that all the modules in testdata have essentially fake Dir - if each dir was otherwise terraform inited directly, there wouldn't be testdata/invalid-names-in-submodules but only the last part (sub).

I'm not sure if that's intentional or not, just something I wanted to point out here.

},
{
"Key": "",
"Source": "",
"Dir": "."
}
]
}
@@ -0,0 +1,3 @@
module "test" {
source = "./sub"
}
@@ -0,0 +1,7 @@
resource "aws-_foo" "test" {

}

data "aws-_bar" "test" {

}
12 changes: 12 additions & 0 deletions internal/configs/provider_validation.go
Expand Up @@ -153,6 +153,18 @@ func validateProviderConfigs(parentCall *ModuleCall, cfg *Config, noProviderConf
}

localName := r.Addr().ImpliedProvider()

_, err := addrs.ParseProviderPart(localName)
if err != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid provider local name",
Detail: fmt.Sprintf("%q is an invalid implied provider local name: %s", localName, err),
Subject: r.DeclRange.Ptr(),
})
continue
}

if _, ok := localNames[localName]; ok {
// OK, this was listed directly in the required_providers
continue
Expand Down