From 18f9ea53b98dd05dcb0b5b6a5866b7d40cb9e5ac Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Mon, 21 Sep 2020 16:32:50 -0400 Subject: [PATCH] command: Providers schema shows required_providers The providers schema command is using the Config.ProviderTypes method, which had not been kept up to date with the changes to provider requirements detection made in Config.ProviderRequirements. This resulted in any currently-unused providers being omitted from the output. This commit changes the ProviderTypes method to use the same underlying logic as ProviderRequirements, which ensures that `required_providers` blocks are taken into account. Includes an integration test case to verify that this fixes the provider schemas command bug. --- .../providers-schema/required/output.json | 41 +++++++++ .../providers-schema/required/provider.tf | 7 ++ configs/config.go | 87 +++++++------------ configs/config_test.go | 1 + .../valid-files/providers-explicit-implied.tf | 8 ++ 5 files changed, 89 insertions(+), 55 deletions(-) create mode 100644 command/testdata/providers-schema/required/output.json create mode 100644 command/testdata/providers-schema/required/provider.tf diff --git a/command/testdata/providers-schema/required/output.json b/command/testdata/providers-schema/required/output.json new file mode 100644 index 000000000..ba58e2f3c --- /dev/null +++ b/command/testdata/providers-schema/required/output.json @@ -0,0 +1,41 @@ +{ + "format_version": "0.1", + "provider_schemas": { + "registry.terraform.io/hashicorp/test": { + "provider": { + "version": 0, + "block": { + "attributes": { + "region": { + "description_kind": "plain", + "optional": true, + "type": "string" + } + }, + "description_kind": "plain" + } + }, + "resource_schemas": { + "test_instance": { + "version": 0, + "block": { + "attributes": { + "ami": { + "type": "string", + "optional": true, + "description_kind": "plain" + }, + "id": { + "type": "string", + "optional": true, + "computed": true, + "description_kind": "plain" + } + }, + "description_kind": "plain" + } + } + } + } + } +} diff --git a/command/testdata/providers-schema/required/provider.tf b/command/testdata/providers-schema/required/provider.tf new file mode 100644 index 000000000..a6475e1bc --- /dev/null +++ b/command/testdata/providers-schema/required/provider.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + } + } +} diff --git a/configs/config.go b/configs/config.go index bd086e25b..affa93d12 100644 --- a/configs/config.go +++ b/configs/config.go @@ -226,33 +226,36 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse var diags hcl.Diagnostics // First we'll deal with the requirements directly in _our_ module... - for _, providerReqs := range c.Module.ProviderRequirements.RequiredProviders { - fqn := providerReqs.Type - if _, ok := reqs[fqn]; !ok { - // We'll at least have an unconstrained dependency then, but might - // add to this in the loop below. - reqs[fqn] = nil + if c.Module.ProviderRequirements != nil { + for _, providerReqs := range c.Module.ProviderRequirements.RequiredProviders { + fqn := providerReqs.Type + if _, ok := reqs[fqn]; !ok { + // We'll at least have an unconstrained dependency then, but might + // add to this in the loop below. + reqs[fqn] = nil + } + // The model of version constraints in this package is still the + // old one using a different upstream module to represent versions, + // so we'll need to shim that out here for now. The two parsers + // don't exactly agree in practice 🙄 so this might produce new errors. + // TODO: Use the new parser throughout this package so we can get the + // better error messages it produces in more situations. + constraints, err := getproviders.ParseVersionConstraints(providerReqs.Requirement.Required.String()) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + // The errors returned by ParseVersionConstraint already include + // the section of input that was incorrect, so we don't need to + // include that here. + Detail: fmt.Sprintf("Incorrect version constraint syntax: %s.", err.Error()), + Subject: providerReqs.Requirement.DeclRange.Ptr(), + }) + } + reqs[fqn] = append(reqs[fqn], constraints...) } - // The model of version constraints in this package is still the - // old one using a different upstream module to represent versions, - // so we'll need to shim that out here for now. The two parsers - // don't exactly agree in practice 🙄 so this might produce new errors. - // TODO: Use the new parser throughout this package so we can get the - // better error messages it produces in more situations. - constraints, err := getproviders.ParseVersionConstraints(providerReqs.Requirement.Required.String()) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid version constraint", - // The errors returned by ParseVersionConstraint already include - // the section of input that was incorrect, so we don't need to - // include that here. - Detail: fmt.Sprintf("Incorrect version constraint syntax: %s.", err.Error()), - Subject: providerReqs.Requirement.DeclRange.Ptr(), - }) - } - reqs[fqn] = append(reqs[fqn], constraints...) } + // Each resource in the configuration creates an *implicit* provider // dependency, though we'll only record it if there isn't already // an explicit dependency on the same provider. @@ -323,11 +326,11 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse // provider version selection in an earlier step and have identified suitable // versions for each provider. func (c *Config) ProviderTypes() []addrs.Provider { - m := make(map[addrs.Provider]struct{}) - c.gatherProviderTypes(m) + // Ignore diagnostics here because they relate to version constraints + reqs, _ := c.ProviderRequirements() - ret := make([]addrs.Provider, 0, len(m)) - for k := range m { + ret := make([]addrs.Provider, 0, len(reqs)) + for k := range reqs { ret = append(ret, k) } sort.Slice(ret, func(i, j int) bool { @@ -336,32 +339,6 @@ func (c *Config) ProviderTypes() []addrs.Provider { return ret } -func (c *Config) gatherProviderTypes(m map[addrs.Provider]struct{}) { - if c == nil { - return - } - - for _, pc := range c.Module.ProviderConfigs { - fqn := c.Module.ProviderForLocalConfig(addrs.LocalProviderConfig{LocalName: pc.Name}) - m[fqn] = struct{}{} - } - for _, rc := range c.Module.ManagedResources { - providerAddr := rc.ProviderConfigAddr() - fqn := c.Module.ProviderForLocalConfig(providerAddr) - m[fqn] = struct{}{} - } - for _, rc := range c.Module.DataResources { - providerAddr := rc.ProviderConfigAddr() - fqn := c.Module.ProviderForLocalConfig(providerAddr) - m[fqn] = struct{}{} - } - - // Must also visit our child modules, recursively. - for _, cc := range c.Children { - cc.gatherProviderTypes(m) - } -} - // ResolveAbsProviderAddr returns the AbsProviderConfig represented by the given // ProviderConfig address, which must not be nil or this method will panic. // diff --git a/configs/config_test.go b/configs/config_test.go index 71b1bbeb1..487c8a20a 100644 --- a/configs/config_test.go +++ b/configs/config_test.go @@ -32,6 +32,7 @@ func TestConfigProviderTypes(t *testing.T) { addrs.NewDefaultProvider("aws"), addrs.NewDefaultProvider("null"), addrs.NewDefaultProvider("template"), + addrs.NewDefaultProvider("test"), } for _, problem := range deep.Equal(got, want) { t.Error(problem) diff --git a/configs/testdata/valid-files/providers-explicit-implied.tf b/configs/testdata/valid-files/providers-explicit-implied.tf index 7216e0433..778b0aa53 100644 --- a/configs/testdata/valid-files/providers-explicit-implied.tf +++ b/configs/testdata/valid-files/providers-explicit-implied.tf @@ -13,3 +13,11 @@ resource "aws_instance" "foo" { resource "null_resource" "foo" { } + +terraform { + required_providers { + test = { + source = "hashicorp/test" + } + } +}