AbsProviderConfig to use addrs.Module

Change ModuleInstance to Module in AbsProviderConfig, because providers
need to be handled before module expansion, and should not be used
defined inside an expanded module at all.

Renaming of the addrs type can happen later, when there's less work
in-flight around provider configuration.

addrs/provider_config.go

@@ -2,6 +2,7 @@ package addrs
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/zclconf/go-cty/cty"
@@ -86,7 +87,7 @@ func (pc LocalProviderConfig) StringCompact() string {
 // AbsProviderConfig is the absolute address of a provider configuration
 // within a particular module instance.
 type AbsProviderConfig struct {
-	Module   ModuleInstance
+	Module   Module
 	Provider Provider
 	Alias    string
 }
@@ -101,7 +102,6 @@ var _ ProviderConfig = AbsProviderConfig{}
 //     provider["registry.terraform.io/hashicorp/aws"].foo
 //     module.bar.provider["registry.terraform.io/hashicorp/aws"]
 //     module.bar.module.baz.provider["registry.terraform.io/hashicorp/aws"].foo
-//     module.foo[1].provider["registry.terraform.io/hashicorp/aws"].foo
 //
 // This type of address is used, for example, to record the relationships
 // between resources and provider configurations in the state structure.
@@ -109,9 +109,23 @@ var _ ProviderConfig = AbsProviderConfig{}
 // messages that refer to provider configurations.
 func ParseAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, tfdiags.Diagnostics) {
 	modInst, remain, diags := parseModuleInstancePrefix(traversal)
-	ret := AbsProviderConfig{
+	var ret AbsProviderConfig
-		Module: modInst,
+
+	// Providers cannot resolve within module instances, so verify that there
+	// are no instance keys in the module path before converting to a Module.
+	for _, step := range modInst {
+		if step.InstanceKey != NoKey {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Invalid provider configuration address",
+				Detail:   "Provider address cannot contain module indexes",
+				Subject:  remain.SourceRange().Ptr(),
+			})
+			return ret, diags
+		}
 	}
