configs: require normalized provider local names (#24945)

* addrs: replace NewLegacyProvider with NewDefaultProvider in ParseProviderSourceString

ParseProviderSourceString was still defaulting to NewLegacyProvider when
encountering single-part strings. This has been fixed.

This commit also adds a new function, IsProviderPartNormalized, which
returns a bool indicating if the string given is the same as a
normalized version (as normalized by ParseProviderPart) or an error.
This is intended for use by the configs package when decoding provider
configurations.

* terraform: fix provider local names in tests

* configs: validate that all provider names are normalized

The addrs package normalizes all source strings, but not the local
names. This caused very odd behavior if for e.g. a provider local name
was capitalized in one place and not another. We considered enabling
case-sensitivity for provider local names, but decided that since this
was not something that worked in previous versions of terraform (and we
have yet to encounter any use cases for this feature) we could generate
an error if the provider local name is not normalized. This error also
provides instructions on how to fix it.

* configs: refactor decodeProviderRequirements to consistently not set an FQN when there are errors

addrs/provider.go

@@ -272,8 +272,7 @@ func ParseProviderSourceString(str string) (Provider, tfdiags.Diagnostics) {
 	ret.Hostname = DefaultRegistryHost
 
 	if len(parts) == 1 {
-		// FIXME: update this to NewDefaultProvider in the provider source release
-		return NewLegacyProvider(parts[0]), diags
+		return NewDefaultProvider(parts[0]), diags
 	}
 
 	if len(parts) >= 2 {
@@ -406,3 +405,15 @@ func MustParseProviderPart(given string) string {
 	}
 	return result
 }
+
+// IsProviderPartNormalized compares a given string to the result of ParseProviderPart(string)
+func IsProviderPartNormalized(str string) (bool, error) {
+	normalized, err := ParseProviderPart(str)
+	if err != nil {
+		return false, err
+	}
+	if str == normalized {
+		return true, nil
+	}
+	return false, nil
+}

addrs/provider_test.go

@@ -282,20 +282,15 @@ func TestParseProviderSourceStr(t *testing.T) {
 		"aws": {
 			Provider{
 				Type:      "aws",
-				Namespace: "-",
+				Namespace: "hashicorp",
 				Hostname:  DefaultRegistryHost,
 			},
 			false,
 		},
 		"AWS": {
 			Provider{
-				// No case folding here because we're currently handling this
-				// as a legacy one. When this changes to be a _default_
-				// address in future (registry.terraform.io/hashicorp/aws)
-				// then we should start applying case folding to it, making
-				// Type appear as "aws" here instead.
-				Type:      "AWS",
-				Namespace: "-",
+				Type:      "aws",
+				Namespace: "hashicorp",
 				Hostname:  DefaultRegistryHost,
 			},
 			false,

configs/provider.go

@@ -40,8 +40,15 @@ func decodeProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) {
 	content, config, moreDiags := block.Body.PartialContent(providerBlockSchema)
 	diags = append(diags, moreDiags...)
 
+	// Provider names must be localized. Produce an error with a message
+	// indicating the action the user can take to fix this message if the local
+	// name is not localized.
+	name := block.Labels[0]
+	nameDiags := checkProviderNameNormalized(name, block.DefRange)
+	diags = append(diags, nameDiags...)
+
 	provider := &Provider{
-		Name:      block.Labels[0],
+		Name:      name,
 		NameRange: block.LabelRanges[0],
 		Config:    config,
 		DeclRange: block.DefRange,
@@ -207,3 +214,31 @@ var providerBlockSchema = &hcl.BodySchema{
 		{Type: "locals"},
 	},
 }
+
+// checkProviderNameNormalized verifies that the given string is already
+// normalized and returns an error if not.
+func checkProviderNameNormalized(name string, declrange hcl.Range) hcl.Diagnostics {
+	var diags hcl.Diagnostics
+	// verify that the provider local name is normalized
+	normalized, err := addrs.IsProviderPartNormalized(name)
+	if err != nil {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid provider local name",
+			Detail:   fmt.Sprintf("%s is an invalid provider local name: %s", name, err),
+			Subject:  &declrange,
+		})
+		return diags
+	}
+	if !normalized {
+		// we would have returned this error already
+		normalizedProvider, _ := addrs.ParseProviderPart(name)
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid provider local name",
+			Detail:   fmt.Sprintf("Provider names must be normalized. Replace %q with %q to fix this error.", name, normalizedProvider),
+			Subject:  &declrange,
+		})
+	}
+	return diags
+}

configs/provider_meta.go

@@ -13,10 +13,13 @@ type ProviderMeta struct {
 }
 
 func decodeProviderMetaBlock(block *hcl.Block) (*ProviderMeta, hcl.Diagnostics) {
+	// verify that the local name is already localized or produce an error.
+	diags := checkProviderNameNormalized(block.Labels[0], block.DefRange)
+
 	return &ProviderMeta{
 		Provider:      block.Labels[0],
 		ProviderRange: block.LabelRanges[0],
 		Config:        block.Body,
 		DeclRange:     block.DefRange,
-	}, nil
+	}, diags
 }

configs/provider_requirements.go

@@ -35,6 +35,10 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia
 			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(),
@@ -69,6 +73,7 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia
 			}
 			if expr.Type().HasAttribute("source") {
 				rp.Source = expr.GetAttr("source").AsString()
+
 				fqn, sourceDiags := addrs.ParseProviderSourceString(rp.Source)
 
 				if sourceDiags.HasErrors() {
@@ -98,7 +103,7 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia
 			})
 		}
 
-		if rp.Type.IsZero() {
+		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{

configs/provider_requirements_test.go

@@ -78,8 +78,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 				Type: "required_providers",
 				Body: hcltest.MockBody(&hcl.BodyContent{
 					Attributes: hcl.Attributes{
-						"my_test": {
-							Name: "my_test",
+						"my-test": {
+							Name: "my-test",
 							Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{
 								"source":  cty.StringVal("mycloud/test"),
 								"version": cty.StringVal("2.0.0"),
@@ -91,8 +91,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 			},
 			Want: &RequiredProviders{
 				RequiredProviders: map[string]*RequiredProvider{
-					"my_test": {
-						Name:        "my_test",
+					"my-test": {
+						Name:        "my-test",
 						Source:      "mycloud/test",
 						Type:        addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"),
 						Requirement: testVC("2.0.0"),
@@ -111,8 +111,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 							Name: "legacy",
 							Expr: hcltest.MockExprLiteral(cty.StringVal("1.0.0")),
 						},
-						"my_test": {
-							Name: "my_test",
+						"my-test": {
+							Name: "my-test",
 							Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{
 								"source":  cty.StringVal("mycloud/test"),
 								"version": cty.StringVal("2.0.0"),
@@ -130,8 +130,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 						Requirement: testVC("1.0.0"),
 						DeclRange:   mockRange,
 					},
-					"my_test": {
-						Name:        "my_test",
+					"my-test": {
+						Name:        "my-test",
 						Source:      "mycloud/test",
 						Type:        addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"),
 						Requirement: testVC("2.0.0"),
@@ -173,8 +173,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 				Type: "required_providers",
 				Body: hcltest.MockBody(&hcl.BodyContent{
 					Attributes: hcl.Attributes{
-						"my_test": {
-							Name: "my_test",
+						"my-test": {
+							Name: "my-test",
 							Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{
 								"source":  cty.StringVal("some/invalid/provider/source/test"),
 								"version": cty.StringVal("~>2.0.0"),
@@ -186,10 +186,9 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 			},
 			Want: &RequiredProviders{
 				RequiredProviders: map[string]*RequiredProvider{
-					"my_test": {
-						Name:        "my_test",
+					"my-test": {
+						Name:        "my-test",
 						Source:      "some/invalid/provider/source/test",
-						Type:        addrs.Provider{},
 						Requirement: testVC("~>2.0.0"),
 						DeclRange:   mockRange,
 					},
@@ -198,7 +197,7 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 			},
 			Error: "Invalid provider source string",
 		},
-		"localname is invalid provider name": {
+		"invalid localname": {
 			Block: &hcl.Block{
 				Type: "required_providers",
 				Body: hcltest.MockBody(&hcl.BodyContent{
@@ -224,15 +223,43 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 				},
 				DeclRange: blockRange,
 			},
-			Error: "Invalid provider name",
+			Error: "Invalid provider local name",
+		},
+		"invalid localname caps": {
+			Block: &hcl.Block{
+				Type: "required_providers",
+				Body: hcltest.MockBody(&hcl.BodyContent{
+					Attributes: hcl.Attributes{
+						"MYTEST": {
+							Name: "MYTEST",
+							Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{
+								"version": cty.StringVal("~>2.0.0"),
+							})),
+						},
+					},
+				}),
+				DefRange: blockRange,
+			},
+			Want: &RequiredProviders{
+				RequiredProviders: map[string]*RequiredProvider{
+					"MYTEST": {
+						Name:        "MYTEST",
+						Type:        addrs.Provider{},
+						Requirement: testVC("~>2.0.0"),
+						DeclRange:   mockRange,
+					},
+				},
+				DeclRange: blockRange,
+			},
+			Error: "Invalid provider local name",
 		},
 		"version constraint error": {
 			Block: &hcl.Block{
 				Type: "required_providers",
 				Body: hcltest.MockBody(&hcl.BodyContent{
 					Attributes: hcl.Attributes{
-						"my_test": {
-							Name: "my_test",
+						"my-test": {
+							Name: "my-test",
 							Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{
 								"source":  cty.StringVal("mycloud/test"),
 								"version": cty.StringVal("invalid"),
@@ -244,8 +271,8 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 			},
 			Want: &RequiredProviders{
 				RequiredProviders: map[string]*RequiredProvider{
-					"my_test": {
-						Name:      "my_test",
+					"my-test": {
+						Name:      "my-test",
 						Source:    "mycloud/test",
 						Type:      addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"),
 						DeclRange: mockRange,
@@ -272,7 +299,6 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) {
 				RequiredProviders: map[string]*RequiredProvider{
 					"test": {
 						Name:      "test",
-						Type:      addrs.NewDefaultProvider("test"),
 						DeclRange: mockRange,
 					},
 				},

configs/resource.go

@@ -418,8 +418,13 @@ func decodeProviderConfigRef(expr hcl.Expression, argName string) (*ProviderConf
 		return nil, diags
 	}
 
+	// verify that the provider local name is normalized
+	name := traversal.RootName()
+	nameDiags := checkProviderNameNormalized(name, traversal[0].SourceRange())
+	diags = append(diags, nameDiags...)
+
 	ret := &ProviderConfigRef{
-		Name:      traversal.RootName(),
+		Name:      name,
 		NameRange: traversal[0].SourceRange(),
 	}
 

configs/testdata/invalid-files/provider-localname-normalization.tf

@@ -0,0 +1,20 @@
+terraform {
+  required_providers {
+    test = {
+      source = "mycorp/test"
+    }
+  }
+}
+
+provider "TEST" {
+
+}
+
+resource test_resource "test" {
+  // this resource is (implicitly) provided by "mycorp/test"
+}
+
+resource test_resource "TEST" {
+  // this resource is (explicitly) provided by "hashicorp/test"
+  provider = TEST
+}

terraform/testdata/transform-provider-fqns-module/child/main.tf

@@ -1,11 +1,11 @@
 terraform {
   required_providers {
-    your_aws = {
+    your-aws = {
       source = "hashicorp/aws"
     }
   }
 }
 
 resource "aws_instance" "web" {
-  provider = "your_aws"
+  provider = "your-aws"
 }

terraform/testdata/transform-provider-fqns-module/main.tf

@@ -1,11 +1,11 @@
 terraform {
   required_providers {
-    my_aws = {
+    my-aws = {
       source = "hashicorp/aws"
     }
   }
 }
 
 resource "aws_instance" "web" {
-  provider = "my_aws"
+  provider = "my-aws"
 }

terraform/testdata/transform-provider-fqns/main.tf

@@ -1,11 +1,11 @@
 terraform {
   required_providers {
-    my_aws = {
+    my-aws = {
       source = "hashicorp/aws"
     }
   }
 }
 
 resource "aws_instance" "web" {
-  provider = "my_aws"
+  provider = "my-aws"
 }