From 0c94e20a83261f794c80b5317e0fc3ce9af9c65f Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 20 Feb 2019 14:27:49 -0800 Subject: [PATCH] command/show enhancements and bugfixes * command/jsonconfig: provider config marshaling enhancements This PR fixes a bug wherein the keys in "provider_config" were the "addrs.ProviderConfig", and therefore being overwritten for each module, instead of the intended "addrs.AbsProviderConfig". We realized that there was still opportunity for ambiguity, for example if a user made a provider alias that was the same name as a module, so we opted to use the syntax `modulename:providername(.provideralias)` * command/json*: fixed a bug where we were attempting to lookup schemas with the provider name, instead of provider type. --- command/jsonconfig/config.go | 55 +++++++++++++------ command/jsonplan/plan.go | 6 +- command/jsonplan/values.go | 2 +- command/jsonstate/state.go | 2 +- .../show-json/basic-create/output.json | 2 +- .../show-json/basic-delete/output.json | 2 +- .../show-json/basic-update/output.json | 2 +- .../show-json/modules/foo/main.tf | 2 + .../test-fixtures/show-json/modules/main.tf | 6 +- .../show-json/modules/modules.json | 1 - .../show-json/modules/output.json | 34 +++++++----- 11 files changed, 72 insertions(+), 42 deletions(-) delete mode 100644 command/test-fixtures/show-json/modules/modules.json diff --git a/command/jsonconfig/config.go b/command/jsonconfig/config.go index f673ab0fa..4fc844c8d 100644 --- a/command/jsonconfig/config.go +++ b/command/jsonconfig/config.go @@ -25,10 +25,11 @@ type config struct { // provider configurations are the one concept in Terraform that can span across // module boundaries. type providerConfig struct { - Name string `json:"name,omitempty"` - Alias string `json:"alias,omitempty"` - ModuleAddress string `json:"module_address,omitempty"` - Expressions map[string]interface{} `json:"expressions,omitempty"` + Name string `json:"name,omitempty"` + Alias string `json:"alias,omitempty"` + VersionConstraint string `json:"version_constraint,omitempty"` + ModuleAddress string `json:"module_address,omitempty"` + Expressions map[string]interface{} `json:"expressions,omitempty"` } type module struct { @@ -71,6 +72,10 @@ type resource struct { // ProviderConfigKey is the key into "provider_configs" (shown above) for // the provider configuration that this resource is associated with. + // + // NOTE: If a given resource is in a ModuleCall, and the provider was + // configured outside of the module (in a higher level configuration file), + // the ProviderConfigKey will not match a key in the ProviderConfigs map. ProviderConfigKey string `json:"provider_config_key,omitempty"` // Provisioners is an optional field which describes any provisioners. @@ -114,7 +119,7 @@ func Marshal(c *configs.Config, schemas *terraform.Schemas) ([]byte, error) { marshalProviderConfigs(c, schemas, pcs) output.ProviderConfigs = pcs - rootModule, err := marshalModule(c, schemas) + rootModule, err := marshalModule(c, schemas, "") if err != nil { return nil, err } @@ -135,12 +140,16 @@ func marshalProviderConfigs( for k, pc := range c.Module.ProviderConfigs { schema := schemas.ProviderConfig(pc.Name) - m[k] = providerConfig{ - Name: pc.Name, - Alias: pc.Alias, - ModuleAddress: c.Path.String(), - Expressions: marshalExpressions(pc.Config, schema), + p := providerConfig{ + Name: pc.Name, + Alias: pc.Alias, + ModuleAddress: c.Path.String(), + Expressions: marshalExpressions(pc.Config, schema), + VersionConstraint: pc.Version.Required.String(), } + absPC := opaqueProviderKey(k, c.Path.String()) + + m[absPC] = p } // Must also visit our child modules, recursively. @@ -149,15 +158,15 @@ func marshalProviderConfigs( } } -func marshalModule(c *configs.Config, schemas *terraform.Schemas) (module, error) { +func marshalModule(c *configs.Config, schemas *terraform.Schemas, addr string) (module, error) { var module module var rs []resource - managedResources, err := marshalResources(c.Module.ManagedResources, schemas) + managedResources, err := marshalResources(c.Module.ManagedResources, schemas, addr) if err != nil { return module, err } - dataResources, err := marshalResources(c.Module.DataResources, schemas) + dataResources, err := marshalResources(c.Module.DataResources, schemas, addr) if err != nil { return module, err } @@ -245,8 +254,8 @@ func marshalModuleCalls(c *configs.Config, schemas *terraform.Schemas) map[strin retMC.Expressions = marshalExpressions(mc.Config, schema) - for _, cc := range c.Children { - childModule, _ := marshalModule(cc, schemas) + for name, cc := range c.Children { + childModule, _ := marshalModule(cc, schemas, name) retMC.Module = childModule } ret[mc.Name] = retMC @@ -256,14 +265,14 @@ func marshalModuleCalls(c *configs.Config, schemas *terraform.Schemas) map[strin } -func marshalResources(resources map[string]*configs.Resource, schemas *terraform.Schemas) ([]resource, error) { +func marshalResources(resources map[string]*configs.Resource, schemas *terraform.Schemas, moduleAddr string) ([]resource, error) { var rs []resource for _, v := range resources { r := resource{ Address: v.Addr().String(), Type: v.Type, Name: v.Name, - ProviderConfigKey: v.ProviderConfigAddr().String(), + ProviderConfigKey: opaqueProviderKey(v.ProviderConfigAddr().StringCompact(), moduleAddr), } switch v.Mode { @@ -286,7 +295,7 @@ func marshalResources(resources map[string]*configs.Resource, schemas *terraform } schema, schemaVer := schemas.ResourceTypeConfig( - v.ProviderConfigAddr().StringCompact(), + v.ProviderConfigAddr().Type, v.Mode, v.Type, ) @@ -332,3 +341,13 @@ func marshalResources(resources map[string]*configs.Resource, schemas *terraform }) return rs, nil } + +// opaqueProviderKey generates a unique absProviderConfig-like string from the module +// address and provider +func opaqueProviderKey(provider string, addr string) (key string) { + key = provider + if addr != "" { + key = fmt.Sprintf("%s:%s", addr, provider) + } + return key +} diff --git a/command/jsonplan/plan.go b/command/jsonplan/plan.go index 36a8782b1..43cb9064f 100644 --- a/command/jsonplan/plan.go +++ b/command/jsonplan/plan.go @@ -177,7 +177,11 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform continue } - schema, _ := schemas.ResourceTypeConfig(rc.ProviderAddr.ProviderConfig.StringCompact(), addr.Resource.Resource.Mode, addr.Resource.Resource.Type) + schema, _ := schemas.ResourceTypeConfig( + rc.ProviderAddr.ProviderConfig.Type, + addr.Resource.Resource.Mode, + addr.Resource.Resource.Type, + ) if schema == nil { return fmt.Errorf("no schema found for %s", r.Address) } diff --git a/command/jsonplan/values.go b/command/jsonplan/values.go index 7e3d268ce..3a91399f1 100644 --- a/command/jsonplan/values.go +++ b/command/jsonplan/values.go @@ -154,7 +154,7 @@ func marshalPlanResources(changes *plans.Changes, ris []addrs.AbsResourceInstanc } schema, schemaVer := schemas.ResourceTypeConfig( - resource.ProviderName, + r.ProviderAddr.ProviderConfig.Type, r.Addr.Resource.Resource.Mode, resource.Type, ) diff --git a/command/jsonstate/state.go b/command/jsonstate/state.go index 0d1056c85..aeb355dd8 100644 --- a/command/jsonstate/state.go +++ b/command/jsonstate/state.go @@ -267,7 +267,7 @@ func marshalResources(resources map[string]*states.Resource, schemas *terraform. } schema, _ := schemas.ResourceTypeConfig( - r.ProviderConfig.ProviderConfig.StringCompact(), + r.ProviderConfig.ProviderConfig.Type, r.Addr.Mode, r.Addr.Type, ) diff --git a/command/test-fixtures/show-json/basic-create/output.json b/command/test-fixtures/show-json/basic-create/output.json index f3ae66777..13ffde5ae 100644 --- a/command/test-fixtures/show-json/basic-create/output.json +++ b/command/test-fixtures/show-json/basic-create/output.json @@ -142,7 +142,7 @@ "mode": "managed", "type": "test_instance", "name": "test", - "provider_config_key": "provider.test", + "provider_config_key": "test", "schema_version": 0, "expressions": { "ami": { diff --git a/command/test-fixtures/show-json/basic-delete/output.json b/command/test-fixtures/show-json/basic-delete/output.json index 9b728ab71..392275b2b 100644 --- a/command/test-fixtures/show-json/basic-delete/output.json +++ b/command/test-fixtures/show-json/basic-delete/output.json @@ -132,7 +132,7 @@ "mode": "managed", "type": "test_instance", "name": "test", - "provider_config_key": "provider.test", + "provider_config_key": "test", "schema_version": 0, "expressions": { "ami": { diff --git a/command/test-fixtures/show-json/basic-update/output.json b/command/test-fixtures/show-json/basic-update/output.json index 5598fb0b2..495d8557f 100644 --- a/command/test-fixtures/show-json/basic-update/output.json +++ b/command/test-fixtures/show-json/basic-update/output.json @@ -103,7 +103,7 @@ "mode": "managed", "type": "test_instance", "name": "test", - "provider_config_key": "provider.test", + "provider_config_key": "test", "schema_version": 0, "expressions": { "ami": { diff --git a/command/test-fixtures/show-json/modules/foo/main.tf b/command/test-fixtures/show-json/modules/foo/main.tf index d9e282763..94a022160 100644 --- a/command/test-fixtures/show-json/modules/foo/main.tf +++ b/command/test-fixtures/show-json/modules/foo/main.tf @@ -9,3 +9,5 @@ resource "test_instance" "test" { output "test" { value = var.test_var } + +provider "test" {} diff --git a/command/test-fixtures/show-json/modules/main.tf b/command/test-fixtures/show-json/modules/main.tf index 252923d24..7ed726c60 100644 --- a/command/test-fixtures/show-json/modules/main.tf +++ b/command/test-fixtures/show-json/modules/main.tf @@ -1,9 +1,9 @@ -module "test" { +module "module_test" { source = "./foo" test_var = "baz" } output "test" { - value = module.test.test - depends_on = [module.test] + value = module.module_test.test + depends_on = [module.module_test] } diff --git a/command/test-fixtures/show-json/modules/modules.json b/command/test-fixtures/show-json/modules/modules.json deleted file mode 100644 index 54b418abf..000000000 --- a/command/test-fixtures/show-json/modules/modules.json +++ /dev/null @@ -1 +0,0 @@ -{"Modules":[{"Key":"","Source":"","Dir":"/Users/kristin/go/src/github.com/hashicorp/terraform/command/test-fixtures/show-json/modules"},{"Key":"test","Source":"./foo","Dir":"/Users/kristin/go/src/github.com/hashicorp/terraform/command/test-fixtures/show-json/modules/foo"}]} \ No newline at end of file diff --git a/command/test-fixtures/show-json/modules/output.json b/command/test-fixtures/show-json/modules/output.json index 216bf893b..5c42b69fb 100644 --- a/command/test-fixtures/show-json/modules/output.json +++ b/command/test-fixtures/show-json/modules/output.json @@ -12,7 +12,7 @@ { "resources": [ { - "address": "module.test.test_instance.test[0]", + "address": "module.module_test.test_instance.test[0]", "mode": "managed", "type": "test_instance", "name": "test", @@ -24,7 +24,7 @@ } }, { - "address": "module.test.test_instance.test[1]", + "address": "module.module_test.test_instance.test[1]", "mode": "managed", "type": "test_instance", "name": "test", @@ -36,7 +36,7 @@ } }, { - "address": "module.test.test_instance.test[2]", + "address": "module.module_test.test_instance.test[2]", "mode": "managed", "type": "test_instance", "name": "test", @@ -48,15 +48,15 @@ } } ], - "address": "module.test" + "address": "module.module_test" } ] } }, "resource_changes": [ { - "address": "module.test.test_instance.test[0]", - "module_address": "module.test", + "address": "module.module_test.test_instance.test[0]", + "module_address": "module.module_test", "mode": "managed", "type": "test_instance", "name": "test", @@ -76,8 +76,8 @@ } }, { - "address": "module.test.test_instance.test[1]", - "module_address": "module.test", + "address": "module.module_test.test_instance.test[1]", + "module_address": "module.module_test", "mode": "managed", "type": "test_instance", "name": "test", @@ -97,8 +97,8 @@ } }, { - "address": "module.test.test_instance.test[2]", - "module_address": "module.test", + "address": "module.module_test.test_instance.test[2]", + "module_address": "module.module_test", "mode": "managed", "type": "test_instance", "name": "test", @@ -134,16 +134,16 @@ "test": { "expression": { "references": [ - "module.test.test" + "module.module_test.test" ] }, "depends_on": [ - "module.test" + "module.module_test" ] } }, "module_calls": { - "test": { + "module_test": { "source": "./foo", "expressions": { "test_var": { @@ -166,7 +166,7 @@ "mode": "managed", "type": "test_instance", "name": "test", - "provider_config_key": "provider.test", + "provider_config_key": "module_test:test", "expressions": { "ami": { "references": [ @@ -188,6 +188,12 @@ } } } + }, + "provider_config": { + "module_test:test": { + "module_address": "module_test", + "name": "test" + } } } } \ No newline at end of file