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

jsonconfig: fix keys for default providers #30525

Merged
merged 3 commits into from Feb 22, 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
3 changes: 2 additions & 1 deletion internal/command/command_test.go
Expand Up @@ -171,7 +171,8 @@ func testFixturePath(name string) string {
func metaOverridesForProvider(p providers.Interface) *testingOverrides {
return &testingOverrides{
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): providers.FactoryFixed(p),
addrs.NewDefaultProvider("test"): providers.FactoryFixed(p),
addrs.NewProvider(addrs.DefaultProviderRegistryHost, "hashicorp2", "test"): providers.FactoryFixed(p),
},
}
}
Expand Down
68 changes: 52 additions & 16 deletions internal/command/jsonconfig/config.go
Expand Up @@ -231,6 +231,36 @@ func marshalProviderConfigs(
p.VersionConstraint = getproviders.VersionConstraintsString(vc)
}

if c.Parent != nil {
parentKey := opaqueProviderKey(pr.Name, c.Parent.Path.String())
p.parentKey = findSourceProviderKey(parentKey, p.FullName, m)
}

m[key] = p
}

// Providers could be implicitly created or inherited from the parent module
// when no requirements and configuration block defined.
for req := range reqs {
// Only default providers could implicitly exist,
// so the provider name must be same as the provider type.
key := opaqueProviderKey(req.Type, c.Path.String())
if _, exists := m[key]; exists {
continue
}

p := providerConfig{
Name: req.Type,
FullName: req.String(),
ModuleAddress: c.Path.String(),
}

// In child modules, providers defined in the parent module can be implicitly used.
if c.Parent != nil {
parentKey := opaqueProviderKey(req.Type, c.Parent.Path.String())
p.parentKey = findSourceProviderKey(parentKey, p.FullName, m)
}

m[key] = p
}

Expand Down Expand Up @@ -259,22 +289,7 @@ func marshalProviderConfigs(

key := opaqueProviderKey(moduleProviderName, cc.Path.String())
parentKey := opaqueProviderKey(parentProviderName, cc.Parent.Path.String())

// Traverse up the module call tree until we find the provider
// configuration which has no linked parent config. This is then
// the source of the configuration used in this module call, so
// we link to it directly
for {
parent, exists := m[parentKey]
if !exists {
break
}
p.parentKey = parentKey
parentKey = parent.parentKey
if parentKey == "" {
break
}
}
p.parentKey = findSourceProviderKey(parentKey, p.FullName, m)

m[key] = p
}
Expand Down Expand Up @@ -527,3 +542,24 @@ func opaqueProviderKey(provider string, addr string) (key string) {
}
return key
}

// Traverse up the module call tree until we find the provider
// configuration which has no linked parent config. This is then
// the source of the configuration used in this module call, so
// we link to it directly
func findSourceProviderKey(startKey string, fullName string, m map[string]providerConfig) string {
var parentKey string

key := startKey
for key != "" {
parent, exists := m[key]
if !exists || parent.FullName != fullName {
break
}

parentKey = key
key = parent.parentKey
}

return parentKey
}
100 changes: 100 additions & 0 deletions internal/command/jsonconfig/config_test.go
@@ -0,0 +1,100 @@
package jsonconfig

import (
"testing"
)

