From 7cae76383a4c9c12c8a67609556034616b82a430 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 5 Feb 2021 14:01:58 -0500 Subject: [PATCH] cli: Fix for provider requirements in JSON plan The JSON plan output format includes a serialized, simplified version of the configuration. One component of this config is a map of provider configurations, which includes version constraints. Until now, only version constraints specified in the provider config blocks were exposed in the JSON plan output. This is a deprecated method of specifying provider versions, and the recommended use of a required_providers block resulted in the version constraints being omitted. This commit fixes this with two changes: - When processing the provider configurations from a module, output the fully-merged version constraints for the entire module, instead of any constraints set in the provider configuration block itself; - After all provider configurations are processed, iterate over the required_providers entries to ensure that any configuration-less providers are output to the JSON plan too. No changes are necessary to the structure of the JSON plan output, so this is effectively a semantic level bug fix. --- command/jsonconfig/config.go | 61 +++++- .../provider-version-no-config/main.tf | 21 ++ .../provider-version-no-config/output.json | 184 +++++++++++++++++ .../show-json/provider-version/main.tf | 26 +++ .../show-json/provider-version/output.json | 189 ++++++++++++++++++ configs/config.go | 12 ++ configs/config_test.go | 36 ++++ 7 files changed, 520 insertions(+), 9 deletions(-) create mode 100644 command/testdata/show-json/provider-version-no-config/main.tf create mode 100644 command/testdata/show-json/provider-version-no-config/output.json create mode 100644 command/testdata/show-json/provider-version/main.tf create mode 100644 command/testdata/show-json/provider-version/output.json diff --git a/command/jsonconfig/config.go b/command/jsonconfig/config.go index bbb56231a..d021a8b1f 100644 --- a/command/jsonconfig/config.go +++ b/command/jsonconfig/config.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/terraform" ) @@ -139,19 +140,61 @@ func marshalProviderConfigs( return } + // We want to determine only the provider requirements from this module, + // ignoring any descendants. Disregard any diagnostics when determining + // requirements because we want this marshalling to succeed even if there + // are invalid constraints. + reqs, _ := c.ProviderRequirementsShallow() + + // Add an entry for each provider configuration block in the module. for k, pc := range c.Module.ProviderConfigs { providerFqn := c.ProviderForConfigAddr(addrs.LocalProviderConfig{LocalName: pc.Name}) schema := schemas.ProviderConfig(providerFqn) - 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 + p := providerConfig{ + Name: pc.Name, + Alias: pc.Alias, + ModuleAddress: c.Path.String(), + Expressions: marshalExpressions(pc.Config, schema), + } + + // Store the fully resolved provider version constraint, rather than + // using the version argument in the configuration block. This is both + // future proof (for when we finish the deprecation of the provider config + // version argument) and more accurate (as it reflects the full set of + // constraints, in case there are multiple). + if vc, ok := reqs[providerFqn]; ok { + p.VersionConstraint = getproviders.VersionConstraintsString(vc) + } + + key := opaqueProviderKey(k, c.Path.String()) + + m[key] = p + } + + // Ensure that any required providers with no associated configuration + // block are included in the set. + for k, pr := range c.Module.ProviderRequirements.RequiredProviders { + // If there exists a value for this provider, we have nothing to add + // to it, so skip. + key := opaqueProviderKey(k, c.Path.String()) + if _, exists := m[key]; exists { + continue + } + + // Given no provider configuration block exists, the only fields we can + // fill here are the local name, module address, and version + // constraints. + p := providerConfig{ + Name: pr.Name, + ModuleAddress: c.Path.String(), + } + + if vc, ok := reqs[pr.Type]; ok { + p.VersionConstraint = getproviders.VersionConstraintsString(vc) + } + + m[key] = p } // Must also visit our child modules, recursively. diff --git a/command/testdata/show-json/provider-version-no-config/main.tf b/command/testdata/show-json/provider-version-no-config/main.tf new file mode 100644 index 000000000..c5df9bf79 --- /dev/null +++ b/command/testdata/show-json/provider-version-no-config/main.tf @@ -0,0 +1,21 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + version = ">= 1.2.3" + } + } +} + +variable "test_var" { + default = "bar" +} + +resource "test_instance" "test" { + ami = var.test_var + count = 3 +} + +output "test" { + value = var.test_var +} diff --git a/command/testdata/show-json/provider-version-no-config/output.json b/command/testdata/show-json/provider-version-no-config/output.json new file mode 100644 index 000000000..bef6f9b00 --- /dev/null +++ b/command/testdata/show-json/provider-version-no-config/output.json @@ -0,0 +1,184 @@ +{ + "format_version": "0.1", + "variables": { + "test_var": { + "value": "bar" + } + }, + "planned_values": { + "outputs": { + "test": { + "sensitive": false, + "value": "bar" + } + }, + "root_module": { + "resources": [ + { + "address": "test_instance.test[0]", + "index": 0, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar" + } + }, + { + "address": "test_instance.test[1]", + "index": 1, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar" + } + }, + { + "address": "test_instance.test[2]", + "index": 2, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar" + } + } + ] + } + }, + "prior_state": { + "format_version": "0.1", + "values": { + "outputs": { + "test": { + "sensitive": false, + "value": "bar" + } + }, + "root_module": {} + } + }, + "resource_changes": [ + { + "address": "test_instance.test[0]", + "index": 0, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar" + } + } + }, + { + "address": "test_instance.test[1]", + "index": 1, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar" + } + } + }, + { + "address": "test_instance.test[2]", + "index": 2, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar" + } + } + } + ], + "output_changes": { + "test": { + "actions": [ + "create" + ], + "before": null, + "after": "bar", + "after_unknown": false + } + }, + "configuration": { + "provider_config": { + "test": { + "name": "test", + "version_constraint": ">= 1.2.3" + } + }, + "root_module": { + "outputs": { + "test": { + "expression": { + "references": [ + "var.test_var" + ] + } + } + }, + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "test", + "schema_version": 0, + "expressions": { + "ami": { + "references": [ + "var.test_var" + ] + } + }, + "count_expression": { + "constant_value": 3 + } + } + ], + "variables": { + "test_var": { + "default": "bar" + } + } + } + } +} diff --git a/command/testdata/show-json/provider-version/main.tf b/command/testdata/show-json/provider-version/main.tf new file mode 100644 index 000000000..b6d3cb974 --- /dev/null +++ b/command/testdata/show-json/provider-version/main.tf @@ -0,0 +1,26 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + version = ">= 1.2.3" + } + } +} + +provider "test" { + region = "somewhere" + version = "1.2.3" +} + +variable "test_var" { + default = "bar" +} + +resource "test_instance" "test" { + ami = var.test_var + count = 3 +} + +output "test" { + value = var.test_var +} diff --git a/command/testdata/show-json/provider-version/output.json b/command/testdata/show-json/provider-version/output.json new file mode 100644 index 000000000..33807107d --- /dev/null +++ b/command/testdata/show-json/provider-version/output.json @@ -0,0 +1,189 @@ +{ + "format_version": "0.1", + "variables": { + "test_var": { + "value": "bar" + } + }, + "planned_values": { + "outputs": { + "test": { + "sensitive": false, + "value": "bar" + } + }, + "root_module": { + "resources": [ + { + "address": "test_instance.test[0]", + "index": 0, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar" + } + }, + { + "address": "test_instance.test[1]", + "index": 1, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar" + } + }, + { + "address": "test_instance.test[2]", + "index": 2, + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar" + } + } + ] + } + }, + "prior_state": { + "format_version": "0.1", + "values": { + "outputs": { + "test": { + "sensitive": false, + "value": "bar" + } + }, + "root_module": {} + } + }, + "resource_changes": [ + { + "address": "test_instance.test[0]", + "index": 0, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar" + } + } + }, + { + "address": "test_instance.test[1]", + "index": 1, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar" + } + } + }, + { + "address": "test_instance.test[2]", + "index": 2, + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after_unknown": { + "id": true + }, + "after": { + "ami": "bar" + } + } + } + ], + "output_changes": { + "test": { + "actions": [ + "create" + ], + "before": null, + "after": "bar", + "after_unknown": false + } + }, + "configuration": { + "provider_config": { + "test": { + "name": "test", + "expressions": { + "region": { + "constant_value": "somewhere" + } + }, + "version_constraint": ">= 1.2.3, 1.2.3" + } + }, + "root_module": { + "outputs": { + "test": { + "expression": { + "references": [ + "var.test_var" + ] + } + } + }, + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "test", + "schema_version": 0, + "expressions": { + "ami": { + "references": [ + "var.test_var" + ] + } + }, + "count_expression": { + "constant_value": 3 + } + } + ], + "variables": { + "test_var": { + "default": "bar" + } + } + } + } +} diff --git a/configs/config.go b/configs/config.go index affa93d12..cc8898178 100644 --- a/configs/config.go +++ b/configs/config.go @@ -190,6 +190,18 @@ func (c *Config) ProviderRequirements() (getproviders.Requirements, hcl.Diagnost return reqs, diags } +// ProviderRequirementsShallow searches only the direct receiver for explicit +// and implicit dependencies on providers. Descendant modules are ignored. +// +// If the returned diagnostics includes errors then the resulting Requirements +// may be incomplete. +func (c *Config) ProviderRequirementsShallow() (getproviders.Requirements, hcl.Diagnostics) { + reqs := make(getproviders.Requirements) + diags := c.addProviderRequirements(reqs, false) + + return reqs, diags +} + // ProviderRequirementsByModule searches the full tree of modules under the // receiver for both explicit and implicit dependencies on providers, // constructing a tree where the requirements are broken out by module. diff --git a/configs/config_test.go b/configs/config_test.go index d3a6974b7..17b6a4d3a 100644 --- a/configs/config_test.go +++ b/configs/config_test.go @@ -157,6 +157,42 @@ func TestConfigProviderRequirements(t *testing.T) { } } +func TestConfigProviderRequirementsShallow(t *testing.T) { + cfg, diags := testNestedModuleConfigFromDir(t, "testdata/provider-reqs") + // TODO: Version Constraint Deprecation. + // Once we've removed the version argument from provider configuration + // blocks, this can go back to expected 0 diagnostics. + // assertNoDiagnostics(t, diags) + assertDiagnosticCount(t, diags, 1) + assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated") + + tlsProvider := addrs.NewProvider( + addrs.DefaultRegistryHost, + "hashicorp", "tls", + ) + nullProvider := addrs.NewDefaultProvider("null") + randomProvider := addrs.NewDefaultProvider("random") + impliedProvider := addrs.NewDefaultProvider("implied") + terraformProvider := addrs.NewBuiltInProvider("terraform") + configuredProvider := addrs.NewDefaultProvider("configured") + + got, diags := cfg.ProviderRequirementsShallow() + assertNoDiagnostics(t, diags) + want := getproviders.Requirements{ + // the nullProvider constraint is only from the root module + nullProvider: getproviders.MustParseVersionConstraints("~> 2.0.0"), + randomProvider: getproviders.MustParseVersionConstraints("~> 1.2.0"), + tlsProvider: getproviders.MustParseVersionConstraints("~> 3.0"), + configuredProvider: getproviders.MustParseVersionConstraints("~> 1.4"), + impliedProvider: nil, + terraformProvider: nil, + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } +} + func TestConfigProviderRequirementsByModule(t *testing.T) { cfg, diags := testNestedModuleConfigFromDir(t, "testdata/provider-reqs") // TODO: Version Constraint Deprecation.