From ac585be079ac51e6bb091ba7e05be961e7e46800 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 9 Feb 2021 08:38:30 -0500 Subject: [PATCH] initial support for parsing configuration_aliases Add support for parsing configuration_aliases in required_providers entries. The decoder needed to be re-written here in order to support the bare reference style usage of provider names so that they match the usage in other location within configuration. The only change to existing handling of the required_providers block is more precise error locations in a couple cases. --- configs/provider_requirements.go | 289 +++++++++++------- configs/provider_requirements_test.go | 78 +---- .../error-files/provider-source-prefix.tf | 8 +- .../valid-modules/provider-aliases/main.tf | 19 ++ 4 files changed, 226 insertions(+), 168 deletions(-) create mode 100644 configs/testdata/valid-modules/provider-aliases/main.tf diff --git a/configs/provider_requirements.go b/configs/provider_requirements.go index ef746f3fa..2c376d3da 100644 --- a/configs/provider_requirements.go +++ b/configs/provider_requirements.go @@ -1,6 +1,8 @@ package configs import ( + "fmt" + version "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/addrs" @@ -17,6 +19,7 @@ type RequiredProvider struct { Type addrs.Provider Requirement VersionConstraint DeclRange hcl.Range + Aliases []addrs.LocalProviderConfig } type RequiredProviders struct { @@ -26,118 +29,27 @@ type RequiredProviders struct { func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Diagnostics) { attrs, diags := block.Body.JustAttributes() + if diags.HasErrors() { + return nil, diags + } + ret := &RequiredProviders{ RequiredProviders: make(map[string]*RequiredProvider), DeclRange: block.DefRange, } + for name, attr := range attrs { - expr, err := attr.Expr.Value(nil) - if err != nil { - diags = append(diags, err...) - } - - // verify that the local name is already localized or produce an error. - nameDiags := checkProviderNameNormalized(name, attr.Expr.Range()) - diags = append(diags, nameDiags...) - rp := &RequiredProvider{ Name: name, DeclRange: attr.Expr.Range(), } - switch { - case expr.Type().IsPrimitiveType(): + // Look for a single static string, in case we have the legacy version-only + // format in the configuration. + if expr, err := attr.Expr.Value(nil); err == nil && expr.Type().IsPrimitiveType() { vc, reqDiags := decodeVersionConstraint(attr) diags = append(diags, reqDiags...) - rp.Requirement = vc - case expr.Type().IsObjectType(): - if expr.Type().HasAttribute("version") { - vc := VersionConstraint{ - DeclRange: attr.Range, - } - constraint := expr.GetAttr("version") - if !constraint.Type().Equals(cty.String) || constraint.IsNull() { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid version constraint", - Detail: "Version must be specified as a string.", - Subject: attr.Expr.Range().Ptr(), - }) - } else { - constraintStr := constraint.AsString() - constraints, err := version.NewConstraint(constraintStr) - if err != nil { - // NewConstraint doesn't return user-friendly errors, so we'll just - // ignore the provided error and produce our own generic one. - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid version constraint", - Detail: "This string does not use correct version constraint syntax.", - Subject: attr.Expr.Range().Ptr(), - }) - } else { - vc.Required = constraints - rp.Requirement = vc - } - } - } - if expr.Type().HasAttribute("source") { - source := expr.GetAttr("source") - if !source.Type().Equals(cty.String) || source.IsNull() { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid source", - Detail: "Source must be specified as a string.", - Subject: attr.Expr.Range().Ptr(), - }) - } else { - rp.Source = source.AsString() - - fqn, sourceDiags := addrs.ParseProviderSourceString(rp.Source) - - if sourceDiags.HasErrors() { - hclDiags := sourceDiags.ToHCL() - // The diagnostics from ParseProviderSourceString don't contain - // source location information because it has no context to compute - // them from, and so we'll add those in quickly here before we - // return. - for _, diag := range hclDiags { - if diag.Subject == nil { - diag.Subject = attr.Expr.Range().Ptr() - } - } - diags = append(diags, hclDiags...) - } else { - rp.Type = fqn - } - } - } - attrTypes := expr.Type().AttributeTypes() - for name := range attrTypes { - if name == "version" || name == "source" { - continue - } - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid required_providers object", - Detail: `required_providers objects can only contain "version" and "source" attributes. To configure a provider, use a "provider" block.`, - Subject: attr.Expr.Range().Ptr(), - }) - break - } - - default: - // should not happen - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid required_providers syntax", - Detail: "required_providers entries must be strings or objects.", - Subject: attr.Expr.Range().Ptr(), - }) - } - - if rp.Type.IsZero() && !diags.HasErrors() { // Don't try to generate an FQN if we've encountered errors pType, err := addrs.ParseProviderPart(rp.Name) if err != nil { diags = append(diags, &hcl.Diagnostic{ @@ -146,12 +58,185 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia Detail: err.Error(), Subject: attr.Expr.Range().Ptr(), }) - } else { - rp.Type = addrs.ImpliedProviderForUnqualifiedType(pType) + continue } + + rp.Requirement = vc + rp.Type = addrs.ImpliedProviderForUnqualifiedType(pType) + ret.RequiredProviders[name] = rp + + continue } - ret.RequiredProviders[rp.Name] = rp + // verify that the local name is already localized or produce an error. + nameDiags := checkProviderNameNormalized(name, attr.Expr.Range()) + if nameDiags.HasErrors() { + diags = append(diags, nameDiags...) + continue + } + + kvs, mapDiags := hcl.ExprMap(attr.Expr) + if mapDiags.HasErrors() { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid required_providers object", + Detail: "required_providers entries must be strings or objects.", + Subject: attr.Expr.Range().Ptr(), + }) + continue + } + + for _, kv := range kvs { + key, keyDiags := kv.Key.Value(nil) + if keyDiags.HasErrors() { + diags = append(diags, keyDiags...) + continue + } + + if key.Type() != cty.String { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid Attribute", + Detail: fmt.Sprintf("Invalid attribute value for provider requirement: %#v", key), + Subject: kv.Key.Range().Ptr(), + }) + continue + } + + switch key.AsString() { + case "version": + vc := VersionConstraint{ + DeclRange: attr.Range, + } + + constraint, valDiags := kv.Value.Value(nil) + if valDiags.HasErrors() || !constraint.Type().Equals(cty.String) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "Version must be specified as a string.", + Subject: kv.Value.Range().Ptr(), + }) + continue + } + + constraintStr := constraint.AsString() + constraints, err := version.NewConstraint(constraintStr) + if err != nil { + // NewConstraint doesn't return user-friendly errors, so we'll just + // ignore the provided error and produce our own generic one. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "This string does not use correct version constraint syntax.", + Subject: kv.Value.Range().Ptr(), + }) + continue + } + + vc.Required = constraints + rp.Requirement = vc + + case "source": + source, err := kv.Value.Value(nil) + if err != nil || !source.Type().Equals(cty.String) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid source", + Detail: "Source must be specified as a string.", + Subject: kv.Value.Range().Ptr(), + }) + continue + } + + fqn, sourceDiags := addrs.ParseProviderSourceString(source.AsString()) + if sourceDiags.HasErrors() { + hclDiags := sourceDiags.ToHCL() + // The diagnostics from ParseProviderSourceString don't contain + // source location information because it has no context to compute + // them from, and so we'll add those in quickly here before we + // return. + for _, diag := range hclDiags { + if diag.Subject == nil { + diag.Subject = kv.Value.Range().Ptr() + } + } + diags = append(diags, hclDiags...) + continue + } + + rp.Source = source.AsString() + rp.Type = fqn + + case "configuration_aliases": + exprs, listDiags := hcl.ExprList(kv.Value) + if listDiags.HasErrors() { + diags = append(diags, listDiags...) + continue + } + + for _, expr := range exprs { + traversal, travDiags := hcl.AbsTraversalForExpr(expr) + if travDiags.HasErrors() { + diags = append(diags, travDiags...) + continue + } + + addr, cfgDiags := ParseProviderConfigCompact(traversal) + if cfgDiags.HasErrors() { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid configuration_aliases value", + Detail: `Configuration aliases can only contain references to local provider configuration names in the format of provider.alias`, + Subject: kv.Value.Range().Ptr(), + }) + continue + } + + if addr.LocalName != name { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid configuration_aliases value", + Detail: fmt.Sprintf(`Configuration aliases must be prefixed with the provider name. Expected %q, but found %q.`, name, addr.LocalName), + Subject: kv.Value.Range().Ptr(), + }) + continue + } + + rp.Aliases = append(rp.Aliases, addr) + } + + default: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid required_providers object", + Detail: `required_providers objects can only contain "version", "source" and "configuration_aliases" attributes. To configure a provider, use a "provider" block.`, + Subject: kv.Key.Range().Ptr(), + }) + break + } + + } + + // finally add the required provider as long as there were no errors + if !diags.HasErrors() { + // if a source was not given, create an implied type + if rp.Type.IsZero() { + pType, err := addrs.ParseProviderPart(rp.Name) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid provider name", + Detail: err.Error(), + Subject: attr.Expr.Range().Ptr(), + }) + } else { + rp.Type = addrs.ImpliedProviderForUnqualifiedType(pType) + } + } + + ret.RequiredProviders[rp.Name] = rp + } } return ret, diags diff --git a/configs/provider_requirements_test.go b/configs/provider_requirements_test.go index bf4d99882..69cac1b5b 100644 --- a/configs/provider_requirements_test.go +++ b/configs/provider_requirements_test.go @@ -185,15 +185,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "my-test": { - Name: "my-test", - Source: "some/invalid/provider/source/test", - Requirement: testVC("~>2.0.0"), - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, Error: "Invalid provider source string", }, @@ -213,15 +206,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "my_test": { - Name: "my_test", - Type: addrs.Provider{}, - Requirement: testVC("~>2.0.0"), - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, Error: "Invalid provider local name", }, @@ -241,15 +227,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "MYTEST": { - Name: "MYTEST", - Type: addrs.Provider{}, - Requirement: testVC("~>2.0.0"), - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, Error: "Invalid provider local name", }, @@ -270,15 +249,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "my-test": { - Name: "my-test", - Source: "mycloud/test", - Type: addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"), - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, Error: "Invalid version constraint", }, @@ -296,15 +268,10 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "test": { - Name: "test", - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, - Error: "Invalid required_providers syntax", + Error: "Invalid required_providers object", }, "invalid source attribute type": { Block: &hcl.Block{ @@ -322,13 +289,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "my-test": { - Name: "my-test", - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, Error: "Invalid source", }, @@ -350,16 +312,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { DefRange: blockRange, }, Want: &RequiredProviders{ - RequiredProviders: map[string]*RequiredProvider{ - "my-test": { - Name: "my-test", - Source: "mycloud/test", - Type: addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"), - Requirement: testVC("2.0.0"), - DeclRange: mockRange, - }, - }, - DeclRange: blockRange, + RequiredProviders: map[string]*RequiredProvider{}, + DeclRange: blockRange, }, Error: "Invalid required_providers object", }, @@ -370,7 +324,7 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { got, diags := decodeRequiredProvidersBlock(test.Block) if diags.HasErrors() { if test.Error == "" { - t.Fatalf("unexpected error") + t.Fatalf("unexpected error: %v", diags) } if gotErr := diags[0].Summary; gotErr != test.Error { t.Errorf("wrong error, got %q, want %q", gotErr, test.Error) diff --git a/configs/testdata/error-files/provider-source-prefix.tf b/configs/testdata/error-files/provider-source-prefix.tf index 99cd76df7..96811699f 100644 --- a/configs/testdata/error-files/provider-source-prefix.tf +++ b/configs/testdata/error-files/provider-source-prefix.tf @@ -1,10 +1,10 @@ terraform { required_providers { - usererror = { # ERROR: Invalid provider type - source = "foo/terraform-provider-foo" + usererror = { + source = "foo/terraform-provider-foo" # ERROR: Invalid provider type } - badname = { # ERROR: Invalid provider type - source = "foo/terraform-foo" + badname = { + source = "foo/terraform-foo" # ERROR: Invalid provider type } } } diff --git a/configs/testdata/valid-modules/provider-aliases/main.tf b/configs/testdata/valid-modules/provider-aliases/main.tf new file mode 100644 index 000000000..b9795b30d --- /dev/null +++ b/configs/testdata/valid-modules/provider-aliases/main.tf @@ -0,0 +1,19 @@ +terraform { + required_providers { + foo-test = { + source = "foo/test" + // TODO: these are strings until the parsing code is refactored to allow + // raw references + configuration_aliases = [foo-test.a, foo-test.b] + } + } +} + +resource "test_instance" "explicit" { + provider = foo-test.a +} + +data "test_resource" "explicit" { + provider = foo-test.b +} +