From 0ce040405cfeacc8c32759aa457ccc81c6b3c81f Mon Sep 17 00:00:00 2001 From: nozaq Date: Sat, 19 Feb 2022 00:19:39 +0900 Subject: [PATCH] jsonconfig: fix provider mappings with same names --- internal/command/command_test.go | 3 +- internal/command/jsonconfig/config.go | 22 +-- internal/command/jsonconfig/config_test.go | 100 ++++++++++++ internal/command/show_test.go | 3 +- .../provider-aliasing-conflict/child/main.tf | 11 ++ .../provider-aliasing-conflict/main.tf | 11 ++ .../provider-aliasing-conflict/output.json | 143 ++++++++++++++++++ .../provider-aliasing-default/child/main.tf | 3 +- .../provider-aliasing-default/output.json | 35 ++--- 9 files changed, 302 insertions(+), 29 deletions(-) create mode 100644 internal/command/jsonconfig/config_test.go create mode 100644 internal/command/testdata/show-json/provider-aliasing-conflict/child/main.tf create mode 100644 internal/command/testdata/show-json/provider-aliasing-conflict/main.tf create mode 100644 internal/command/testdata/show-json/provider-aliasing-conflict/output.json diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 4a56ecc73..2f75ec30d 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -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), }, } } diff --git a/internal/command/jsonconfig/config.go b/internal/command/jsonconfig/config.go index 9670fd5e6..4727bc098 100644 --- a/internal/command/jsonconfig/config.go +++ b/internal/command/jsonconfig/config.go @@ -233,7 +233,7 @@ func marshalProviderConfigs( if c.Parent != nil { parentKey := opaqueProviderKey(pr.Name, c.Parent.Path.String()) - p.parentKey = findSourceProviderKey(parentKey, m) + p.parentKey = findSourceProviderKey(parentKey, p.FullName, m) } m[key] = p @@ -255,7 +255,7 @@ func marshalProviderConfigs( Name: req.Type, FullName: req.String(), ModuleAddress: c.Path.String(), - parentKey: findSourceProviderKey(parentKey, m), + parentKey: findSourceProviderKey(parentKey, req.String(), m), } m[key] = p @@ -287,7 +287,7 @@ func marshalProviderConfigs( key := opaqueProviderKey(moduleProviderName, cc.Path.String()) parentKey := opaqueProviderKey(parentProviderName, cc.Parent.Path.String()) - p.parentKey = findSourceProviderKey(parentKey, m) + p.parentKey = findSourceProviderKey(parentKey, p.FullName, m) m[key] = p } @@ -545,14 +545,18 @@ func opaqueProviderKey(provider string, addr string) (key string) { // 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, m map[string]providerConfig) string { - parentKey := startKey - for { - parent, exists := m[parentKey] - if !exists || parent.parentKey == "" { +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 = parent.parentKey + + parentKey = key + key = parent.parentKey } return parentKey diff --git a/internal/command/jsonconfig/config_test.go b/internal/command/jsonconfig/config_test.go new file mode 100644 index 000000000..69aeae3f0 --- /dev/null +++ b/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) + } + } +} diff --git a/internal/command/show_test.go b/internal/command/show_test.go index 00a9e555a..80ae6a9f2 100644 --- a/internal/command/show_test.go +++ b/internal/command/show_test.go @@ -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() diff --git a/internal/command/testdata/show-json/provider-aliasing-conflict/child/main.tf b/internal/command/testdata/show-json/provider-aliasing-conflict/child/main.tf new file mode 100644 index 000000000..2479df335 --- /dev/null +++ b/internal/command/testdata/show-json/provider-aliasing-conflict/child/main.tf @@ -0,0 +1,11 @@ +terraform { + required_providers { + test = { + source = "hashicorp2/test" + } + } +} + +resource "test_instance" "test" { + ami = "bar" +} diff --git a/internal/command/testdata/show-json/provider-aliasing-conflict/main.tf b/internal/command/testdata/show-json/provider-aliasing-conflict/main.tf new file mode 100644 index 000000000..2d8bc0b90 --- /dev/null +++ b/internal/command/testdata/show-json/provider-aliasing-conflict/main.tf @@ -0,0 +1,11 @@ +provider "test" { + region = "somewhere" +} + +resource "test_instance" "test" { + ami = "foo" +} + +module "child" { + source = "./child" +} diff --git a/internal/command/testdata/show-json/provider-aliasing-conflict/output.json b/internal/command/testdata/show-json/provider-aliasing-conflict/output.json new file mode 100644 index 000000000..6b7ee48c8 --- /dev/null +++ b/internal/command/testdata/show-json/provider-aliasing-conflict/output.json @@ -0,0 +1,143 @@ +{ + "format_version": "1.0", + "terraform_version": "1.1.0-dev", + "planned_values": { + "root_module": { + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "foo" + }, + "sensitive_values": {} + } + ], + "child_modules": [ + { + "resources": [ + { + "address": "module.child.test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp2/test", + "schema_version": 0, + "values": { + "ami": "bar" + }, + "sensitive_values": {} + } + ], + "address": "module.child" + } + ] + } + }, + "resource_changes": [ + { + "address": "module.child.test_instance.test", + "module_address": "module.child", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp2/test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after": { + "ami": "bar" + }, + "after_unknown": { + "id": true + }, + "before_sensitive": false, + "after_sensitive": {} + } + }, + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after": { + "ami": "foo" + }, + "after_unknown": { + "id": true + }, + "before_sensitive": false, + "after_sensitive": {} + } + } + ], + "configuration": { + "provider_config": { + "test": { + "name": "test", + "full_name": "registry.terraform.io/hashicorp/test", + "expressions": { + "region": { + "constant_value": "somewhere" + } + } + }, + "module.child:test": { + "module_address": "module.child", + "name": "test", + "full_name": "registry.terraform.io/hashicorp2/test" + } + }, + "root_module": { + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "test", + "expressions": { + "ami": { + "constant_value": "foo" + } + }, + "schema_version": 0 + } + ], + "module_calls": { + "child": { + "source": "./child", + "module": { + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "module.child:test", + "expressions": { + "ami": { + "constant_value": "bar" + } + }, + "schema_version": 0 + } + ] + } + } + } + } + } +} diff --git a/internal/command/testdata/show-json/provider-aliasing-default/child/main.tf b/internal/command/testdata/show-json/provider-aliasing-default/child/main.tf index 1ff9e8969..b865e3e14 100644 --- a/internal/command/testdata/show-json/provider-aliasing-default/child/main.tf +++ b/internal/command/testdata/show-json/provider-aliasing-default/child/main.tf @@ -11,7 +11,8 @@ resource "test_instance" "test" { } module "with_requirement" { - source = "./nested" + source = "./nested" + depends_on = [module.no_requirements] } module "no_requirements" { diff --git a/internal/command/testdata/show-json/provider-aliasing-default/output.json b/internal/command/testdata/show-json/provider-aliasing-default/output.json index d59e111b4..d6b170643 100644 --- a/internal/command/testdata/show-json/provider-aliasing-default/output.json +++ b/internal/command/testdata/show-json/provider-aliasing-default/output.json @@ -35,23 +35,6 @@ ], "address": "module.child", "child_modules": [ - { - "resources": [ - { - "address": "module.child.module.with_requirement.test_instance.test", - "mode": "managed", - "type": "test_instance", - "name": "test", - "provider_name": "registry.terraform.io/hashicorp/test", - "schema_version": 0, - "values": { - "ami": "baz" - }, - "sensitive_values": {} - } - ], - "address": "module.child.module.with_requirement" - }, { "resources": [ { @@ -68,6 +51,23 @@ } ], "address": "module.child.module.no_requirements" + }, + { + "resources": [ + { + "address": "module.child.module.with_requirement.test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "baz" + }, + "sensitive_values": {} + } + ], + "address": "module.child.module.with_requirement" } ] } @@ -243,6 +243,7 @@ }, "with_requirement": { "source": "./nested", + "depends_on": ["module.no_requirements"], "module": { "resources": [ {