Merge pull request #15313 from hashicorp/jbardin/discovery-paths

remove OS_ARCH knowledge from discovery

command/command.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"runtime"
 
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/cli"
@@ -19,10 +20,13 @@ const DefaultDataDir = ".terraform"
 // of directories supplied by the user with the `-plugin-dir` flag during init.
 const PluginPathFile = "plugin_path"
 
+// pluginMachineName is the directory name used in new plugin paths.
+const pluginMachineName = runtime.GOOS + "_" + runtime.GOARCH
+
 // DefaultPluginVendorDir is the location in the config directory to look for
 // user-added plugin binaries. Terraform only reads from this path if it
 // exists, it is never created by terraform.
-const DefaultPluginVendorDir = "terraform.d/plugins"
+const DefaultPluginVendorDir = "terraform.d/plugins/" + pluginMachineName
 
 // DefaultStateFilename is the default filename used for the state file.
 const DefaultStateFilename = "terraform.tfstate"

command/init_test.go

@@ -519,11 +519,7 @@ func TestInit_findVendoredProviders(t *testing.T) {
 	if err := os.MkdirAll(c.pluginDir(), 0755); err != nil {
 		t.Fatal(err)
 	}
-	vendorMachineDir := filepath.Join(
+	if err := os.MkdirAll(DefaultPluginVendorDir, 0755); err != nil {
-		DefaultPluginVendorDir,
-		fmt.Sprintf("%s_%s", runtime.GOOS, runtime.GOARCH),
-	)
-	if err := os.MkdirAll(vendorMachineDir, 0755); err != nil {
 		t.Fatal(err)
 	}
 
@@ -534,7 +530,7 @@ func TestInit_findVendoredProviders(t *testing.T) {
 		t.Fatal(err)
 	}
 	// the vendor path
-	greaterThanPath := filepath.Join(vendorMachineDir, "terraform-provider-greater_than_v2.3.4_x4")
+	greaterThanPath := filepath.Join(DefaultPluginVendorDir, "terraform-provider-greater_than_v2.3.4_x4")
 	if err := ioutil.WriteFile(greaterThanPath, []byte("test bin"), 0755); err != nil {
 		t.Fatal(err)
 	}

plugin/discovery/find.go

@@ -4,12 +4,9 @@ import (
 	"io/ioutil"
 	"log"
 	"path/filepath"
-	"runtime"
 	"strings"
 )
 
-const machineName = runtime.GOOS + "_" + runtime.GOARCH
-
 // FindPlugins looks in the given directories for files whose filenames
 // suggest that they are plugins of the given kind (e.g. "provider") and
 // returns a PluginMetaSet representing the discovered potential-plugins.
@@ -41,75 +38,56 @@ func FindPluginPaths(kind string, dirs []string) []string {
 	// This is just a thin wrapper around findPluginPaths so that we can
 	// use the latter in tests with a fake machineName so we can use our
 	// test fixtures.
-	return findPluginPaths(kind, machineName, dirs)
+	return findPluginPaths(kind, dirs)
 }
 
-func findPluginPaths(kind string, machineName string, dirs []string) []string {
+func findPluginPaths(kind string, dirs []string) []string {
 	prefix := "terraform-" + kind + "-"
 
 	ret := make([]string, 0, len(dirs))
 
-	for _, baseDir := range dirs {
+	for _, dir := range dirs {
-		baseItems, err := ioutil.ReadDir(baseDir)
+		items, err := ioutil.ReadDir(dir)
 		if err != nil {
 			// Ignore missing dirs, non-dirs, etc
 			continue
 		}
 
-		log.Printf("[DEBUG] checking for plugins in %q", baseDir)
+		log.Printf("[DEBUG] checking for plugins in %q", dir)
 
-		for _, item := range baseItems {
+		for _, item := range items {
-			fullName := item.Name()
-
-			if fullName == machineName && item.Mode().IsDir() {
-				// Current-style $GOOS-$GOARCH directory prefix
-				machineDir := filepath.Join(baseDir, machineName)
-				machineItems, err := ioutil.ReadDir(machineDir)
-				if err != nil {
-					continue
-				}
-
-				log.Printf("[DEBUG] checking for plugins in %q", machineDir)
-
-				for _, item := range machineItems {
 			fullName := item.Name()
 
 			if !strings.HasPrefix(fullName, prefix) {
+				log.Printf("[DEBUG] skipping %q, not a plugin", fullName)
 				continue
 			}
 
 			// New-style paths must have a version segment in filename
-					if !strings.Contains(strings.ToLower(fullName), "_v") {
+			if strings.Contains(strings.ToLower(fullName), "_v") {
-						continue
+				absPath, err := filepath.Abs(filepath.Join(dir, fullName))
-					}
-
-					absPath, err := filepath.Abs(filepath.Join(machineDir, fullName))
 				if err != nil {
+					log.Printf("[ERROR] plugin filepath error: %s", err)
 					continue
 				}
 
 				log.Printf("[DEBUG] found plugin %q", fullName)
-
 				ret = append(ret, filepath.Clean(absPath))
-				}
-
 				continue
 			}
 
-			// FIXME: we pass in GOOS_GOARCH paths directly, so these may not be "legacy"
-			if strings.HasPrefix(fullName, prefix) {
 			// Legacy style with files directly in the base directory
-				absPath, err := filepath.Abs(filepath.Join(baseDir, fullName))
+			absPath, err := filepath.Abs(filepath.Join(dir, fullName))
 			if err != nil {
+				log.Printf("[ERROR] plugin filepath error: %s", err)
 				continue
 			}
 
-				log.Printf("[DEBUG] found legacy plugin %q", fullName)
+			log.Printf("[WARNING] found legacy plugin %q", fullName)
 
 			ret = append(ret, filepath.Clean(absPath))
 		}
 	}
-	}
 
 	return ret
 }

plugin/discovery/find_test.go

@@ -10,9 +10,8 @@ import (
 func TestFindPluginPaths(t *testing.T) {
 	got := findPluginPaths(
 		"foo",
-		"mockos_mockarch",
 		[]string{
-			"test-fixtures/current-style-plugins",
+			"test-fixtures/current-style-plugins/mockos_mockarch",
 			"test-fixtures/legacy-style-plugins",
 			"test-fixtures/non-existent",
 			"test-fixtures/not-a-dir",
@@ -21,6 +20,8 @@ func TestFindPluginPaths(t *testing.T) {
 	want := []string{
 		filepath.Join("test-fixtures", "current-style-plugins", "mockos_mockarch", "terraform-foo-bar_v0.0.1"),
 		filepath.Join("test-fixtures", "current-style-plugins", "mockos_mockarch", "terraform-foo-bar_v1.0.0"),
+		// un-versioned plugins are still picked up, even in current-style paths
+		filepath.Join("test-fixtures", "current-style-plugins", "mockos_mockarch", "terraform-foo-missing-version"),
 		filepath.Join("test-fixtures", "legacy-style-plugins", "terraform-foo-bar"),
 		filepath.Join("test-fixtures", "legacy-style-plugins", "terraform-foo-baz"),
 	}