+	ret.Module = modInst.Module()
+
 	if len(remain) < 2 || remain.RootName() != "provider" {
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
@@ -223,16 +237,28 @@ func ParseLegacyAbsProviderConfigStr(str string) (AbsProviderConfig, tfdiags.Dia
 //     provider.aws.foo
 //     module.bar.provider.aws
 //     module.bar.module.baz.provider.aws.foo
-//     module.foo[1].provider.aws.foo
 //
 // This type of address is used in legacy state and may appear in state v4 if
 // the provider config addresses have not been normalized to include provider
 // FQN.
 func ParseLegacyAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, tfdiags.Diagnostics) {
 	modInst, remain, diags := parseModuleInstancePrefix(traversal)
-	ret := AbsProviderConfig{
+	var ret AbsProviderConfig
-		Module: modInst,
+
+	// Providers cannot resolve within module instances, so verify that there
+	// are no instance keys in the module path before converting to a Module.
+	for _, step := range modInst {
+		if step.InstanceKey != NoKey {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Invalid provider configuration address",
+				Detail:   "Provider address cannot contain module indexes",
+				Subject:  remain.SourceRange().Ptr(),
+			})
+			return ret, diags
+		}
 	}
+	ret.Module = modInst.Module()
 
 	if len(remain) < 2 || remain.RootName() != "provider" {
 		diags = diags.Append(&hcl.Diagnostic{
@@ -287,7 +313,7 @@ func ParseLegacyAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, t
 // the given type inside the recieving module instance.
 func (m ModuleInstance) ProviderConfigDefault(provider Provider) AbsProviderConfig {
 	return AbsProviderConfig{
-		Module:   m,
+		Module:   m.Module(),
 		Provider: provider,
 	}
 }
@@ -296,7 +322,7 @@ func (m ModuleInstance) ProviderConfigDefault(provider Provider) AbsProviderConf
 // the given type and alias inside the recieving module instance.
 func (m ModuleInstance) ProviderConfigAliased(provider Provider, alias string) AbsProviderConfig {
 	return AbsProviderConfig{
-		Module:   m,
+		Module:   m.Module(),
 		Provider: provider,
 		Alias:    alias,
 	}
@@ -359,16 +385,16 @@ func (pc AbsProviderConfig) LegacyString() string {
 // 	module.module-name.provider["example.com/namespace/name"]
 // 	module.module-name.provider["example.com/namespace/name"].alias
 func (pc AbsProviderConfig) String() string {
-	if pc.Alias != "" {
+	var parts []string
-		if len(pc.Module) == 0 {
+	if len(pc.Module) > 0 {
-			return fmt.Sprintf("%s[%q].%s", "provider", pc.Provider.String(), pc.Alias)
+		parts = append(parts, pc.Module.String())
-		} else {
-			return fmt.Sprintf("%s.%s[%q].%s", pc.Module.String(), "provider", pc.Provider.String(), pc.Alias)
-		}
-	}
-	if len(pc.Module) == 0 {
-		return fmt.Sprintf("%s[%q]", "provider", pc.Provider.String())
 	}
 
-	return fmt.Sprintf("%s.%s[%q]", pc.Module.String(), "provider", pc.Provider.String())
+	parts = append(parts, fmt.Sprintf("provider[%q]", pc.Provider))
+
+	if pc.Alias != "" {
+		parts = append(parts, pc.Alias)
+	}
+
+	return strings.Join(parts, ".")
 }

addrs/provider_config_test.go

@@ -18,7 +18,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
 		{
 			`provider["registry.terraform.io/hashicorp/aws"]`,
 			AbsProviderConfig{
-				Module: RootModuleInstance,
+				Module: RootModule,
 				Provider: Provider{
 					Type:      "aws",
 					Namespace: "hashicorp",
@@ -30,7 +30,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
 		{
 			`provider["registry.terraform.io/hashicorp/aws"].foo`,
 			AbsProviderConfig{
-				Module: RootModuleInstance,
+				Module: RootModule,
 				Provider: Provider{
 					Type:      "aws",
 					Namespace: "hashicorp",
@@ -43,11 +43,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
 		{
 			`module.baz.provider["registry.terraform.io/hashicorp/aws"]`,
 			AbsProviderConfig{
-				Module: ModuleInstance{
+				Module: Module{"baz"},
-					{
-						Name: "baz",
-					},
-				},
 				Provider: Provider{
 					Type:      "aws",
 					Namespace: "hashicorp",
@@ -59,11 +55,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
 		{
 			`module.baz.provider["registry.terraform.io/hashicorp/aws"].foo`,
 			AbsProviderConfig{
-				Module: ModuleInstance{
+				Module: Module{"baz"},
-					{
-						Name: "baz",
-					},
-				},
 				Provider: Provider{
 					Type:      "aws",
 					Namespace: "hashicorp",
@@ -75,57 +67,18 @@ func TestParseAbsProviderConfig(t *testing.T) {
 		},
 		{
 			`module.baz["foo"].provider["registry.terraform.io/hashicorp/aws"]`,
-			AbsProviderConfig{
+			AbsProviderConfig{},
-				Module: ModuleInstance{
+			`Provider address cannot contain module indexes`,
-					{
-						Name:        "baz",
-						InstanceKey: StringKey("foo"),
-					},
-				},
-				Provider: Provider{
-					Type:      "aws",
-					Namespace: "hashicorp",
-					Hostname:  "registry.terraform.io",
-				},
-			},
-			``,
 		},
 		{
 			`module.baz[1].provider["registry.terraform.io/hashicorp/aws"]`,
-			AbsProviderConfig{
+			AbsProviderConfig{},
-				Module: ModuleInstance{
+			`Provider address cannot contain module indexes`,
-					{
-						Name:        "baz",
-						InstanceKey: IntKey(1),
-					},
-				},
-				Provider: Provider{
-					Type:      "aws",
-					Namespace: "hashicorp",
-					Hostname:  "registry.terraform.io",
-				},
-			},
-			``,
 		},
 		{
 			`module.baz[1].module.bar.provider["registry.terraform.io/hashicorp/aws"]`,
-			AbsProviderConfig{
+			AbsProviderConfig{},
-				Module: ModuleInstance{
+			`Provider address cannot contain module indexes`,
-					{
-						Name:        "baz",
-						InstanceKey: IntKey(1),
-					},
-					{
-						Name: "bar",
-					},
-				},
-				Provider: Provider{
-					Type:      "aws",
-					Namespace: "hashicorp",
-					Hostname:  "registry.terraform.io",
-				},
-			},
-			``,
 		},
 		{
 			`aws`,
@@ -206,21 +159,21 @@ func TestAbsProviderConfigString(t *testing.T) {
 	}{
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance,
+				Module:   RootModule,
 				Provider: NewLegacyProvider("foo"),
 			},
 			`provider["registry.terraform.io/-/foo"]`,
 		},
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance.Child("child_module", NoKey),
+				Module:   RootModule.Child("child_module"),
 				Provider: NewLegacyProvider("foo"),
 			},
 			`module.child_module.provider["registry.terraform.io/-/foo"]`,
 		},
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance,
+				Module:   RootModule,
 				Alias:    "bar",
 				Provider: NewLegacyProvider("foo"),
 			},
@@ -228,7 +181,7 @@ func TestAbsProviderConfigString(t *testing.T) {
 		},
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance.Child("child_module", NoKey),
+				Module:   RootModule.Child("child_module"),
 				Alias:    "bar",
 				Provider: NewLegacyProvider("foo"),
 			},
@@ -251,21 +204,21 @@ func TestAbsProviderConfigLegacyString(t *testing.T) {
 	}{
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance,
+				Module:   RootModule,
 				Provider: NewLegacyProvider("foo"),
 			},
 			`provider.foo`,
 		},
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance.Child("child_module", NoKey),
+				Module:   RootModule.Child("child_module"),
 				Provider: NewLegacyProvider("foo"),
 			},
 			`module.child_module.provider.foo`,
 		},
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance,
+				Module:   RootModule,
 				Alias:    "bar",
 				Provider: NewLegacyProvider("foo"),
 			},
@@ -273,7 +226,7 @@ func TestAbsProviderConfigLegacyString(t *testing.T) {
 		},
 		{
 			AbsProviderConfig{
-				Module:   RootModuleInstance.Child("child_module", NoKey),
+				Module:   RootModule.Child("child_module"),
 				Alias:    "bar",
 				Provider: NewLegacyProvider("foo"),
 			},

configs/config.go

@@ -222,7 +222,7 @@ func (c *Config) gatherProviderTypes(m map[addrs.Provider]struct{}) {
 // The module address to resolve local addresses in must be given in the second
 // argument, and must refer to a module that exists under the receiver or
 // else this method will panic.
-func (c *Config) ResolveAbsProviderAddr(addr addrs.ProviderConfig, inModule addrs.ModuleInstance) addrs.AbsProviderConfig {
+func (c *Config) ResolveAbsProviderAddr(addr addrs.ProviderConfig, inModule addrs.Module) addrs.AbsProviderConfig {
 	switch addr := addr.(type) {
 
 	case addrs.AbsProviderConfig:
@@ -231,7 +231,7 @@ func (c *Config) ResolveAbsProviderAddr(addr addrs.ProviderConfig, inModule addr
 	case addrs.LocalProviderConfig:
 		// Find the descendent Config that contains the module that this
 		// local config belongs to.
-		mc := c.DescendentForInstance(inModule)
+		mc := c.Descendent(inModule)
 		if mc == nil {
 			panic(fmt.Sprintf("ResolveAbsProviderAddr with non-existent module %s", inModule.String()))
 		}
@@ -265,5 +265,5 @@ func (c *Config) ProviderForConfigAddr(addr addrs.LocalProviderConfig) addrs.Pro
 	if provider, exists := c.Module.ProviderRequirements[addr.LocalName]; exists {
 		return provider.Type
 	}
-	return c.ResolveAbsProviderAddr(addr, addrs.RootModuleInstance).Provider
+	return c.ResolveAbsProviderAddr(addr, addrs.RootModule).Provider
 }

configs/config_test.go

@@ -43,11 +43,11 @@ func TestConfigResolveAbsProviderAddr(t *testing.T) {
 
 	t.Run("already absolute", func(t *testing.T) {
 		addr := addrs.AbsProviderConfig{
-			Module:   addrs.RootModuleInstance,
+			Module:   addrs.RootModule,
 			Provider: addrs.NewLegacyProvider("test"),
 			Alias:    "boop",
 		}
-		got := cfg.ResolveAbsProviderAddr(addr, addrs.RootModuleInstance)
+		got := cfg.ResolveAbsProviderAddr(addr, addrs.RootModule)
 		if got, want := got.String(), addr.String(); got != want {
 			t.Errorf("wrong result\ngot:  %s\nwant: %s", got, want)
 		}
@@ -57,9 +57,9 @@ func TestConfigResolveAbsProviderAddr(t *testing.T) {
 			LocalName: "implied",
 			Alias:     "boop",
 		}
-		got := cfg.ResolveAbsProviderAddr(addr, addrs.RootModuleInstance)
+		got := cfg.ResolveAbsProviderAddr(addr, addrs.RootModule)
 		want := addrs.AbsProviderConfig{
-			Module: addrs.RootModuleInstance,
+			Module: addrs.RootModule,
 			// FIXME: At the time of writing we still have LocalProviderConfig
 			// nested inside AbsProviderConfig, but a future change will
 			// stop tis embedding and just have an addrs.Provider and an alias
@@ -78,9 +78,9 @@ func TestConfigResolveAbsProviderAddr(t *testing.T) {
 			LocalName: "foo-test", // this is explicitly set in the config
 			Alias:     "boop",
 		}
-		got := cfg.ResolveAbsProviderAddr(addr, addrs.RootModuleInstance)
+		got := cfg.ResolveAbsProviderAddr(addr, addrs.RootModule)
 		want := addrs.AbsProviderConfig{
-			Module:   addrs.RootModuleInstance,
+			Module:   addrs.RootModule,
 			Provider: addrs.NewProvider(addrs.DefaultRegistryHost, "foo", "test"),
 			Alias:    "boop",
 		}

states/statefile/version3_upgrade.go

@@ -136,13 +136,13 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 							return nil, fmt.Errorf("invalid legacy provider config reference %q for %s: %s", oldProviderAddr, instAddr, diags.Err())
 						}
 						providerAddr = addrs.AbsProviderConfig{
-							Module:   moduleAddr,
+							Module:   moduleAddr.Module(),
 							Provider: addrs.NewLegacyProvider(localAddr.LocalName),
 							Alias:    localAddr.Alias,
 						}
 					} else {
 						providerAddr = addrs.AbsProviderConfig{
-							Module:   moduleAddr,
+							Module:   moduleAddr.Module(),
 							Provider: resAddr.DefaultProvider(),
 						}
 					}