provider source enhancements

* configs: move ProviderConfigCompact[Str] from addrs to configs

The configs package is aware of provider name and type (which are the
same thing today, but expected to be two different things in a future
release), and should be the source of truth for a provider config
address. This is an intermediate step; the next step will change the returned types to something based in the configs package.

* command: rename choosePlugins to chooseProviders to clarify scope of function

* use `Provider.LegacyString()` (instead of `Provider.Type`) consistently
* explicitly create legacy-style provider (continuing from above change)

addrs/module_instance.go

@@ -57,7 +57,7 @@ func ParseModuleInstance(traversal hcl.Traversal) (ModuleInstance, tfdiags.Diagn
 // If a reference string is coming from a source that should be identified in
 // error messages then the caller should instead parse it directly using a
 // suitable function from the HCL API and pass the traversal itself to
-// ParseProviderConfigCompact.
+// ParseModuleInstance.
 //
 // Error diagnostics are returned if either the parsing fails or the analysis
 // of the traversal fails. There is no way for the caller to distinguish the

addrs/provider_config.go

@@ -26,84 +26,6 @@ func NewDefaultProviderConfig(typeName string) ProviderConfig {
 	}
 }
 
-// ParseProviderConfigCompact parses the given absolute traversal as a relative
-// provider address in compact form. The following are examples of traversals
-// that can be successfully parsed as compact relative provider configuration
-// addresses:
-//
-//     aws
-//     aws.foo
-//
-// This function will panic if given a relative traversal.
-//
-// If the returned diagnostics contains errors then the result value is invalid
-// and must not be used.
-func ParseProviderConfigCompact(traversal hcl.Traversal) (ProviderConfig, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
-	ret := ProviderConfig{
-		Type: NewLegacyProvider(traversal.RootName()),
-	}
-
-	if len(traversal) < 2 {
-		// Just a type name, then.
-		return ret, diags
-	}
-
-	aliasStep := traversal[1]
-	switch ts := aliasStep.(type) {
-	case hcl.TraverseAttr:
-		ret.Alias = ts.Name
-		return ret, diags
-	default:
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Invalid provider configuration address",
-			Detail:   "The provider type name must either stand alone or be followed by an alias name separated with a dot.",
-			Subject:  aliasStep.SourceRange().Ptr(),
-		})
-	}
-
-	if len(traversal) > 2 {
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Invalid provider configuration address",
-			Detail:   "Extraneous extra operators after provider configuration address.",
-			Subject:  traversal[2:].SourceRange().Ptr(),
-		})
-	}
-
-	return ret, diags
-}
-
-// ParseProviderConfigCompactStr is a helper wrapper around ParseProviderConfigCompact
-// that takes a string and parses it with the HCL native syntax traversal parser
-// before interpreting it.
-//
-// This should be used only in specialized situations since it will cause the
-// created references to not have any meaningful source location information.
-// If a reference string is coming from a source that should be identified in
-// error messages then the caller should instead parse it directly using a
-// suitable function from the HCL API and pass the traversal itself to
-// ParseProviderConfigCompact.
-//
-// Error diagnostics are returned if either the parsing fails or the analysis
-// of the traversal fails. There is no way for the caller to distinguish the
-// two kinds of diagnostics programmatically. If error diagnostics are returned
-// then the returned address is invalid.
-func ParseProviderConfigCompactStr(str string) (ProviderConfig, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
-
-	traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1})
-	diags = diags.Append(parseDiags)
-	if parseDiags.HasErrors() {
-		return ProviderConfig{}, diags
-	}
-
-	addr, addrDiags := ParseProviderConfigCompact(traversal)
-	diags = diags.Append(addrDiags)
-	return addr, diags
-}
-
 // Absolute returns an AbsProviderConfig from the receiver and the given module
 // instance address.
 func (pc ProviderConfig) Absolute(module ModuleInstance) AbsProviderConfig {

addrs/provider_config_test.go

@@ -9,68 +9,6 @@ import (
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 )
 
-func TestParseProviderConfigCompact(t *testing.T) {
-	tests := []struct {
-		Input    string
-		Want     ProviderConfig
-		WantDiag string
-	}{
-		{
-			`aws`,
-			ProviderConfig{
-				Type: NewLegacyProvider("aws"),
-			},
-			``,
-		},
-		{
-			`aws.foo`,
-			ProviderConfig{
-				Type:  NewLegacyProvider("aws"),
-				Alias: "foo",
-			},
-			``,
-		},
-		{
-			`aws["foo"]`,
-			ProviderConfig{},
-			`The provider type name must either stand alone or be followed by an alias name separated with a dot.`,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.Input, func(t *testing.T) {
-			traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(test.Input), "", hcl.Pos{})
-			if len(parseDiags) != 0 {
-				t.Errorf("unexpected diagnostics during parse")
-				for _, diag := range parseDiags {
-					t.Logf("- %s", diag)
-				}
-				return
-			}
-
-			got, diags := ParseProviderConfigCompact(traversal)
-
-			if test.WantDiag != "" {
-				if len(diags) != 1 {
-					t.Fatalf("got %d diagnostics; want 1", len(diags))
-				}
-				gotDetail := diags[0].Description().Detail
-				if gotDetail != test.WantDiag {
-					t.Fatalf("wrong diagnostic detail\ngot:  %s\nwant: %s", gotDetail, test.WantDiag)
-				}
-				return
-			} else {
-				if len(diags) != 0 {
-					t.Fatalf("got %d diagnostics; want 0", len(diags))
-				}
-			}
-
-			for _, problem := range deep.Equal(got, test.Want) {
-				t.Error(problem)
-			}
-		})
-	}
-}
 func TestParseAbsProviderConfig(t *testing.T) {
 	tests := []struct {
 		Input    string

command/import.go

@@ -166,7 +166,7 @@ func (c *ImportCommand) Run(args []string) int {
 			c.Ui.Info(importCommandInvalidAddressReference)
 			return 1
 		}
-		relAddr, addrDiags := addrs.ParseProviderConfigCompact(traversal)
+		relAddr, addrDiags := configs.ParseProviderConfigCompact(traversal)
 		diags = diags.Append(addrDiags)
 		if addrDiags.HasErrors() {
 			c.showDiagnostics(diags)

command/import_test.go

@@ -891,17 +891,18 @@ func TestImport_pluginDir(t *testing.T) {
 
 	// Now we need to go through some plugin init.
 	// This discovers our fake plugin and writes the lock file.
+	initUi := new(cli.MockUi)
 	initCmd := &InitCommand{
 		Meta: Meta{
 			pluginPath: []string{"./plugins"},
-			Ui:         cli.NewMockUi(),
+			Ui:         initUi,
 		},
 		providerInstaller: &discovery.ProviderInstaller{
 			PluginProtocolVersion: discovery.PluginInstallProtocolVersion,
 		},
 	}
 	if code := initCmd.Run(nil); code != 0 {
-		t.Fatal(initCmd.Meta.Ui.(*cli.MockUi).ErrorWriter.String())
+		t.Fatal(initUi.ErrorWriter.String())
 	}
 
 	args := []string{

command/init.go

@@ -496,7 +496,7 @@ func (c *InitCommand) getProviders(earlyConfig *earlyconfig.Config, state *state
 	configReqs := configDeps.AllPluginRequirements()
 	// FIXME: This is weird because ConfigTreeDependencies was written before
 	// we switched over to using earlyConfig as the main source of dependencies.
-	// In future we should clean this up to be a more reasoable API.
+	// In future we should clean this up to be a more reasonable API.
 	stateReqs := terraform.ConfigTreeDependencies(nil, state).AllPluginRequirements()
 
 	requirements := configReqs.Merge(stateReqs)
@@ -517,7 +517,7 @@ func (c *InitCommand) getProviders(earlyConfig *earlyconfig.Config, state *state
 		}
 
 		for provider, reqd := range missing {
-			pty := addrs.Provider{Type: provider}
+			pty := addrs.NewLegacyProvider(provider)
 			_, providerDiags, err := c.providerInstaller.Get(pty, reqd.Versions)
 			diags = diags.Append(providerDiags)
 
@@ -597,7 +597,7 @@ func (c *InitCommand) getProviders(earlyConfig *earlyconfig.Config, state *state
 	available = c.providerPluginSet() // re-discover to see newly-installed plugins
 
 	// internal providers were already filtered out, since we don't need to get them.
-	chosen := choosePlugins(available, nil, requirements)
+	chosen := chooseProviders(available, nil, requirements)
 
 	digests := map[string][]byte{}
 	for name, meta := range chosen {

command/plugins.go

@@ -39,7 +39,7 @@ type multiVersionProviderResolver struct {
 	Internal map[addrs.Provider]providers.Factory
 }
 
-func choosePlugins(avail discovery.PluginMetaSet, internal map[addrs.Provider]providers.Factory, reqd discovery.PluginRequirements) map[string]discovery.PluginMeta {
+func chooseProviders(avail discovery.PluginMetaSet, internal map[addrs.Provider]providers.Factory, reqd discovery.PluginRequirements) map[string]discovery.PluginMeta {
 	candidates := avail.ConstrainVersions(reqd)
 	ret := map[string]discovery.PluginMeta{}
 	for name, metas := range candidates {
@@ -63,7 +63,7 @@ func (r *multiVersionProviderResolver) ResolveProviders(
 	factories := make(map[addrs.Provider]providers.Factory, len(reqd))
 	var errs []error
 
-	chosen := choosePlugins(r.Available, r.Internal, reqd)
+	chosen := chooseProviders(r.Available, r.Internal, reqd)
 	for name, req := range reqd {
 		if factory, isInternal := r.Internal[addrs.NewLegacyProvider(name)]; isInternal {
 			if !req.Versions.Unconstrained() {

configs/provider.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/tfdiags"
 )
 
 // Provider represents a "provider" block in a module or file. A provider
@@ -107,28 +108,82 @@ func (p *Provider) moduleUniqueKey() string {
 	return p.Name
 }
 
-// ProviderRequirement represents a declaration of a dependency on a particular
+// ParseProviderConfigCompact parses the given absolute traversal as a relative
-// provider version without actually configuring that provider. This is used in
+// provider address in compact form. The following are examples of traversals
-// child modules that expect a provider to be passed in from their parent.
+// that can be successfully parsed as compact relative provider configuration
-type ProviderRequirement struct {
+// addresses:
-	Name        string
+//
-	Requirement VersionConstraint
+//     aws
+//     aws.foo
+//
+// This function will panic if given a relative traversal.
+//
+// If the returned diagnostics contains errors then the result value is invalid
+// and must not be used.
+func ParseProviderConfigCompact(traversal hcl.Traversal) (addrs.ProviderConfig, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	ret := addrs.ProviderConfig{
+		Type: addrs.NewLegacyProvider(traversal.RootName()),
+	}
+
+	if len(traversal) < 2 {
+		// Just a type name, then.
+		return ret, diags
+	}
+
+	aliasStep := traversal[1]
+	switch ts := aliasStep.(type) {
+	case hcl.TraverseAttr:
+		ret.Alias = ts.Name
+		return ret, diags
+	default:
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid provider configuration address",
+			Detail:   "The provider type name must either stand alone or be followed by an alias name separated with a dot.",
+			Subject:  aliasStep.SourceRange().Ptr(),
+		})
+	}
+
+	if len(traversal) > 2 {
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid provider configuration address",
+			Detail:   "Extraneous extra operators after provider configuration address.",
+			Subject:  traversal[2:].SourceRange().Ptr(),
+		})
+	}
+
+	return ret, diags
 }
 
-func decodeRequiredProvidersBlock(block *hcl.Block) ([]*ProviderRequirement, hcl.Diagnostics) {
+// ParseProviderConfigCompactStr is a helper wrapper around ParseProviderConfigCompact
-	attrs, diags := block.Body.JustAttributes()
+// that takes a string and parses it with the HCL native syntax traversal parser
-	var reqs []*ProviderRequirement
+// before interpreting it.
-	for name, attr := range attrs {
+//
-		req, reqDiags := decodeVersionConstraint(attr)
+// This should be used only in specialized situations since it will cause the
-		diags = append(diags, reqDiags...)
+// created references to not have any meaningful source location information.
-		if !diags.HasErrors() {
+// If a reference string is coming from a source that should be identified in
-			reqs = append(reqs, &ProviderRequirement{
+// error messages then the caller should instead parse it directly using a
-				Name:        name,
+// suitable function from the HCL API and pass the traversal itself to
-				Requirement: req,
+// ParseProviderConfigCompact.
-			})
+//
-		}
+// Error diagnostics are returned if either the parsing fails or the analysis
+// of the traversal fails. There is no way for the caller to distinguish the
+// two kinds of diagnostics programmatically. If error diagnostics are returned
+// then the returned address is invalid.
+func ParseProviderConfigCompactStr(str string) (addrs.ProviderConfig, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+
+	traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1})
+	diags = diags.Append(parseDiags)
+	if parseDiags.HasErrors() {
+		return addrs.ProviderConfig{}, diags
 	}
-	return reqs, diags
+
+	addr, addrDiags := ParseProviderConfigCompact(traversal)
+	diags = diags.Append(addrDiags)
+	return addr, diags
 }
 
 var providerBlockSchema = &hcl.BodySchema{

configs/provider_requirements.go

@@ -0,0 +1,32 @@
+package configs
+
+import (
+	"github.com/hashicorp/hcl/v2"
+)
+
+// ProviderRequirement represents a declaration of a dependency on a particular
+// provider version without actually configuring that provider. This is used in
+// child modules that expect a provider to be passed in from their parent.
+//
+// TODO: "Source" is a placeholder for an attribute that is not yet supported.
+type ProviderRequirement struct {
+	Name        string
+	Source      string // TODO
+	Requirement VersionConstraint
+}
+
+func decodeRequiredProvidersBlock(block *hcl.Block) ([]*ProviderRequirement, hcl.Diagnostics) {
+	attrs, diags := block.Body.JustAttributes()
+	var reqs []*ProviderRequirement
+	for name, attr := range attrs {
+		req, reqDiags := decodeVersionConstraint(attr)
+		diags = append(diags, reqDiags...)
+		if !diags.HasErrors() {
+			reqs = append(reqs, &ProviderRequirement{
+				Name:        name,
+				Requirement: req,
+			})
+		}
+	}
+	return reqs, diags
+}

configs/provider_test.go

@@ -3,6 +3,11 @@ package configs
 import (
 	"io/ioutil"
 	"testing"
+
+	"github.com/go-test/deep"
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/hashicorp/terraform/addrs"
 )
 
 func TestProviderReservedNames(t *testing.T) {
@@ -24,3 +29,66 @@ func TestProviderReservedNames(t *testing.T) {
 		`config.tf:13,3-9: Reserved argument name in provider block; The provider argument name "source" is reserved for use by Terraform in a future version.`,
 	})
 }
+
+func TestParseProviderConfigCompact(t *testing.T) {
+	tests := []struct {
+		Input    string
+		Want     addrs.ProviderConfig
+		WantDiag string
+	}{
+		{
+			`aws`,
+			addrs.ProviderConfig{
+				Type: addrs.NewLegacyProvider("aws"),
+			},
+			``,
+		},
+		{
+			`aws.foo`,
+			addrs.ProviderConfig{
+				Type:  addrs.NewLegacyProvider("aws"),
+				Alias: "foo",
+			},
+			``,
+		},
+		{
+			`aws["foo"]`,
+			addrs.ProviderConfig{},
+			`The provider type name must either stand alone or be followed by an alias name separated with a dot.`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.Input, func(t *testing.T) {
+			traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(test.Input), "", hcl.Pos{})
+			if len(parseDiags) != 0 {
+				t.Errorf("unexpected diagnostics during parse")
+				for _, diag := range parseDiags {
+					t.Logf("- %s", diag)
+				}
+				return
+			}
+
+			got, diags := ParseProviderConfigCompact(traversal)
+
+			if test.WantDiag != "" {
+				if len(diags) != 1 {
+					t.Fatalf("got %d diagnostics; want 1", len(diags))
+				}
+				gotDetail := diags[0].Description().Detail
+				if gotDetail != test.WantDiag {
+					t.Fatalf("wrong diagnostic detail\ngot:  %s\nwant: %s", gotDetail, test.WantDiag)
+				}
+				return
+			} else {
+				if len(diags) != 0 {
+					t.Fatalf("got %d diagnostics; want 0", len(diags))
+				}
+			}
+
+			for _, problem := range deep.Equal(got, test.Want) {
+				t.Error(problem)
+			}
+		})
+	}
+}

plugin/discovery/get.go

@@ -232,7 +232,7 @@ func (i *ProviderInstaller) Get(provider addrs.Provider, req Constraints) (Plugi
 		}
 	}
 
-	printedProviderName := fmt.Sprintf("%q (%s)", provider.Type, providerSource)
+	printedProviderName := fmt.Sprintf("%q (%s)", provider.LegacyString(), providerSource)
 	i.Ui.Info(fmt.Sprintf("- Downloading plugin for provider %s %s...", printedProviderName, versionMeta.Version))
 	log.Printf("[DEBUG] getting provider %s version %q", printedProviderName, versionMeta.Version)
 	err = i.install(provider, v, providerURL)
@@ -244,7 +244,7 @@ func (i *ProviderInstaller) Get(provider addrs.Provider, req Constraints) (Plugi
 	// (This is weird, because go-getter doesn't directly return
 	//  information about what was extracted, and we just extracted
 	//  the archive directly into a shared dir here.)
-	log.Printf("[DEBUG] looking for the %s %s plugin we just installed", provider.Type, versionMeta.Version)
+	log.Printf("[DEBUG] looking for the %s %s plugin we just installed", provider.LegacyString(), versionMeta.Version)
 	metas := FindPlugins("provider", []string{i.Dir})
 	log.Printf("[DEBUG] all plugins found %#v", metas)
 	metas, _ = metas.ValidateVersions()
@@ -278,10 +278,10 @@ func (i *ProviderInstaller) Get(provider addrs.Provider, req Constraints) (Plugi
 
 func (i *ProviderInstaller) install(provider addrs.Provider, version Version, url string) error {
 	if i.Cache != nil {
-		log.Printf("[DEBUG] looking for provider %s %s in plugin cache", provider.Type, version)
+		log.Printf("[DEBUG] looking for provider %s %s in plugin cache", provider.LegacyString(), version)
 		cached := i.Cache.CachedPluginPath("provider", provider.Type, version)
 		if cached == "" {
-			log.Printf("[DEBUG] %s %s not yet in cache, so downloading %s", provider.Type, version, url)
+			log.Printf("[DEBUG] %s %s not yet in cache, so downloading %s", provider.LegacyString(), version, url)
 			err := getter.Get(i.Cache.InstallDir(), url)
 			if err != nil {
 				return err
@@ -308,7 +308,7 @@ func (i *ProviderInstaller) install(provider addrs.Provider, version Version, ur
 			return err
 		}
 
-		log.Printf("[DEBUG] installing %s %s to %s from local cache %s", provider.Type, version, targetPath, cached)
+		log.Printf("[DEBUG] installing %s %s to %s from local cache %s", provider.LegacyString(), version, targetPath, cached)
 
 		// Delete if we can. If there's nothing there already then no harm done.
 		// This is important because we can't create a link if there's
@@ -366,7 +366,7 @@ func (i *ProviderInstaller) install(provider addrs.Provider, version Version, ur
 		// One way or another, by the time we get here we should have either
 		// a link or a copy of the cached plugin within i.Dir, as expected.
 	} else {
-		log.Printf("[DEBUG] plugin cache is disabled, so downloading %s %s from %s", provider.Type, version, url)
+		log.Printf("[DEBUG] plugin cache is disabled, so downloading %s %s from %s", provider.LegacyString(), version, url)
 		err := getter.Get(i.Dir, url)
 		if err != nil {
 			return err

plugin/discovery/get_test.go

@@ -419,7 +419,7 @@ func TestProviderInstallerGet(t *testing.T) {
 		registry:              registry.NewClient(Disco(server), nil),
 	}
 
-	_, _, err = i.Get(addrs.Provider{Type: "test"}, AllVersions)
+	_, _, err = i.Get(addrs.NewLegacyProvider("test"), AllVersions)
 
 	if err != ErrorNoVersionCompatibleWithPlatform {
 		t.Fatal("want error for incompatible version")
@@ -436,21 +436,21 @@ func TestProviderInstallerGet(t *testing.T) {
 	}
 
 	{
-		_, _, err := i.Get(addrs.Provider{Type: "test"}, ConstraintStr(">9.0.0").MustParse())
+		_, _, err := i.Get(addrs.NewLegacyProvider("test"), ConstraintStr(">9.0.0").MustParse())
 		if err != ErrorNoSuitableVersion {
 			t.Fatal("want error for mismatching constraints")
 		}
 	}
 
 	{
-		provider := addrs.Provider{Type: "nonexist"}
+		provider := addrs.NewLegacyProvider("nonexist")
 		_, _, err := i.Get(provider, AllVersions)
 		if err != ErrorNoSuchProvider {
 			t.Fatal("want error for no such provider")
 		}
 	}
 
-	gotMeta, _, err := i.Get(addrs.Provider{Type: "test"}, AllVersions)
+	gotMeta, _, err := i.Get(addrs.NewLegacyProvider("test"), AllVersions)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -508,7 +508,7 @@ func TestProviderInstallerGet_cache(t *testing.T) {
 		Arch:                  "mockarch",
 	}
 
-	gotMeta, _, err := i.Get(addrs.Provider{Type: "test"}, AllVersions)
+	gotMeta, _, err := i.Get(addrs.NewLegacyProvider("test"), AllVersions)
 	if err != nil {
 		t.Fatal(err)
 	}

states/statefile/version3_upgrade.go

@@ -12,6 +12,7 @@ import (
 	ctyjson "github.com/zclconf/go-cty/cty/json"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/states"
 	"github.com/hashicorp/terraform/tfdiags"
 )
@@ -123,7 +124,7 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 					// incorrect but it'll get fixed up next time any updates
 					// are made to an instance.
 					if oldProviderAddr != "" {
-						localAddr, diags := addrs.ParseProviderConfigCompactStr(oldProviderAddr)
+						localAddr, diags := configs.ParseProviderConfigCompactStr(oldProviderAddr)
 						if diags.HasErrors() {
 							if strings.Contains(oldProviderAddr, "${") {
 								// There seems to be a common misconception that