Merge pull request #25504 from hashicorp/alisdair/post-install-cache-validate

Add post-install provider cache validation and error reporting

command/init.go

@@ -653,6 +653,18 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State,
 					"https://www.terraform.io/docs/plugins/signing.html"))
 			}
 		},
+		HashPackageFailure: func(provider addrs.Provider, version getproviders.Version, err error) {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Failed to validate installed provider",
+				fmt.Sprintf(
+					"Validating provider %s v%s failed: %s",
+					provider.ForDisplay(),
+					version,
+					err,
+				),
+			))
+		},
 	}
 
 	mode := providercache.InstallNewProvidersOnly

command/init_test.go

@@ -951,6 +951,67 @@ func TestInit_getProviderSource(t *testing.T) {
 	}
 }
 
+func TestInit_getProviderInvalidPackage(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	copy.CopyDir(testFixturePath("init-get-provider-invalid-package"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	overrides := metaOverridesForProvider(testProvider())
+	ui := new(cli.MockUi)
+
+	// create a provider source which allows installing an invalid package
+	addr := addrs.MustParseProviderSourceString("invalid/package")
+	version := getproviders.MustParseVersion("1.0.0")
+	meta, close, err := getproviders.FakeInstallablePackageMeta(
+		addr,
+		version,
+		getproviders.VersionList{getproviders.MustParseVersion("5.0")},
+		getproviders.CurrentPlatform,
+		"terraform-package", // should be "terraform-provider-package"
+	)
+	defer close()
+	if err != nil {
+		t.Fatalf("failed to prepare fake package for %s %s: %s", addr.ForDisplay(), version, err)
+	}
+	providerSource := getproviders.NewMockSource([]getproviders.PackageMeta{meta}, nil)
+
+	m := Meta{
+		testingOverrides: overrides,
+		Ui:               ui,
+		ProviderSource:   providerSource,
+	}
+
+	c := &InitCommand{
+		Meta: m,
+	}
+
+	args := []string{
+		"-backend=false", // should be possible to install plugins without backend init
+	}
+	if code := c.Run(args); code != 1 {
+		t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, ui.ErrorWriter.String(), ui.OutputWriter.String())
+	}
+
+	// invalid provider should be installed
+	packagePath := fmt.Sprintf(".terraform/plugins/registry.terraform.io/invalid/package/1.0.0/%s/terraform-package", getproviders.CurrentPlatform)
+	if _, err := os.Stat(packagePath); os.IsNotExist(err) {
+		t.Fatal("provider 'invalid/package' not downloaded")
+	}
+
+	wantErrors := []string{
+		"Failed to validate installed provider",
+		"could not find executable file starting with terraform-provider-package",
+	}
+	got := ui.ErrorWriter.String()
+	for _, wantError := range wantErrors {
+		if !strings.Contains(got, wantError) {
+			t.Fatalf("missing error:\nwant: %q\n got: %q", wantError, got)
+		}
+	}
+}
+
 func TestInit_getProviderDetectedLegacy(t *testing.T) {
 	// Create a temporary working directory that is empty
 	td := tempDir(t)
@@ -1060,26 +1121,23 @@ func TestInit_providerSource(t *testing.T) {
 	wantPackages := map[addrs.Provider][]providercache.CachedProvider{
 		addrs.NewDefaultProvider("test"): {
 			{
-				Provider:       addrs.NewDefaultProvider("test"),
+				Provider:   addrs.NewDefaultProvider("test"),
-				Version:        getproviders.MustParseVersion("1.2.3"),
+				Version:    getproviders.MustParseVersion("1.2.3"),
-				PackageDir:     expectedPackageInstallPath("test", "1.2.3", false),
+				PackageDir: expectedPackageInstallPath("test", "1.2.3", false),
-				ExecutableFile: expectedPackageInstallPath("test", "1.2.3", true),
 			},
 		},
 		addrs.NewDefaultProvider("test-beta"): {
 			{
-				Provider:       addrs.NewDefaultProvider("test-beta"),
+				Provider:   addrs.NewDefaultProvider("test-beta"),
-				Version:        getproviders.MustParseVersion("1.2.4"),
+				Version:    getproviders.MustParseVersion("1.2.4"),
-				PackageDir:     expectedPackageInstallPath("test-beta", "1.2.4", false),
+				PackageDir: expectedPackageInstallPath("test-beta", "1.2.4", false),
-				ExecutableFile: expectedPackageInstallPath("test-beta", "1.2.4", true),
 			},
 		},
 		addrs.NewDefaultProvider("source"): {
 			{
-				Provider:       addrs.NewDefaultProvider("source"),
+				Provider:   addrs.NewDefaultProvider("source"),
-				Version:        getproviders.MustParseVersion("1.2.3"),
+				Version:    getproviders.MustParseVersion("1.2.3"),
-				PackageDir:     expectedPackageInstallPath("source", "1.2.3", false),
+				PackageDir: expectedPackageInstallPath("source", "1.2.3", false),
-				ExecutableFile: expectedPackageInstallPath("source", "1.2.3", true),
 			},
 		},
 	}
@@ -1094,22 +1152,19 @@ func TestInit_providerSource(t *testing.T) {
 	}
 	wantSelected := map[addrs.Provider]*providercache.CachedProvider{
 		addrs.NewDefaultProvider("test-beta"): {
-			Provider:       addrs.NewDefaultProvider("test-beta"),
+			Provider:   addrs.NewDefaultProvider("test-beta"),
-			Version:        getproviders.MustParseVersion("1.2.4"),
+			Version:    getproviders.MustParseVersion("1.2.4"),
-			PackageDir:     expectedPackageInstallPath("test-beta", "1.2.4", false),
+			PackageDir: expectedPackageInstallPath("test-beta", "1.2.4", false),
-			ExecutableFile: expectedPackageInstallPath("test-beta", "1.2.4", true),
 		},
 		addrs.NewDefaultProvider("test"): {
-			Provider:       addrs.NewDefaultProvider("test"),
+			Provider:   addrs.NewDefaultProvider("test"),
-			Version:        getproviders.MustParseVersion("1.2.3"),
+			Version:    getproviders.MustParseVersion("1.2.3"),
-			PackageDir:     expectedPackageInstallPath("test", "1.2.3", false),
+			PackageDir: expectedPackageInstallPath("test", "1.2.3", false),
-			ExecutableFile: expectedPackageInstallPath("test", "1.2.3", true),
 		},
 		addrs.NewDefaultProvider("source"): {
-			Provider:       addrs.NewDefaultProvider("source"),
+			Provider:   addrs.NewDefaultProvider("source"),
-			Version:        getproviders.MustParseVersion("1.2.3"),
+			Version:    getproviders.MustParseVersion("1.2.3"),
-			PackageDir:     expectedPackageInstallPath("source", "1.2.3", false),
+			PackageDir: expectedPackageInstallPath("source", "1.2.3", false),
-			ExecutableFile: expectedPackageInstallPath("source", "1.2.3", true),
 		},
 	}
 	if diff := cmp.Diff(wantSelected, gotSelected); diff != "" {
@@ -1169,27 +1224,24 @@ func TestInit_getUpgradePlugins(t *testing.T) {
 		// the newest available version that matched the version constraints.
 		addrs.NewDefaultProvider("between"): {
 			{
-				Provider:       addrs.NewDefaultProvider("between"),
+				Provider:   addrs.NewDefaultProvider("between"),
-				Version:        getproviders.MustParseVersion("2.3.4"),
+				Version:    getproviders.MustParseVersion("2.3.4"),
-				PackageDir:     expectedPackageInstallPath("between", "2.3.4", false),
+				PackageDir: expectedPackageInstallPath("between", "2.3.4", false),
-				ExecutableFile: expectedPackageInstallPath("between", "2.3.4", true),
 			},
 		},
 		// The existing version of "exact" did not match the version constraints,
 		// so we installed what the configuration selected as well.
 		addrs.NewDefaultProvider("exact"): {
 			{
-				Provider:       addrs.NewDefaultProvider("exact"),
+				Provider:   addrs.NewDefaultProvider("exact"),
-				Version:        getproviders.MustParseVersion("1.2.3"),
+				Version:    getproviders.MustParseVersion("1.2.3"),
-				PackageDir:     expectedPackageInstallPath("exact", "1.2.3", false),
+				PackageDir: expectedPackageInstallPath("exact", "1.2.3", false),
-				ExecutableFile: expectedPackageInstallPath("exact", "1.2.3", true),
 			},
 			// Previous version is still there, but not selected
 			{
-				Provider:       addrs.NewDefaultProvider("exact"),
+				Provider:   addrs.NewDefaultProvider("exact"),
-				Version:        getproviders.MustParseVersion("0.0.1"),
+				Version:    getproviders.MustParseVersion("0.0.1"),
-				PackageDir:     expectedPackageInstallPath("exact", "0.0.1", false),
+				PackageDir: expectedPackageInstallPath("exact", "0.0.1", false),
-				ExecutableFile: expectedPackageInstallPath("exact", "0.0.1", true),
 			},
 		},
 		// The existing version of "greater-than" _did_ match the constraints,
@@ -1197,17 +1249,15 @@ func TestInit_getUpgradePlugins(t *testing.T) {
 		// -upgrade and so we upgraded it anyway.
 		addrs.NewDefaultProvider("greater-than"): {
 			{
-				Provider:       addrs.NewDefaultProvider("greater-than"),
+				Provider:   addrs.NewDefaultProvider("greater-than"),
-				Version:        getproviders.MustParseVersion("2.3.4"),
+				Version:    getproviders.MustParseVersion("2.3.4"),
-				PackageDir:     expectedPackageInstallPath("greater-than", "2.3.4", false),
+				PackageDir: expectedPackageInstallPath("greater-than", "2.3.4", false),
-				ExecutableFile: expectedPackageInstallPath("greater-than", "2.3.4", true),
 			},
 			// Previous version is still there, but not selected
 			{
-				Provider:       addrs.NewDefaultProvider("greater-than"),
+				Provider:   addrs.NewDefaultProvider("greater-than"),
-				Version:        getproviders.MustParseVersion("2.3.3"),
+				Version:    getproviders.MustParseVersion("2.3.3"),
-				PackageDir:     expectedPackageInstallPath("greater-than", "2.3.3", false),
+				PackageDir: expectedPackageInstallPath("greater-than", "2.3.3", false),
-				ExecutableFile: expectedPackageInstallPath("greater-than", "2.3.3", true),
 			},
 		},
 	}
@@ -1222,22 +1272,19 @@ func TestInit_getUpgradePlugins(t *testing.T) {
 	}
 	wantSelected := map[addrs.Provider]*providercache.CachedProvider{
 		addrs.NewDefaultProvider("between"): {
-			Provider:       addrs.NewDefaultProvider("between"),
+			Provider:   addrs.NewDefaultProvider("between"),
-			Version:        getproviders.MustParseVersion("2.3.4"),
+			Version:    getproviders.MustParseVersion("2.3.4"),
-			PackageDir:     expectedPackageInstallPath("between", "2.3.4", false),
+			PackageDir: expectedPackageInstallPath("between", "2.3.4", false),
-			ExecutableFile: expectedPackageInstallPath("between", "2.3.4", true),
 		},
 		addrs.NewDefaultProvider("exact"): {
-			Provider:       addrs.NewDefaultProvider("exact"),
+			Provider:   addrs.NewDefaultProvider("exact"),
-			Version:        getproviders.MustParseVersion("1.2.3"),
+			Version:    getproviders.MustParseVersion("1.2.3"),
-			PackageDir:     expectedPackageInstallPath("exact", "1.2.3", false),
+			PackageDir: expectedPackageInstallPath("exact", "1.2.3", false),
-			ExecutableFile: expectedPackageInstallPath("exact", "1.2.3", true),
 		},
 		addrs.NewDefaultProvider("greater-than"): {
-			Provider:       addrs.NewDefaultProvider("greater-than"),
+			Provider:   addrs.NewDefaultProvider("greater-than"),
-			Version:        getproviders.MustParseVersion("2.3.4"),
+			Version:    getproviders.MustParseVersion("2.3.4"),
-			PackageDir:     expectedPackageInstallPath("greater-than", "2.3.4", false),
+			PackageDir: expectedPackageInstallPath("greater-than", "2.3.4", false),
-			ExecutableFile: expectedPackageInstallPath("greater-than", "2.3.4", true),
 		},
 	}
 	if diff := cmp.Diff(wantSelected, gotSelected); diff != "" {
@@ -1480,22 +1527,19 @@ func TestInit_pluginDirProviders(t *testing.T) {
 	}
 	wantSelected := map[addrs.Provider]*providercache.CachedProvider{
 		addrs.NewDefaultProvider("between"): {
-			Provider:       addrs.NewDefaultProvider("between"),
+			Provider:   addrs.NewDefaultProvider("between"),
-			Version:        getproviders.MustParseVersion("2.3.4"),
+			Version:    getproviders.MustParseVersion("2.3.4"),
-			PackageDir:     expectedPackageInstallPath("between", "2.3.4", false),
+			PackageDir: expectedPackageInstallPath("between", "2.3.4", false),
-			ExecutableFile: expectedPackageInstallPath("between", "2.3.4", true),
 		},
 		addrs.NewDefaultProvider("exact"): {
-			Provider:       addrs.NewDefaultProvider("exact"),
+			Provider:   addrs.NewDefaultProvider("exact"),
-			Version:        getproviders.MustParseVersion("1.2.3"),
+			Version:    getproviders.MustParseVersion("1.2.3"),
-			PackageDir:     expectedPackageInstallPath("exact", "1.2.3", false),
+			PackageDir: expectedPackageInstallPath("exact", "1.2.3", false),
-			ExecutableFile: expectedPackageInstallPath("exact", "1.2.3", true),
 		},
 		addrs.NewDefaultProvider("greater-than"): {
-			Provider:       addrs.NewDefaultProvider("greater-than"),
+			Provider:   addrs.NewDefaultProvider("greater-than"),
-			Version:        getproviders.MustParseVersion("2.3.4"),
+			Version:    getproviders.MustParseVersion("2.3.4"),
-			PackageDir:     expectedPackageInstallPath("greater-than", "2.3.4", false),
+			PackageDir: expectedPackageInstallPath("greater-than", "2.3.4", false),
-			ExecutableFile: expectedPackageInstallPath("greater-than", "2.3.4", true),
 		},
 	}
 	if diff := cmp.Diff(wantSelected, gotSelected); diff != "" {
@@ -1695,7 +1739,7 @@ func newMockProviderSource(t *testing.T, availableProviderVersions map[string][]
 				close()
 				t.Fatalf("failed to parse %q as a version number for %q: %s", versionStr, addr.ForDisplay(), err)
 			}
-			meta, close, err := getproviders.FakeInstallablePackageMeta(addr, version, getproviders.VersionList{getproviders.MustParseVersion("5.0")}, getproviders.CurrentPlatform)
+			meta, close, err := getproviders.FakeInstallablePackageMeta(addr, version, getproviders.VersionList{getproviders.MustParseVersion("5.0")}, getproviders.CurrentPlatform, "")
 			if err != nil {
 				close()
 				t.Fatalf("failed to prepare fake package for %s %s: %s", addr.ForDisplay(), versionStr, err)
@@ -1755,7 +1799,7 @@ func installFakeProviderPackagesElsewhere(t *testing.T, cacheDir *providercache.
 			if err != nil {
 				t.Fatalf("failed to parse %q as a version number for %q: %s", versionStr, name, err)
 			}
-			meta, close, err := getproviders.FakeInstallablePackageMeta(addr, version, getproviders.VersionList{getproviders.MustParseVersion("5.0")}, getproviders.CurrentPlatform)
+			meta, close, err := getproviders.FakeInstallablePackageMeta(addr, version, getproviders.VersionList{getproviders.MustParseVersion("5.0")}, getproviders.CurrentPlatform, "")
 			// We're going to install all these fake packages before we return,
 			// so we don't need to preserve them afterwards.
 			defer close()

command/meta_providers.go

@@ -209,12 +209,17 @@ func providerFactory(meta *providercache.CachedProvider) providers.Factory {
 			Output: os.Stderr,
 		})
 
+		execFile, err := meta.ExecutableFile()
+		if err != nil {
+			return nil, err
+		}
+
 		config := &plugin.ClientConfig{
 			HandshakeConfig:  tfplugin.Handshake,
 			Logger:           logger,
 			AllowedProtocols: []plugin.Protocol{plugin.ProtocolGRPC},
 			Managed:          true,
-			Cmd:              exec.Command(meta.ExecutableFile),
+			Cmd:              exec.Command(execFile),
 			AutoMTLS:         enableProviderAutoMTLS,
 			VersionedPlugins: tfplugin.VersionedPlugins,
 		}

command/testdata/init-get-provider-invalid-package/main.tf

@@ -0,0 +1,8 @@
+terraform {
+  required_providers {
+    package = {
+      source  = "invalid/package"
+      version = "1.0.0"
+    }
+  }
+}

internal/getproviders/mock_source.go

@@ -143,13 +143,15 @@ func FakePackageMeta(provider addrs.Provider, version Version, protocols Version
 // to a temporary archive file that could actually be installed in principle.
 //
 // Installing it will not produce a working provider though: just a fake file
-// posing as an executable.
+// posing as an executable. The filename for the executable defaults to the
+// standard terraform-provider-NAME_X.Y.Z format, but can be overridden with
+// the execFilename argument.
 //
 // It's the caller's responsibility to call the close callback returned
 // alongside the result in order to clean up the temporary file. The caller
 // should call the callback even if this function returns an error, because
 // some error conditions leave a partially-created file on disk.
-func FakeInstallablePackageMeta(provider addrs.Provider, version Version, protocols VersionList, target Platform) (PackageMeta, func(), error) {
+func FakeInstallablePackageMeta(provider addrs.Provider, version Version, protocols VersionList, target Platform, execFilename string) (PackageMeta, func(), error) {
 	f, err := ioutil.TempFile("", "terraform-getproviders-fake-package-")
 	if err != nil {
 		return PackageMeta{}, func() {}, err
@@ -162,10 +164,12 @@ func FakeInstallablePackageMeta(provider addrs.Provider, version Version, protoc
 		os.Remove(f.Name())
 	}
 
-	execFilename := fmt.Sprintf("terraform-provider-%s_%s", provider.Type, version.String())
+	if execFilename == "" {
-	if target.OS == "windows" {
+		execFilename = fmt.Sprintf("terraform-provider-%s_%s", provider.Type, version.String())
-		// For a little more (technically unnecessary) realism...
+		if target.OS == "windows" {
-		execFilename += ".exe"
+			// For a little more (technically unnecessary) realism...
+			execFilename += ".exe"
+		}
 	}
 
 	zw := zip.NewWriter(f)

internal/providercache/cached_provider.go

@@ -1,6 +1,11 @@
 package providercache
 
 import (
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/internal/getproviders"
 )
@@ -20,16 +25,6 @@ type CachedProvider struct {
 	// both slashes and backslashes as long as the separators are consistent
 	// within a particular path string.
 	PackageDir string
-
-	// ExecutableFile is the local filesystem path to the main plugin executable
-	// for the provider, which is always a file within the directory given
-	// in PackageDir.
-	//
-	// The path always uses slashes as path separators, even on Windows, so
-	// that the results are consistent between platforms. Windows accepts
-	// both slashes and backslashes as long as the separators are consistent
-	// within a particular path string.
-	ExecutableFile string
 }
 
 // PackageLocation returns the package directory given in the PackageDir field
@@ -77,3 +72,70 @@ func (cp *CachedProvider) MatchesHash(want string) (bool, error) {
 func (cp *CachedProvider) HashV1() (string, error) {
 	return getproviders.PackageHashV1(cp.PackageLocation())
 }
+
+// ExecutableFile inspects the cached provider's unpacked package directory for
+// something that looks like it's intended to be the executable file for the
+// plugin.
+//
+// This is a bit messy and heuristic-y because historically Terraform used the
+// filename itself for local filesystem discovery, allowing some variance in
+// the filenames to capture extra metadata, whereas now we're using the
+// directory structure leading to the executable instead but need to remain
+// compatible with the executable names bundled into existing provider packages.
+//
+// It will return an error if it can't find a file following the expected
+// convention in the given directory.
+//
+// If found, the path always uses slashes as path separators, even on Windows,
+// so that the results are consistent between platforms. Windows accepts both
+// slashes and backslashes as long as the separators are consistent within a
+// particular path string.
+func (cp *CachedProvider) ExecutableFile() (string, error) {
+	infos, err := ioutil.ReadDir(cp.PackageDir)
+	if err != nil {
+		// If the directory itself doesn't exist or isn't readable then we
+		// can't access an executable in it.
+		return "", fmt.Errorf("could not read package directory: %s", err)
+	}
+
+	// For a provider named e.g. tf.example.com/awesomecorp/happycloud, we
+	// expect an executable file whose name starts with
+	// "terraform-provider-happycloud", followed by zero or more additional
+	// characters. If there _are_ additional characters then the first one
+	// must be an underscore or a period, like in thse examples:
+	// - terraform-provider-happycloud_v1.0.0
+	// - terraform-provider-happycloud.exe
+	//
+	// We don't require the version in the filename to match because the
+	// executable's name is no longer authoritative, but packages of "official"
+	// providers may continue to use versioned executable names for backward
+	// compatibility with Terraform 0.12.
+	//
+	// We also presume that providers packaged for Windows will include the
+	// necessary .exe extension on their filenames but do not explicitly check
+	// for that. If there's a provider package for Windows that has a file
+	// without that suffix then it will be detected as an executable but then
+	// we'll presumably fail later trying to run it.
+	wantPrefix := "terraform-provider-" + cp.Provider.Type
+
+	// We'll visit all of the directory entries and take the first (in
+	// name-lexical order) that looks like a plausible provider executable
+	// name. A package with multiple files meeting these criteria is degenerate
+	// but we will tolerate it by ignoring the subsequent entries.
+	for _, info := range infos {
+		if info.IsDir() {
+			continue // A directory can never be an executable
+		}
+		name := info.Name()
+		if !strings.HasPrefix(name, wantPrefix) {
+			continue
+		}
+		remainder := name[len(wantPrefix):]
+		if len(remainder) > 0 && (remainder[0] != '_' && remainder[0] != '.') {
+			continue // subsequent characters must be delimited by _ or .
+		}
+		return filepath.ToSlash(filepath.Join(cp.PackageDir, name)), nil
+	}
+
+	return "", fmt.Errorf("could not find executable file starting with %s", wantPrefix)
+}

internal/providercache/cached_provider_test.go

@@ -15,8 +15,7 @@ func TestCachedProviderHash(t *testing.T) {
 		),
 		Version: getproviders.MustParseVersion("2.0.0"),
 
-		PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/darwin_amd64",
+		PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/darwin_amd64",
-		ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/darwin_amd64/terraform-provider-null",
 	}
 
 	want := "h1:qjsREM4DqEWECD43FcPqddZ9oxCG+IaMTxvWPciS05g="
@@ -46,8 +45,7 @@ func TestCachedProviderHash(t *testing.T) {
 		),
 		Version: getproviders.MustParseVersion("2.0.0"),
 
-		PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
+		PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
-		ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64/terraform-provider-null",
 	}
 	gotMatches, err = cp2.MatchesHash(want)
 	if err != nil {
@@ -58,3 +56,58 @@ func TestCachedProviderHash(t *testing.T) {
 	}
 
 }
+
+func TestExecutableFile(t *testing.T) {
+	testCases := map[string]struct {
+		cp   *CachedProvider
+		file string
+		err  string
+	}{
+		"linux": {
+			cp: &CachedProvider{
+				Provider:   addrs.NewProvider(addrs.DefaultRegistryHost, "hashicorp", "null"),
+				Version:    getproviders.MustParseVersion("2.0.0"),
+				PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/linux_amd64",
+			},
+			file: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/linux_amd64/terraform-provider-null",
+		},
+		"windows": {
+			cp: &CachedProvider{
+				Provider:   addrs.NewProvider(addrs.DefaultRegistryHost, "hashicorp", "null"),
+				Version:    getproviders.MustParseVersion("2.0.0"),
+				PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
+			},
+			file: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64/terraform-provider-null.exe",
+		},
+		"missing-executable": {
+			cp: &CachedProvider{
+				Provider:   addrs.NewProvider(addrs.DefaultRegistryHost, "missing", "executable"),
+				Version:    getproviders.MustParseVersion("2.0.0"),
+				PackageDir: "testdata/cachedir/registry.terraform.io/missing/executable/2.0.0/linux_amd64",
+			},
+			err: "could not find executable file starting with terraform-provider-executable",
+		},
+		"missing-dir": {
+			cp: &CachedProvider{
+				Provider:   addrs.NewProvider(addrs.DefaultRegistryHost, "missing", "packagedir"),
+				Version:    getproviders.MustParseVersion("2.0.0"),
+				PackageDir: "testdata/cachedir/registry.terraform.io/missing/packagedir/2.0.0/linux_amd64",
+			},
+			err: "could not read package directory: open testdata/cachedir/registry.terraform.io/missing/packagedir/2.0.0/linux_amd64: no such file or directory",
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			file, err := tc.cp.ExecutableFile()
+			if file != tc.file {
+				t.Errorf("wrong file\n got: %q\nwant: %q", file, tc.file)
+			}
+			if err == nil && tc.err != "" {
+				t.Fatalf("no error returned, want: %q", tc.err)
+			} else if err != nil && err.Error() != tc.err {
+				t.Errorf("wrong error\n got: %q\nwant: %q", err, tc.err)
+			}
+		})
+	}
+}

internal/providercache/dir.go

@@ -1,11 +1,9 @@
 package providercache
 
 import (
-	"io/ioutil"
 	"log"
 	"path/filepath"
 	"sort"
-	"strings"
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/internal/getproviders"
@@ -165,20 +163,12 @@ func (d *Dir) fillMetaCache() error {
 			}
 
 			packageDir := filepath.Clean(string(meta.Location.(getproviders.PackageLocalDir)))
-			execFile := findProviderExecutableInLocalPackage(meta)
-			if execFile == "" {
-				// If the package doesn't contain a suitable executable then
-				// it isn't considered to be part of our cache.
-				log.Printf("[TRACE] providercache.fillMetaCache: ignoring %s because it is does not seem to contain a suitable plugin executable", meta.Location)
-				continue
-			}
 
 			log.Printf("[TRACE] providercache.fillMetaCache: including %s as a candidate package for %s %s", meta.Location, providerAddr, meta.Version)
 			data[providerAddr] = append(data[providerAddr], CachedProvider{
-				Provider:       providerAddr,
+				Provider:   providerAddr,
-				Version:        meta.Version,
+				Version:    meta.Version,
-				PackageDir:     filepath.ToSlash(packageDir),
+				PackageDir: filepath.ToSlash(packageDir),
-				ExecutableFile: filepath.ToSlash(execFile),
 			})
 		}
 	}
@@ -200,75 +190,3 @@ func (d *Dir) fillMetaCache() error {
 	d.metaCache = data
 	return nil
 }
-
-// This is a helper function to peep into the unpacked directory associated
-// with the given package meta and find something that looks like it's intended
-// to be the executable file for the plugin.
-//
-// This is a bit messy and heuristic-y because historically Terraform used the
-// filename itself for local filesystem discovery, allowing some variance in
-// the filenames to capture extra metadata, whereas now we're using the
-// directory structure leading to the executable instead but need to remain
-// compatible with the executable names bundled into existing provider packages.
-//
-// It will return a zero-length string if it can't find a file following
-// the expected convention in the given directory.
-func findProviderExecutableInLocalPackage(meta getproviders.PackageMeta) string {
-	packageDir, ok := meta.Location.(getproviders.PackageLocalDir)
-	if !ok {
-		// This should never happen because the providercache package only
-		// uses the local unpacked directory layout. If anything else ends
-		// up in here then we'll indicate that no executable is available,
-		// because all other locations require a fetch/unpack step first.
-		return ""
-	}
-
-	infos, err := ioutil.ReadDir(string(packageDir))
-	if err != nil {
-		// If the directory itself doesn't exist or isn't readable then we
-		// can't access an executable in it.
-		return ""
-	}
-
-	// For a provider named e.g. tf.example.com/awesomecorp/happycloud, we
-	// expect an executable file whose name starts with
-	// "terraform-provider-happycloud", followed by zero or more additional
-	// characters. If there _are_ additional characters then the first one
-	// must be an underscore or a period, like in thse examples:
-	// - terraform-provider-happycloud_v1.0.0
-	// - terraform-provider-happycloud.exe
-	//
-	// We don't require the version in the filename to match because the
-	// executable's name is no longer authoritative, but packages of "official"
-	// providers may continue to use versioned executable names for backward
-	// compatibility with Terraform 0.12.
-	//
-	// We also presume that providers packaged for Windows will include the
-	// necessary .exe extension on their filenames but do not explicitly check
-	// for that. If there's a provider package for Windows that has a file
-	// without that suffix then it will be detected as an executable but then
-	// we'll presumably fail later trying to run it.
-	wantPrefix := "terraform-provider-" + meta.Provider.Type
-
-	// We'll visit all of the directory entries and take the first (in
-	// name-lexical order) that looks like a plausible provider executable
-	// name. A package with multiple files meeting these criteria is degenerate
-	// but we will tolerate it by ignoring the subsequent entries.
-	for _, info := range infos {
-		if info.IsDir() {
-			continue // A directory can never be an executable
-		}
-		name := info.Name()
-		if !strings.HasPrefix(name, wantPrefix) {
-			continue
-		}
-		remainder := name[len(wantPrefix):]
-		if len(remainder) > 0 && (remainder[0] != '_' && remainder[0] != '.') {
-			continue // subsequent characters must be delimited by _
-		}
-		return filepath.Join(string(packageDir), name)
-	}
-
-	// If we fall out here then nothing has matched.
-	return ""
-}

internal/providercache/dir_modify_test.go

@@ -58,8 +58,7 @@ func TestInstallPackage(t *testing.T) {
 
 				Version: versions.MustParseVersion("2.1.0"),
 
-				PackageDir:     tmpDirPath + "/registry.terraform.io/hashicorp/null/2.1.0/linux_amd64",
+				PackageDir: tmpDirPath + "/registry.terraform.io/hashicorp/null/2.1.0/linux_amd64",
-				ExecutableFile: tmpDirPath + "/registry.terraform.io/hashicorp/null/2.1.0/linux_amd64/terraform-provider-null",
 			},
 		},
 	}
@@ -101,8 +100,7 @@ func TestLinkFromOtherCache(t *testing.T) {
 				// still packed and thus not considered to be a cache member.
 				Version: versions.MustParseVersion("2.0.0"),
 
-				PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
+				PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
-				ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64/terraform-provider-null.exe",
 			},
 		},
 	}
@@ -138,8 +136,7 @@ func TestLinkFromOtherCache(t *testing.T) {
 				// still packed and thus not considered to be a cache member.
 				Version: versions.MustParseVersion("2.0.0"),
 
-				PackageDir:     tmpDirPath + "/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
+				PackageDir: tmpDirPath + "/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
-				ExecutableFile: tmpDirPath + "/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64/terraform-provider-null.exe",
 			},
 		},
 	}

internal/providercache/dir_test.go

@@ -37,6 +37,9 @@ func TestDirReading(t *testing.T) {
 		addrs.DefaultRegistryHost, "bloop", "nonexist",
 	)
 	legacyProvider := addrs.NewLegacyProvider("legacy")
+	missingExecutableProvider := addrs.NewProvider(
+		addrs.DefaultRegistryHost, "missing", "executable",
+	)
 
 	t.Run("ProviderLatestVersion", func(t *testing.T) {
 		t.Run("exists", func(t *testing.T) {
@@ -50,8 +53,7 @@ func TestDirReading(t *testing.T) {
 				// still packed and thus not considered to be a cache member.
 				Version: versions.MustParseVersion("2.0.0"),
 
-				PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
+				PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
-				ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64/terraform-provider-null.exe",
 			}
 
 			if diff := cmp.Diff(want, got); diff != "" {
@@ -91,8 +93,7 @@ func TestDirReading(t *testing.T) {
 				Provider: nullProvider,
 				Version:  versions.MustParseVersion("2.0.0"),
 
-				PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
+				PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64",
-				ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/windows_amd64/terraform-provider-null.exe",
 			}
 
 			if diff := cmp.Diff(want, got); diff != "" {
@@ -141,34 +142,37 @@ func TestDirReading(t *testing.T) {
 		want := map[addrs.Provider][]CachedProvider{
 			legacyProvider: {
 				{
-					Provider:       legacyProvider,
+					Provider:   legacyProvider,
-					Version:        versions.MustParseVersion("1.0.0"),
+					Version:    versions.MustParseVersion("1.0.0"),
-					PackageDir:     "testdata/cachedir/registry.terraform.io/-/legacy/1.0.0/linux_amd64",
+					PackageDir: "testdata/cachedir/registry.terraform.io/-/legacy/1.0.0/linux_amd64",
-					ExecutableFile: "testdata/cachedir/registry.terraform.io/-/legacy/1.0.0/linux_amd64/terraform-provider-legacy",
 				},
 			},
 			nullProvider: {
 				{
-					Provider:       nullProvider,
+					Provider:   nullProvider,
-					Version:        versions.MustParseVersion("2.0.0"),
+					Version:    versions.MustParseVersion("2.0.0"),
-					PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/linux_amd64",
+					PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/linux_amd64",
-					ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/null/2.0.0/linux_amd64/terraform-provider-null",
 				},
 			},
 			randomProvider: {
 				{
-					Provider:       randomProvider,
+					Provider:   randomProvider,
-					Version:        versions.MustParseVersion("1.2.0"),
+					Version:    versions.MustParseVersion("1.2.0"),
-					PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/random/1.2.0/linux_amd64",
+					PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/random/1.2.0/linux_amd64",
-					ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/random/1.2.0/linux_amd64/terraform-provider-random",
 				},
 			},
 			randomBetaProvider: {
 				{
-					Provider:       randomBetaProvider,
+					Provider:   randomBetaProvider,
-					Version:        versions.MustParseVersion("1.2.0"),
+					Version:    versions.MustParseVersion("1.2.0"),
-					PackageDir:     "testdata/cachedir/registry.terraform.io/hashicorp/random-beta/1.2.0/linux_amd64",
+					PackageDir: "testdata/cachedir/registry.terraform.io/hashicorp/random-beta/1.2.0/linux_amd64",
-					ExecutableFile: "testdata/cachedir/registry.terraform.io/hashicorp/random-beta/1.2.0/linux_amd64/terraform-provider-random-beta",
+				},
+			},
+			missingExecutableProvider: {
+				{
+					Provider:   missingExecutableProvider,
+					Version:    versions.MustParseVersion("2.0.0"),
+					PackageDir: "testdata/cachedir/registry.terraform.io/missing/executable/2.0.0/linux_amd64",
 				},
 			},
 		}

internal/providercache/installer.go

@@ -400,6 +400,14 @@ NeedProvider:
 			}
 			continue
 		}
+		if _, err := cached.ExecutableFile(); err != nil {
+			err := fmt.Errorf("provider binary not found: %s", err)
+			errs[provider] = err
+			if cb := evts.HashPackageFailure; cb != nil {
+				cb(provider, version, err)
+			}
+			continue
+		}
 		hash, err := cached.Hash()
 		if err != nil {
 			errs[provider] = fmt.Errorf("failed to calculate checksum for installed provider %s package: %s", provider, err)

internal/providercache/installer_test.go

@@ -11,12 +11,102 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	svchost "github.com/hashicorp/terraform-svchost"
 	"github.com/hashicorp/terraform-svchost/disco"
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/internal/getproviders"
 )
 
+func TestEnsureProviderVersions_local_source(t *testing.T) {
+	// create filesystem source using the test provider cache dir
+	source := getproviders.NewFilesystemMirrorSource("testdata/cachedir")
+
+	// create a temporary workdir
+	tmpDirPath, err := ioutil.TempDir("", "terraform-test-providercache")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDirPath)
+
+	// set up the installer using the temporary directory and filesystem source
+	platform := getproviders.Platform{OS: "linux", Arch: "amd64"}
+	dir := NewDirWithPlatform(tmpDirPath, platform)
+	installer := NewInstaller(dir, source)
+
+	tests := map[string]struct {
+		provider  string
+		version   string
+		installed bool
+		err       string
+	}{
+		"install-unpacked": {
+			provider:  "null",
+			version:   "2.0.0",
+			installed: true,
+		},
+		"invalid-zip-file": {
+			provider:  "null",
+			version:   "2.1.0",
+			installed: false,
+			err:       "zip: not a valid zip file",
+		},
+		"version-constraint-unmet": {
+			provider:  "null",
+			version:   "2.2.0",
+			installed: false,
+			err:       "no available releases match the given constraints 2.2.0",
+		},
+		"missing-executable": {
+			provider:  "missing/executable",
+			version:   "2.0.0",
+			installed: true,
+			err:       "provider binary not found: could not find executable file starting with terraform-provider-executable",
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			ctx := context.TODO()
+
+			provider := addrs.MustParseProviderSourceString(test.provider)
+			versionConstraint := getproviders.MustParseVersionConstraints(test.version)
+			version := getproviders.MustParseVersion(test.version)
+			reqs := getproviders.Requirements{
+				provider: versionConstraint,
+			}
+			wantSelected := getproviders.Selections{provider: version}
+			if !test.installed {
+				wantSelected = getproviders.Selections{}
+			}
+
+			selected, err := installer.EnsureProviderVersions(ctx, reqs, InstallNewProvidersOnly)
+
+			if diff := cmp.Diff(wantSelected, selected); diff != "" {
+				t.Errorf("wrong selected\n%s", diff)
+			}
+
+			if test.err == "" && err == nil {
+				return
+			}
+
+			switch err := err.(type) {
+			case InstallerError:
+				providerError, ok := err.ProviderErrors[provider]
+				if !ok {
+					t.Fatalf("did not get error for provider %s", provider)
+				}
+
+				if got := providerError.Error(); got != test.err {
+					t.Fatalf("wrong result\ngot:  %s\nwant: %s\n", got, test.err)
+				}
+			default:
+				t.Fatalf("wrong error type. Expected InstallerError, got %T", err)
+			}
+		})
+	}
+}
+
 // This test only verifies protocol errors and does not try for successfull
 // installation (at the time of writing, the test files aren't signed so the
 // signature verification fails); that's left to the e2e tests.

internal/providercache/testdata/cachedir/registry.terraform.io/missing/executable/2.0.0/linux_amd64/executable

@@ -0,0 +1 @@
+This file represents a misnamed provider executable.