From f35ebe2d658a808f997b773b04b4efc73fdfe5ef Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 1 Apr 2020 14:59:55 -0700 Subject: [PATCH] internal/providercache: Fix incorrect logic in Installer.SetGlobalCacheDir Due to some incomplete rework of this function in an earlier commit, the safety check for using the same directory as both the target and the cache was inverted and was raising an error _unless_ they matched, rather than _if_ they matched. This change is verified by the e2etest TestInitProviders_pluginCache, which is also updated to use the new-style cache directory layout as part of this commit. --- command/e2etest/init_test.go | 17 ++++++++++------- .../terraform-provider-template_v2.1.0_x4 | 0 internal/providercache/installer.go | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) rename command/e2etest/testdata/plugin-cache/cache/{ => registry.terraform.io/hashicorp/template/2.1.0}/os_arch/terraform-provider-template_v2.1.0_x4 (100%) diff --git a/command/e2etest/init_test.go b/command/e2etest/init_test.go index 1f3775bcb2..cc47d3165f 100644 --- a/command/e2etest/init_test.go +++ b/command/e2etest/init_test.go @@ -96,16 +96,19 @@ func TestInitProviders_pluginCache(t *testing.T) { // Our fixture dir has a generic os_arch dir, which we need to customize // to the actual OS/arch where this test is running in order to get the // desired result. - fixtMachineDir := tf.Path("cache/os_arch") - wantMachineDir := tf.Path("cache", fmt.Sprintf("%s_%s", runtime.GOOS, runtime.GOARCH)) - os.Rename(fixtMachineDir, wantMachineDir) + fixtMachineDir := tf.Path("cache/registry.terraform.io/hashicorp/template/2.1.0/os_arch") + wantMachineDir := tf.Path("cache/registry.terraform.io/hashicorp/template/2.1.0/", fmt.Sprintf("%s_%s", runtime.GOOS, runtime.GOARCH)) + err := os.Rename(fixtMachineDir, wantMachineDir) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } cmd := tf.Cmd("init") cmd.Env = append(cmd.Env, "TF_PLUGIN_CACHE_DIR=./cache") cmd.Stdin = nil cmd.Stderr = &bytes.Buffer{} - err := cmd.Run() + err = cmd.Run() if err != nil { t.Errorf("unexpected error: %s", err) } @@ -115,7 +118,7 @@ func TestInitProviders_pluginCache(t *testing.T) { t.Errorf("unexpected stderr output:\n%s\n", stderr) } - path := fmt.Sprintf(".terraform/plugins/%s_%s/terraform-provider-template_v2.1.0_x4", runtime.GOOS, runtime.GOARCH) + path := fmt.Sprintf(".terraform/plugins/registry.terraform.io/hashicorp/template/2.1.0/%s_%s/terraform-provider-template_v2.1.0_x4", runtime.GOOS, runtime.GOARCH) content, err := tf.ReadFile(path) if err != nil { t.Fatalf("failed to read installed plugin from %s: %s", path, err) @@ -124,11 +127,11 @@ func TestInitProviders_pluginCache(t *testing.T) { t.Errorf("template plugin was not installed from local cache") } - if !tf.FileExists(fmt.Sprintf(".terraform/plugins/%s_%s/terraform-provider-null_v2.1.0_x4", runtime.GOOS, runtime.GOARCH)) { + if !tf.FileExists(fmt.Sprintf(".terraform/plugins/registry.terraform.io/hashicorp/null/2.1.0/%s_%s/terraform-provider-null_v2.1.0_x4", runtime.GOOS, runtime.GOARCH)) { t.Errorf("null plugin was not installed") } - if !tf.FileExists(fmt.Sprintf("cache/%s_%s/terraform-provider-null_v2.1.0_x4", runtime.GOOS, runtime.GOARCH)) { + if !tf.FileExists(fmt.Sprintf("cache/registry.terraform.io/hashicorp/null/2.1.0/%s_%s/terraform-provider-null_v2.1.0_x4", runtime.GOOS, runtime.GOARCH)) { t.Errorf("null plugin is not in cache after install") } } diff --git a/command/e2etest/testdata/plugin-cache/cache/os_arch/terraform-provider-template_v2.1.0_x4 b/command/e2etest/testdata/plugin-cache/cache/registry.terraform.io/hashicorp/template/2.1.0/os_arch/terraform-provider-template_v2.1.0_x4 similarity index 100% rename from command/e2etest/testdata/plugin-cache/cache/os_arch/terraform-provider-template_v2.1.0_x4 rename to command/e2etest/testdata/plugin-cache/cache/registry.terraform.io/hashicorp/template/2.1.0/os_arch/terraform-provider-template_v2.1.0_x4 diff --git a/internal/providercache/installer.go b/internal/providercache/installer.go index 82e5beccdd..ae5fbb3526 100644 --- a/internal/providercache/installer.go +++ b/internal/providercache/installer.go @@ -64,8 +64,8 @@ func (i *Installer) SetGlobalCacheDir(cacheDir *Dir) { // A little safety check to catch straightforward mistakes where the // directories overlap. Better to panic early than to do // possibly-distructive actions on the cache directory downstream. - if same, err := copydir.SameFile(i.targetDir.baseDir, cacheDir.baseDir); err == nil && !same { - panic(fmt.Sprintf("global cache directory %s must not match the installation target directory", i.targetDir.baseDir)) + if same, err := copydir.SameFile(i.targetDir.baseDir, cacheDir.baseDir); err == nil && same { + panic(fmt.Sprintf("global cache directory %s must not match the installation target directory %s", cacheDir.baseDir, i.targetDir.baseDir)) } i.globalCacheDir = cacheDir }