func TestFindSourceProviderConfig(t *testing.T) {
tests := []struct {
StartKey string
FullName string
ProviderMap map[string]providerConfig
Want string
}{
{
StartKey: "null",
FullName: "hashicorp/null",
ProviderMap: map[string]providerConfig{},
Want: "",
},
{
StartKey: "null",
FullName: "hashicorp/null",
ProviderMap: map[string]providerConfig{
"null": {
Name: "null",
FullName: "hashicorp/null",
ModuleAddress: "",
},
},
Want: "null",
},
{
StartKey: "null2",
FullName: "hashicorp/null",
ProviderMap: map[string]providerConfig{
"null": {
Name: "null",
FullName: "hashicorp/null",
ModuleAddress: "",
},
},
Want: "",
},
{
StartKey: "null",
FullName: "hashicorp2/null",
ProviderMap: map[string]providerConfig{
"null": {
Name: "null",
FullName: "hashicorp/null",
ModuleAddress: "",
},
},
Want: "",
},
{
StartKey: "module.a:null",
FullName: "hashicorp/null",
ProviderMap: map[string]providerConfig{
"null": {
Name: "null",
FullName: "hashicorp/null",
ModuleAddress: "",
},
"module.a:null": {
Name: "module.a:null",
FullName: "hashicorp/null",
ModuleAddress: "module.a",
parentKey: "null",
},
},
Want: "null",
},
{
StartKey: "module.a:null",
FullName: "hashicorp2/null",
ProviderMap: map[string]providerConfig{
"null": {
Name: "null",
FullName: "hashicorp/null",
ModuleAddress: "",
},
"module.a:null": {
Name: "module.a:null",
FullName: "hashicorp2/null",
ModuleAddress: "module.a",
parentKey: "null",
},
},
Want: "module.a:null",
},
}

for _, test := range tests {
got := findSourceProviderKey(test.StartKey, test.FullName, test.ProviderMap)
if got != test.Want {
t.Errorf("wrong result:\nGot: %#v\nWant: %#v\n", got, test.Want)
}
}
}
3 changes: 2 additions & 1 deletion internal/command/show_test.go
Expand Up @@ -493,7 +493,8 @@ func TestShow_json_output(t *testing.T) {
expectError := strings.Contains(entry.Name(), "error")

providerSource, close := newMockProviderSource(t, map[string][]string{
"test": {"1.2.3"},
"test": {"1.2.3"},
"hashicorp2/test": {"1.2.3"},
})
defer close()

Expand Down
6 changes: 6 additions & 0 deletions internal/command/testdata/show-json/basic-delete/output.json
Expand Up @@ -130,6 +130,12 @@
}
},
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"outputs": {
"test": {
Expand Down
6 changes: 6 additions & 0 deletions internal/command/testdata/show-json/basic-update/output.json
Expand Up @@ -96,6 +96,12 @@
}
},
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"outputs": {
"test": {
Expand Down
6 changes: 6 additions & 0 deletions internal/command/testdata/show-json/drift/output.json
Expand Up @@ -141,6 +141,12 @@
}
},
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"resources": [
{
Expand Down
Expand Up @@ -43,6 +43,12 @@
}
],
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"resources": [
{
Expand Down
5 changes: 5 additions & 0 deletions internal/command/testdata/show-json/modules/output.json
Expand Up @@ -293,6 +293,11 @@
"module_address": "module.module_test_foo",
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
},
"module.module_test_bar:test": {
"module_address": "module.module_test_bar",
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions internal/command/testdata/show-json/moved-drift/output.json
Expand Up @@ -143,6 +143,12 @@
}
},
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"resources": [
{
Expand Down
6 changes: 6 additions & 0 deletions internal/command/testdata/show-json/moved/output.json
Expand Up @@ -68,6 +68,12 @@
}
},
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"resources": [
{
Expand Down
Expand Up @@ -137,6 +137,12 @@
}
},
"configuration": {
"provider_config": {
"test": {
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"outputs": {
"test": {
Expand Down
15 changes: 9 additions & 6 deletions internal/command/testdata/show-json/nested-modules/output.json
Expand Up @@ -37,9 +37,7 @@
"name": "test",
"provider_name": "registry.terraform.io/hashicorp/test",
"change": {
"actions": [
"create"
],
"actions": ["create"],
"before": null,
"after": {
"ami": "bar-var"
Expand All @@ -53,6 +51,13 @@
}
],
"configuration": {
"provider_config": {
"module.my_module.module.more:test": {
"module_address": "module.my_module.module.more",
"name": "test",
"full_name": "registry.terraform.io/hashicorp/test"
}
},
"root_module": {
"module_calls": {
"my_module": {
Expand All @@ -71,9 +76,7 @@
"provider_config_key": "module.my_module.module.more:test",
"expressions": {
"ami": {
"references": [
"var.test_var"
]
"references": ["var.test_var"]
}
},
"schema_version": 0
Expand Down
@@ -0,0 +1,11 @@
terraform {
required_providers {
test = {
source = "hashicorp2/test"
}
}
}

resource "test_instance" "test" {
ami = "bar"
}
@@ -0,0 +1,11 @@
provider "test" {
region = "somewhere"
}

resource "test_instance" "test" {
ami = "foo"
}

module "child" {
source = "./child"
}