internal/initwd: fix panics with relative submodules in DirFromModule (#25250)
* internal/initwd: fix panics with relative submodules in DirFromModule
There were two related issues here:
1. panic with any local module with submodules
1. panic with a relative directory that was above the workdir ("../")
The first panic was caused by the local installer looking up the root
module with the (nonexistant) key "root.", instead of "".
The second panic was caused by the installer trying to determine the
relative path from ".". This was fixed by detecting "." as the source
path and using the absolute path for the call to filepath.Rel.
Added test cases for both panics and updated the existing e2e tests with
the correct install paths.
This commit is contained in:
Kristin Laemmert
GitHub
parent
1a43187e49
commit
1b8f4566fa
|
|
@ -254,21 +254,52 @@ func DirFromModule(rootDir, modulesDir, sourceAddr string, reg *registry.Client,
|
|||
var parentKey string
|
||||
if lastDot := strings.LastIndexByte(newKey, '.'); lastDot != -1 {
|
||||
parentKey = newKey[:lastDot]
|
||||
} else {
|
||||
parentKey = "" // parent is the root module
|
||||
}
|
||||
|
||||
parentOld := instManifest[initFromModuleRootKeyPrefix+parentKey]
|
||||
var parentOld modsdir.Record
|
||||
// "" is the root module; all other modules get `root.` added as a prefix
|
||||
if parentKey == "" {
|
||||
parentOld = instManifest[parentKey]
|
||||
} else {
|
||||
parentOld = instManifest[initFromModuleRootKeyPrefix+parentKey]
|
||||
}
|
||||
parentNew := retManifest[parentKey]
|
||||
|
||||
// We need to figure out which portion of our directory is the
|
||||
// parent package path and which portion is the subdirectory
|
||||
// under that.
|
||||
baseDirRel, err := filepath.Rel(parentOld.Dir, record.Dir)
|
||||
var baseDirRel string
|
||||
baseDirRel, err = filepath.Rel(parentOld.Dir, record.Dir)
|
||||
if err != nil {
|
||||
// Should never happen, because we constructed both directories
|
||||
// from the same base and so they must have a common prefix.
|
||||
panic(err)
|
||||
// This error may occur when installing a local module with a
|
||||
// relative path, for e.g. if the source is in a directory above
|
||||
// the destination ("../")
|
||||
if parentOld.Dir == "." {
|
||||
absDir, err := filepath.Abs(parentOld.Dir)
|
||||
if err != nil {
|
||||
diags = diags.Append(tfdiags.Sourceless(
|
||||
tfdiags.Error,
|
||||
"Failed to determine module install directory",
|
||||
fmt.Sprintf("Error determine relative source directory for module %s: %s.", newKey, err),
|
||||
))
|
||||
continue
|
||||
}
|
||||
baseDirRel, err = filepath.Rel(absDir, record.Dir)
|
||||
if err != nil {
|
||||
diags = diags.Append(tfdiags.Sourceless(
|
||||
tfdiags.Error,
|
||||
"Failed to determine relative module source location",
|
||||
fmt.Sprintf("Error determining relative source for module %s: %s.", newKey, err),
|
||||
))
|
||||
continue
|
||||
}
|
||||
} else {
|
||||
diags = diags.Append(tfdiags.Sourceless(
|
||||
tfdiags.Error,
|
||||
"Failed to determine relative module source location",
|
||||
fmt.Sprintf("Error determining relative source for module %s: %s.", newKey, err),
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
newDir := filepath.Join(parentNew.Dir, baseDirRel)
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
package initwd
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
|
@ -9,6 +11,7 @@ import (
|
|||
version "github.com/hashicorp/go-version"
|
||||
"github.com/hashicorp/terraform/configs"
|
||||
"github.com/hashicorp/terraform/configs/configload"
|
||||
"github.com/hashicorp/terraform/internal/copydir"
|
||||
"github.com/hashicorp/terraform/registry"
|
||||
"github.com/hashicorp/terraform/tfdiags"
|
||||
)
|
||||
|
|
@ -20,6 +23,7 @@ func TestDirFromModule_registry(t *testing.T) {
|
|||
|
||||
fixtureDir := filepath.Clean("testdata/empty")
|
||||
tmpDir, done := tempChdir(t, fixtureDir)
|
||||
defer done()
|
||||
|
||||
// the module installer runs filepath.EvalSymlinks() on the destination
|
||||
// directory before copying files, and the resultant directory is what is
|
||||
|
|
@ -30,7 +34,6 @@ func TestDirFromModule_registry(t *testing.T) {
|
|||
t.Error(err)
|
||||
}
|
||||
modsDir := filepath.Join(dir, ".terraform/modules")
|
||||
defer done()
|
||||
|
||||
hooks := &testInstallHooks{}
|
||||
|
||||
|
|
@ -38,7 +41,7 @@ func TestDirFromModule_registry(t *testing.T) {
|
|||
diags := DirFromModule(dir, modsDir, "hashicorp/module-installer-acctest/aws//examples/main", reg, hooks)
|
||||
assertNoDiagnostics(t, diags)
|
||||
|
||||
v := version.Must(version.NewVersion("0.0.1"))
|
||||
v := version.Must(version.NewVersion("0.0.2"))
|
||||
|
||||
wantCalls := []testInstallHookCall{
|
||||
// The module specified to populate the root directory is not mentioned
|
||||
|
|
@ -61,17 +64,17 @@ func TestDirFromModule_registry(t *testing.T) {
|
|||
Name: "Install",
|
||||
ModuleAddr: "root",
|
||||
Version: v,
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff"),
|
||||
LocalPath: filepath.Join(dir, fmt.Sprintf(".terraform/modules/root/terraform-aws-module-installer-acctest-%s", v)),
|
||||
},
|
||||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "root.child_a",
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_a"),
|
||||
LocalPath: filepath.Join(dir, fmt.Sprintf(".terraform/modules/root/terraform-aws-module-installer-acctest-%s/modules/child_a", v)),
|
||||
},
|
||||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "root.child_a.child_b",
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/root/hashicorp-terraform-aws-module-installer-acctest-5e87aff/modules/child_b"),
|
||||
LocalPath: filepath.Join(dir, fmt.Sprintf(".terraform/modules/root/terraform-aws-module-installer-acctest-%s/modules/child_b", v)),
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -111,3 +114,165 @@ func TestDirFromModule_registry(t *testing.T) {
|
|||
})
|
||||
assertResultDeepEqual(t, gotTraces, wantTraces)
|
||||
}
|
||||
|
||||
func TestDirFromModule_submodules(t *testing.T) {
|
||||
fixtureDir := filepath.Clean("testdata/empty")
|
||||
fromModuleDir, err := filepath.Abs("./testdata/local-modules")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tmpDir, done := tempChdir(t, fixtureDir)
|
||||
defer done()
|
||||
|
||||
hooks := &testInstallHooks{}
|
||||
dir, err := filepath.EvalSymlinks(tmpDir)
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
modInstallDir := filepath.Join(dir, ".terraform/modules")
|
||||
|
||||
diags := DirFromModule(dir, modInstallDir, fromModuleDir, nil, hooks)
|
||||
assertNoDiagnostics(t, diags)
|
||||
wantCalls := []testInstallHookCall{
|
||||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "child_a",
|
||||
LocalPath: filepath.Join(fromModuleDir, "child_a"),
|
||||
},
|
||||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "child_a.child_b",
|
||||
LocalPath: filepath.Join(fromModuleDir, "child_a/child_b"),
|
||||
},
|
||||
}
|
||||
|
||||
if assertResultDeepEqual(t, hooks.Calls, wantCalls) {
|
||||
return
|
||||
}
|
||||
|
||||
loader, err := configload.NewLoader(&configload.Config{
|
||||
ModulesDir: modInstallDir,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Make sure the configuration is loadable now.
|
||||
// (This ensures that correct information is recorded in the manifest.)
|
||||
config, loadDiags := loader.LoadConfig(".")
|
||||
if assertNoDiagnostics(t, tfdiags.Diagnostics{}.Append(loadDiags)) {
|
||||
return
|
||||
}
|
||||
wantTraces := map[string]string{
|
||||
"": "in root module",
|
||||
"child_a": "in child_a module",
|
||||
"child_a.child_b": "in child_b module",
|
||||
}
|
||||
gotTraces := map[string]string{}
|
||||
|
||||
config.DeepEach(func(c *configs.Config) {
|
||||
path := strings.Join(c.Path, ".")
|
||||
if c.Module.Variables["v"] == nil {
|
||||
gotTraces[path] = "<missing>"
|
||||
return
|
||||
}
|
||||
varDesc := c.Module.Variables["v"].Description
|
||||
gotTraces[path] = varDesc
|
||||
})
|
||||
assertResultDeepEqual(t, gotTraces, wantTraces)
|
||||
}
|
||||
|
||||
// TestDirFromModule_rel_submodules is similar to the test above, but the
|
||||
// from-module is relative to the install dir ("../"):
|
||||
// https://github.com/hashicorp/terraform/issues/23010
|
||||
func TestDirFromModule_rel_submodules(t *testing.T) {
|
||||
// This test creates a tmpdir with the following directory structure:
|
||||
// - tmpdir/local-modules (with contents of testdata/local-modules)
|
||||
// - tmpdir/empty: the workDir we CD into for the test
|
||||
// - tmpdir/empty/target (target, the destination for init -from-module)
|
||||
tmpDir, err := ioutil.TempDir("", "terraform-configload")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fromModuleDir := filepath.Join(tmpDir, "local-modules")
|
||||
workDir := filepath.Join(tmpDir, "empty")
|
||||
if err := os.Mkdir(fromModuleDir, os.ModePerm); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := copydir.CopyDir(fromModuleDir, "testdata/local-modules"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Mkdir(workDir, os.ModePerm); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
targetDir := filepath.Join(tmpDir, "target")
|
||||
if err := os.Mkdir(targetDir, os.ModePerm); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
oldDir, err := os.Getwd()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
err = os.Chdir(targetDir)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to switch to temp dir %s: %s", tmpDir, err)
|
||||
}
|
||||
defer os.Chdir(oldDir)
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
hooks := &testInstallHooks{}
|
||||
|
||||
modInstallDir := ".terraform/modules"
|
||||
sourceDir := "../local-modules"
|
||||
diags := DirFromModule(".", modInstallDir, sourceDir, nil, hooks)
|
||||
assertNoDiagnostics(t, diags)
|
||||
wantCalls := []testInstallHookCall{
|
||||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "child_a",
|
||||
LocalPath: filepath.Join(sourceDir, "child_a"),
|
||||
},
|
||||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "child_a.child_b",
|
||||
LocalPath: filepath.Join(sourceDir, "child_a/child_b"),
|
||||
},
|
||||
}
|
||||
|
||||
if assertResultDeepEqual(t, hooks.Calls, wantCalls) {
|
||||
return
|
||||
}
|
||||
|
||||
loader, err := configload.NewLoader(&configload.Config{
|
||||
ModulesDir: modInstallDir,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Make sure the configuration is loadable now.
|
||||
// (This ensures that correct information is recorded in the manifest.)
|
||||
config, loadDiags := loader.LoadConfig(".")
|
||||
if assertNoDiagnostics(t, tfdiags.Diagnostics{}.Append(loadDiags)) {
|
||||
return
|
||||
}
|
||||
wantTraces := map[string]string{
|
||||
"": "in root module",
|
||||
"child_a": "in child_a module",
|
||||
"child_a.child_b": "in child_b module",
|
||||
}
|
||||
gotTraces := map[string]string{}
|
||||
|
||||
config.DeepEach(func(c *configs.Config) {
|
||||
path := strings.Join(c.Path, ".")
|
||||
if c.Module.Variables["v"] == nil {
|
||||
gotTraces[path] = "<missing>"
|
||||
return
|
||||
}
|
||||
varDesc := c.Module.Variables["v"].Description
|
||||
gotTraces[path] = varDesc
|
||||
})
|
||||
assertResultDeepEqual(t, gotTraces, wantTraces)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -268,7 +268,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
|
|||
Name: "Install",
|
||||
ModuleAddr: "acctest_child_a",
|
||||
Version: v,
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a"),
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/terraform-aws-module-installer-acctest-0.0.1/modules/child_a"),
|
||||
},
|
||||
|
||||
// acctest_child_a.child_b
|
||||
|
|
@ -276,7 +276,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
|
|||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "acctest_child_a.child_b",
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"),
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_a/terraform-aws-module-installer-acctest-0.0.1/modules/child_b"),
|
||||
},
|
||||
|
||||
// acctest_child_b accesses //modules/child_b directly
|
||||
|
|
@ -290,7 +290,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
|
|||
Name: "Install",
|
||||
ModuleAddr: "acctest_child_b",
|
||||
Version: v,
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_b/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"),
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_child_b/terraform-aws-module-installer-acctest-0.0.1/modules/child_b"),
|
||||
},
|
||||
|
||||
// acctest_root
|
||||
|
|
@ -304,7 +304,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
|
|||
Name: "Install",
|
||||
ModuleAddr: "acctest_root",
|
||||
Version: v,
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038"),
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/terraform-aws-module-installer-acctest-0.0.1"),
|
||||
},
|
||||
|
||||
// acctest_root.child_a
|
||||
|
|
@ -312,7 +312,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
|
|||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "acctest_root.child_a",
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a"),
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/terraform-aws-module-installer-acctest-0.0.1/modules/child_a"),
|
||||
},
|
||||
|
||||
// acctest_root.child_a.child_b
|
||||
|
|
@ -320,7 +320,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
|
|||
{
|
||||
Name: "Install",
|
||||
ModuleAddr: "acctest_root.child_a.child_b",
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b"),
|
||||
LocalPath: filepath.Join(dir, ".terraform/modules/acctest_root/terraform-aws-module-installer-acctest-0.0.1/modules/child_b"),
|
||||
},
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue