From ba5b0dc6098de5b0a29f1892ac74e779771478ea Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 16 Jun 2017 13:58:40 -0400 Subject: [PATCH 1/3] mostly remove OS_ARCH knowledge from discovery Since the command package also needs to know about the specific OS_ARCH directories, remove the logic fom the discovery package. This doesn't completely remove the knowledge of the path from discovery, in order to maintain the current behavior of skipping legacy plugin names within a new-style path. --- plugin/discovery/find.go | 76 +++++++++++++++-------------------- plugin/discovery/find_test.go | 7 +++- 2 files changed, 38 insertions(+), 45 deletions(-) diff --git a/plugin/discovery/find.go b/plugin/discovery/find.go index 539b9358c..e42bb49f6 100644 --- a/plugin/discovery/find.go +++ b/plugin/discovery/find.go @@ -4,11 +4,14 @@ import ( "io/ioutil" "log" "path/filepath" + "regexp" "runtime" "strings" ) -const machineName = runtime.GOOS + "_" + runtime.GOARCH +// Store the machine name for excluding legacy plugins in new-style directories. +// This is a var to override in testing +var 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 @@ -41,66 +44,53 @@ 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 { + hasMachineSuffix := regexp.MustCompile(machineName + "/?").MatchString + prefix := "terraform-" + kind + "-" ret := make([]string, 0, len(dirs)) - for _, baseDir := range dirs { - baseItems, err := ioutil.ReadDir(baseDir) + for _, dir := range dirs { + 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 { + isMachineDir := hasMachineSuffix(dir) + + 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) { - continue - } - - // New-style paths must have a version segment in filename - if !strings.Contains(strings.ToLower(fullName), "_v") { - continue - } - - absPath, err := filepath.Abs(filepath.Join(machineDir, fullName)) - if err != nil { - continue - } - - log.Printf("[DEBUG] found plugin %q", fullName) - - ret = append(ret, filepath.Clean(absPath)) - } - + if !strings.HasPrefix(fullName, prefix) { + log.Printf("[DEBUG] skipping %q, not a plugin", fullName) 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)) + // New-style paths must have a version segment in filename + if strings.Contains(strings.ToLower(fullName), "_v") { + absPath, err := filepath.Abs(filepath.Join(dir, 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 + } + + if !isMachineDir { + // Legacy style with files directly in the base directory + absPath, err := filepath.Abs(filepath.Join(dir, fullName)) + if err != nil { + log.Printf("[ERROR] plugin filepath error: %s", err) continue } diff --git a/plugin/discovery/find_test.go b/plugin/discovery/find_test.go index a788882bb..58e2946d1 100644 --- a/plugin/discovery/find_test.go +++ b/plugin/discovery/find_test.go @@ -7,12 +7,15 @@ import ( "testing" ) +func init() { + machineName = "mockos_mockarch" +} + 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", From 270eedd4b8f6f4a66b277924fe4c69825499db07 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 16 Jun 2017 14:09:47 -0400 Subject: [PATCH 2/3] always pass in the full plugin path to dicovery Discovery no longer tries to walk into OS_ARCH dirs, so always pass in the full search path. --- command/command.go | 6 +++++- command/init_test.go | 8 ++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/command/command.go b/command/command.go index bb8c7be5c..0fbb44df3 100644 --- a/command/command.go +++ b/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" diff --git a/command/init_test.go b/command/init_test.go index dd296d945..bf1d0dfca 100644 --- a/command/init_test.go +++ b/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( - DefaultPluginVendorDir, - fmt.Sprintf("%s_%s", runtime.GOOS, runtime.GOARCH), - ) - if err := os.MkdirAll(vendorMachineDir, 0755); err != nil { + if err := os.MkdirAll(DefaultPluginVendorDir, 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) } From 6faace287db4bcd74c3a3b52e4bf4f8668f08c01 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 16 Jun 2017 15:28:48 -0400 Subject: [PATCH 3/3] remove restriction on unversioned plugins Discover unversioned plugins regarless of location. --- plugin/discovery/find.go | 30 +++++++++--------------------- plugin/discovery/find_test.go | 6 ++---- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/plugin/discovery/find.go b/plugin/discovery/find.go index e42bb49f6..71db22a4f 100644 --- a/plugin/discovery/find.go +++ b/plugin/discovery/find.go @@ -4,15 +4,9 @@ import ( "io/ioutil" "log" "path/filepath" - "regexp" - "runtime" "strings" ) -// Store the machine name for excluding legacy plugins in new-style directories. -// This is a var to override in testing -var 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. @@ -48,8 +42,6 @@ func FindPluginPaths(kind string, dirs []string) []string { } func findPluginPaths(kind string, dirs []string) []string { - hasMachineSuffix := regexp.MustCompile(machineName + "/?").MatchString - prefix := "terraform-" + kind + "-" ret := make([]string, 0, len(dirs)) @@ -63,8 +55,6 @@ func findPluginPaths(kind string, dirs []string) []string { log.Printf("[DEBUG] checking for plugins in %q", dir) - isMachineDir := hasMachineSuffix(dir) - for _, item := range items { fullName := item.Name() @@ -86,18 +76,16 @@ func findPluginPaths(kind string, dirs []string) []string { continue } - if !isMachineDir { - // Legacy style with files directly in the base directory - 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) - - ret = append(ret, filepath.Clean(absPath)) + // Legacy style with files directly in the base directory + absPath, err := filepath.Abs(filepath.Join(dir, fullName)) + if err != nil { + log.Printf("[ERROR] plugin filepath error: %s", err) + continue } + + log.Printf("[WARNING] found legacy plugin %q", fullName) + + ret = append(ret, filepath.Clean(absPath)) } } diff --git a/plugin/discovery/find_test.go b/plugin/discovery/find_test.go index 58e2946d1..7115e2872 100644 --- a/plugin/discovery/find_test.go +++ b/plugin/discovery/find_test.go @@ -7,10 +7,6 @@ import ( "testing" ) -func init() { - machineName = "mockos_mockarch" -} - func TestFindPluginPaths(t *testing.T) { got := findPluginPaths( "foo", @@ -24,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"), }