diff --git a/configs/configload/copy_dir.go b/configs/configload/copy_dir.go new file mode 100644 index 000000000..ad34a3204 --- /dev/null +++ b/configs/configload/copy_dir.go @@ -0,0 +1,114 @@ +package configload + +import ( + "io" + "os" + "path/filepath" + "strings" +) + +// copyDir copies the src directory contents into dst. Both directories +// should already exist. +func copyDir(dst, src string) error { + src, err := filepath.EvalSymlinks(src) + if err != nil { + return err + } + + walkFn := func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if path == src { + return nil + } + + if strings.HasPrefix(filepath.Base(path), ".") { + // Skip any dot files + if info.IsDir() { + return filepath.SkipDir + } else { + return nil + } + } + + // The "path" has the src prefixed to it. We need to join our + // destination with the path without the src on it. + dstPath := filepath.Join(dst, path[len(src):]) + + // we don't want to try and copy the same file over itself. + if eq, err := sameFile(path, dstPath); eq { + return nil + } else if err != nil { + return err + } + + // If we have a directory, make that subdirectory, then continue + // the walk. + if info.IsDir() { + if path == filepath.Join(src, dst) { + // dst is in src; don't walk it. + return nil + } + + if err := os.MkdirAll(dstPath, 0755); err != nil { + return err + } + + return nil + } + + // If we have a file, copy the contents. + srcF, err := os.Open(path) + if err != nil { + return err + } + defer srcF.Close() + + dstF, err := os.Create(dstPath) + if err != nil { + return err + } + defer dstF.Close() + + if _, err := io.Copy(dstF, srcF); err != nil { + return err + } + + // Chmod it + return os.Chmod(dstPath, info.Mode()) + } + + return filepath.Walk(src, walkFn) +} + +// sameFile tried to determine if to paths are the same file. +// If the paths don't match, we lookup the inode on supported systems. +func sameFile(a, b string) (bool, error) { + if a == b { + return true, nil + } + + aIno, err := inode(a) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + + bIno, err := inode(b) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + + if aIno > 0 && aIno == bIno { + return true, nil + } + + return false, nil +} diff --git a/configs/configload/getter.go b/configs/configload/getter.go new file mode 100644 index 000000000..fffc48c04 --- /dev/null +++ b/configs/configload/getter.go @@ -0,0 +1,122 @@ +package configload + +import ( + "log" + "path/filepath" + + cleanhttp "github.com/hashicorp/go-cleanhttp" + getter "github.com/hashicorp/go-getter" +) + +// We configure our own go-getter detector and getter sets here, because +// the set of sources we support is part of Terraform's documentation and +// so we don't want any new sources introduced in go-getter to sneak in here +// and work even though they aren't documented. This also insulates us from +// any meddling that might be done by other go-getter callers linked into our +// executable. + +var goGetterDetectors = []getter.Detector{ + new(getter.GitHubDetector), + new(getter.BitBucketDetector), + new(getter.S3Detector), + new(getter.FileDetector), +} + +var goGetterNoDetectors = []getter.Detector{} + +var goGetterDecompressors = map[string]getter.Decompressor{ + "bz2": new(getter.Bzip2Decompressor), + "gz": new(getter.GzipDecompressor), + "xz": new(getter.XzDecompressor), + "zip": new(getter.ZipDecompressor), + + "tar.bz2": new(getter.TarBzip2Decompressor), + "tar.tbz2": new(getter.TarBzip2Decompressor), + + "tar.gz": new(getter.TarGzipDecompressor), + "tgz": new(getter.TarGzipDecompressor), + + "tar.xz": new(getter.TarXzDecompressor), + "txz": new(getter.TarXzDecompressor), +} + +var goGetterGetters = map[string]getter.Getter{ + "file": new(getter.FileGetter), + "git": new(getter.GitGetter), + "hg": new(getter.HgGetter), + "s3": new(getter.S3Getter), + "http": getterHTTPGetter, + "https": getterHTTPGetter, +} + +var getterHTTPClient = cleanhttp.DefaultClient() + +var getterHTTPGetter = &getter.HttpGetter{ + Client: getterHTTPClient, + Netrc: true, +} + +// getWithGoGetter retrieves the package referenced in the given address +// into the installation path and then returns the full path to any subdir +// indicated in the address. +// +// The errors returned by this function are those surfaced by the underlying +// go-getter library, which have very inconsistent quality as +// end-user-actionable error messages. At this time we do not have any +// reasonable way to improve these error messages at this layer because +// the underlying errors are not separatelyr recognizable. +func getWithGoGetter(instPath, addr string) (string, error) { + packageAddr, subDir := splitAddrSubdir(addr) + + log.Printf("[DEBUG] will download %q to %s", packageAddr, instPath) + + realAddr, err := getter.Detect(packageAddr, instPath, getter.Detectors) + if err != nil { + return "", err + } + + var realSubDir string + realAddr, realSubDir = splitAddrSubdir(realAddr) + if realSubDir != "" { + subDir = filepath.Join(realSubDir, subDir) + } + + if realAddr != packageAddr { + log.Printf("[TRACE] go-getter detectors rewrote %q to %q", packageAddr, realAddr) + } + + client := getter.Client{ + Src: realAddr, + Dst: instPath, + Pwd: instPath, + + Mode: getter.ClientModeDir, + + Detectors: goGetterNoDetectors, // we already did detection above + Decompressors: goGetterDecompressors, + Getters: goGetterGetters, + } + err = client.Get() + if err != nil { + return "", err + } + + // Our subDir string can contain wildcards until this point, so that + // e.g. a subDir of * can expand to one top-level directory in a .tar.gz + // archive. Now that we've expanded the archive successfully we must + // resolve that into a concrete path. + var finalDir string + if subDir != "" { + finalDir, err = getter.SubdirGlob(instPath, subDir) + log.Printf("[TRACE] expanded %q to %q", subDir, finalDir) + if err != nil { + return "", err + } + } else { + finalDir = instPath + } + + // If we got this far then we have apparently succeeded in downloading + // the requested object! + return filepath.Clean(finalDir), nil +} diff --git a/configs/configload/inode.go b/configs/configload/inode.go new file mode 100644 index 000000000..57df04145 --- /dev/null +++ b/configs/configload/inode.go @@ -0,0 +1,21 @@ +// +build linux darwin openbsd netbsd solaris dragonfly + +package configload + +import ( + "fmt" + "os" + "syscall" +) + +// lookup the inode of a file on posix systems +func inode(path string) (uint64, error) { + stat, err := os.Stat(path) + if err != nil { + return 0, err + } + if st, ok := stat.Sys().(*syscall.Stat_t); ok { + return st.Ino, nil + } + return 0, fmt.Errorf("could not determine file inode") +} diff --git a/configs/configload/inode_freebsd.go b/configs/configload/inode_freebsd.go new file mode 100644 index 000000000..4dc28eaa8 --- /dev/null +++ b/configs/configload/inode_freebsd.go @@ -0,0 +1,21 @@ +// +build freebsd + +package configload + +import ( + "fmt" + "os" + "syscall" +) + +// lookup the inode of a file on posix systems +func inode(path string) (uint64, error) { + stat, err := os.Stat(path) + if err != nil { + return 0, err + } + if st, ok := stat.Sys().(*syscall.Stat_t); ok { + return uint64(st.Ino), nil + } + return 0, fmt.Errorf("could not determine file inode") +} diff --git a/configs/configload/inode_windows.go b/configs/configload/inode_windows.go new file mode 100644 index 000000000..0d22e6726 --- /dev/null +++ b/configs/configload/inode_windows.go @@ -0,0 +1,8 @@ +// +build windows + +package configload + +// no syscall.Stat_t on windows, return 0 for inodes +func inode(path string) (uint64, error) { + return 0, nil +} diff --git a/configs/configload/loader_install.go b/configs/configload/loader_install.go index 2ab6529d3..41ef4224d 100644 --- a/configs/configload/loader_install.go +++ b/configs/configload/loader_install.go @@ -10,6 +10,7 @@ import ( version "github.com/hashicorp/go-version" "github.com/hashicorp/hcl2/hcl" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/registry/regsrc" ) @@ -154,15 +155,16 @@ func (l *Loader) InstallModules(rootDir string, upgrade bool, hooks InstallHooks } log.Printf("[TRACE] %s is a registry module at %s", key, addr) - // TODO: Implement - panic("registry source installation not yet implemented") + mod, v, mDiags := l.installRegistryModule(req, key, instPath, addr, hooks) + diags = append(diags, mDiags...) + return mod, v, diags default: - log.Printf("[TRACE] %s address %q will be interpreted with go-getter", key, req.SourceAddr) - - // TODO: Implement - panic("fallback source installation not yet implemented") + log.Printf("[TRACE] %s address %q will be handled by go-getter", key, req.SourceAddr) + mod, mDiags := l.installGoGetterModule(req, key, instPath, hooks) + diags = append(diags, mDiags...) + return mod, nil, diags } }, @@ -226,12 +228,267 @@ func (l *Loader) installLocalModule(req *configs.ModuleRequest, key string, hook Dir: newDir, SourceAddr: req.SourceAddr, } - log.Printf("[TRACE] Module installer: %s installed at %s", key, newDir) + log.Printf("[DEBUG] Module installer: %s installed at %s", key, newDir) hooks.Install(key, nil, newDir) return mod, diags } +func (l *Loader) installRegistryModule(req *configs.ModuleRequest, key string, instPath string, addr *regsrc.Module, hooks InstallHooks) (*configs.Module, *version.Version, hcl.Diagnostics) { + var diags hcl.Diagnostics + + hostname, err := addr.SvcHost() + if err != nil { + // If it looks like the user was trying to use punycode then we'll generate + // a specialized error for that case. We require the unicode form of + // hostname so that hostnames are always human-readable in configuration + // and punycode can't be used to hide a malicious module hostname. + if strings.HasPrefix(addr.RawHost.Raw, "xn--") { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module registry hostname", + Detail: "The hostname portion of this source address is not an acceptable hostname. Internationalized domain names must be given in unicode form rather than ASCII (\"punycode\") form.", + Subject: &req.SourceAddrRange, + }) + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module registry hostname", + Detail: "The hostname portion of this source address is not a valid hostname.", + Subject: &req.SourceAddrRange, + }) + } + return nil, nil, diags + } + + reg := l.modules.Registry + + log.Printf("[DEBUG] %s listing available versions of %s at %s", key, addr, hostname) + resp, err := reg.Versions(addr) + if err != nil { + if registry.IsModuleNotFound(err) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Module not found", + Detail: fmt.Sprintf("The specified module could not be found in the module registry at %s.", hostname), + Subject: &req.SourceAddrRange, + }) + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Error accessing remote module registry", + Detail: fmt.Sprintf("Failed to retrieve available versions for this module from %s: %s.", hostname, err), + Subject: &req.SourceAddrRange, + }) + } + return nil, nil, diags + } + + // The response might contain information about dependencies to allow us + // to potentially optimize future requests, but we don't currently do that + // and so for now we'll just take the first item which is guaranteed to + // be the address we requested. + if len(resp.Modules) < 1 { + // Should never happen, but since this is a remote service that may + // be implemented by third-parties we will handle it gracefully. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid response from remote module registry", + Detail: fmt.Sprintf("The registry at %s returned an invalid response when Terraform requested available versions for this module.", hostname), + Subject: &req.SourceAddrRange, + }) + return nil, nil, diags + } + + modMeta := resp.Modules[0] + + var latestMatch *version.Version + var latestVersion *version.Version + for _, mv := range modMeta.Versions { + v, err := version.NewVersion(mv.Version) + if err != nil { + // Should never happen if the registry server is compliant with + // the protocol, but we'll warn if not to assist someone who + // might be developing a module registry server. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Invalid response from remote module registry", + Detail: fmt.Sprintf("The registry at %s returned an invalid version string %q for this module, which Terraform ignored.", hostname, mv.Version), + Subject: &req.SourceAddrRange, + }) + continue + } + + // If we've found a pre-release version then we'll ignore it unless + // it was exactly requested. + if v.Prerelease() != "" && req.VersionConstraint.Required.String() != v.String() { + log.Printf("[TRACE] %s ignoring %s because it is a pre-release and was not requested exactly", key, v) + continue + } + + if latestVersion == nil || v.GreaterThan(latestVersion) { + latestVersion = v + } + + if req.VersionConstraint.Required.Check(v) { + if latestMatch == nil || v.GreaterThan(latestMatch) { + latestMatch = v + } + } + } + + if latestVersion == nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Module has no versions", + Detail: fmt.Sprintf("The specified module does not have any available versions."), + Subject: &req.SourceAddrRange, + }) + return nil, nil, diags + } + + if latestMatch == nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unresolvable module version constraint", + Detail: fmt.Sprintf("There is no available version of %q that matches the given version constraint. The newest available version is %s.", addr, latestVersion), + Subject: &req.VersionConstraint.DeclRange, + }) + return nil, nil, diags + } + + // Report up to the caller that we're about to start downloading. + packageAddr, _ := splitAddrSubdir(req.SourceAddr) + hooks.Download(key, packageAddr, latestMatch) + + // If we manage to get down here then we've found a suitable version to + // install, so we need to ask the registry where we should download it from. + // The response to this is a go-getter-style address string. + dlAddr, err := reg.Location(addr, latestMatch.String()) + if err != nil { + log.Printf("[ERROR] %s from %s %s: %s", key, addr, latestMatch, err) + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid response from remote module registry", + Detail: fmt.Sprintf("The remote registry at %s failed to return a download URL for %s %s.", hostname, addr, latestMatch), + Subject: &req.VersionConstraint.DeclRange, + }) + return nil, nil, diags + } + + log.Printf("[TRACE] %s %s %s is available at %q", key, addr, latestMatch, dlAddr) + + modDir, err := getWithGoGetter(instPath, dlAddr) + if err != nil { + // Errors returned by go-getter have very inconsistent quality as + // end-user error messages, but for now we're accepting that because + // we have no way to recognize any specific errors to improve them + // and masking the error entirely would hide valuable diagnostic + // information from the user. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to download module", + Detail: fmt.Sprintf("Error attempting to download module source code from %q: %s", dlAddr, err), + Subject: &req.CallRange, + }) + return nil, nil, diags + } + + log.Printf("[TRACE] %s %q was downloaded to %s", key, dlAddr, modDir) + + if addr.RawSubmodule != "" { + // Append the user's requested subdirectory to any subdirectory that + // was implied by any of the nested layers we expanded within go-getter. + modDir = filepath.Join(modDir, addr.RawSubmodule) + } + + log.Printf("[TRACE] %s should now be at %s", key, modDir) + + // Finally we are ready to try actually loading the module. + mod, mDiags := l.parser.LoadConfigDir(modDir) + if mod == nil { + // nil indicates missing or unreadable directory, so we'll + // discard the returned diags and return a more specific + // error message here. For registry modules this actually + // indicates a bug in the code above, since it's not the + // user's responsibility to create the directory in this case. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unreadable module directory", + Detail: fmt.Sprintf("The directory %s could not be read. This is a bug in Terraform and should be reported.", modDir), + Subject: &req.CallRange, + }) + } else { + diags = append(diags, mDiags...) + } + + // Note the local location in our manifest. + l.modules.manifest[key] = moduleRecord{ + Key: key, + Version: latestMatch, + Dir: modDir, + SourceAddr: req.SourceAddr, + } + log.Printf("[DEBUG] Module installer: %s installed at %s", key, modDir) + hooks.Install(key, latestMatch, modDir) + + return mod, latestMatch, diags +} + +func (l *Loader) installGoGetterModule(req *configs.ModuleRequest, key string, instPath string, hooks InstallHooks) (*configs.Module, hcl.Diagnostics) { + var diags hcl.Diagnostics + + // Report up to the caller that we're about to start downloading. + packageAddr, _ := splitAddrSubdir(req.SourceAddr) + hooks.Download(key, packageAddr, nil) + + modDir, err := getWithGoGetter(instPath, req.SourceAddr) + if err != nil { + // Errors returned by go-getter have very inconsistent quality as + // end-user error messages, but for now we're accepting that because + // we have no way to recognize any specific errors to improve them + // and masking the error entirely would hide valuable diagnostic + // information from the user. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to download module", + Detail: fmt.Sprintf("Error attempting to download module source code from %q: %s", packageAddr, err), + Subject: &req.SourceAddrRange, + }) + return nil, diags + } + + log.Printf("[TRACE] %s %q was downloaded to %s", key, req.SourceAddr, modDir) + + mod, mDiags := l.parser.LoadConfigDir(modDir) + if mod == nil { + // nil indicates missing or unreadable directory, so we'll + // discard the returned diags and return a more specific + // error message here. For registry modules this actually + // indicates a bug in the code above, since it's not the + // user's responsibility to create the directory in this case. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unreadable module directory", + Detail: fmt.Sprintf("The directory %s could not be read. This is a bug in Terraform and should be reported.", modDir), + Subject: &req.CallRange, + }) + } else { + diags = append(diags, mDiags...) + } + + // Note the local location in our manifest. + l.modules.manifest[key] = moduleRecord{ + Key: key, + Dir: modDir, + SourceAddr: req.SourceAddr, + } + log.Printf("[DEBUG] Module installer: %s installed at %s", key, modDir) + hooks.Install(key, nil, modDir) + + return mod, diags +} + func (l *Loader) packageInstallPath(modulePath []string) string { return filepath.Join(l.modules.Dir, strings.Join(modulePath, ".")) } diff --git a/configs/configload/loader_install_test.go b/configs/configload/loader_install_test.go index 608b08a00..2be85a6ba 100644 --- a/configs/configload/loader_install_test.go +++ b/configs/configload/loader_install_test.go @@ -1,6 +1,7 @@ package configload import ( + "os" "path/filepath" "testing" @@ -9,11 +10,12 @@ import ( func TestLoaderInstallModules_local(t *testing.T) { fixtureDir := filepath.Clean("test-fixtures/local-modules") - loader := newTestLoader(filepath.Join(fixtureDir, ".terraform/modules")) + loader, done := tempChdirLoader(t, fixtureDir) + defer done() hooks := &testInstallHooks{} - diags := loader.InstallModules(fixtureDir, false, hooks) + diags := loader.InstallModules(".", false, hooks) assertNoDiagnostics(t, diags) wantCalls := []testInstallHookCall{ @@ -21,17 +23,210 @@ func TestLoaderInstallModules_local(t *testing.T) { Name: "Install", ModuleAddr: "child_a", PackageAddr: "", - LocalPath: "test-fixtures/local-modules/child_a", + LocalPath: "child_a", }, { Name: "Install", ModuleAddr: "child_a.child_b", PackageAddr: "", - LocalPath: "test-fixtures/local-modules/child_a/child_b", + LocalPath: "child_a/child_b", }, } - assertResultDeepEqual(t, hooks.Calls, wantCalls) + if assertResultDeepEqual(t, hooks.Calls, wantCalls) { + return + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + _, loadDiags := loader.LoadConfig(".") + assertNoDiagnostics(t, loadDiags) +} + +func TestLoaderInstallModules_registry(t *testing.T) { + if os.Getenv("TF_ACC") == "" { + t.Skip("this test accesses registry.terraform.io and github.com; set TF_ACC=1 to run it") + } + + fixtureDir := filepath.Clean("test-fixtures/registry-modules") + loader, done := tempChdirLoader(t, fixtureDir) + defer done() + + hooks := &testInstallHooks{} + + diags := loader.InstallModules(".", false, hooks) + assertNoDiagnostics(t, diags) + + v := version.Must(version.NewVersion("0.0.1")) + + wantCalls := []testInstallHookCall{ + // the configuration builder visits each level of calls in lexicographical + // order by name, so the following list is kept in the same order. + + // acctest_child_a accesses //modules/child_a directly + { + Name: "Download", + ModuleAddr: "acctest_child_a", + PackageAddr: "hashicorp/module-installer-acctest/aws", // intentionally excludes the subdir because we're downloading the whole package here + Version: v, + }, + { + Name: "Install", + ModuleAddr: "acctest_child_a", + Version: v, + LocalPath: ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a", + }, + + // acctest_child_a.child_b + // (no download because it's a relative path inside acctest_child_a) + { + Name: "Install", + ModuleAddr: "acctest_child_a.child_b", + LocalPath: ".terraform/modules/acctest_child_a/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b", + }, + + // acctest_child_b accesses //modules/child_b directly + { + Name: "Download", + ModuleAddr: "acctest_child_b", + PackageAddr: "hashicorp/module-installer-acctest/aws", // intentionally excludes the subdir because we're downloading the whole package here + Version: v, + }, + { + Name: "Install", + ModuleAddr: "acctest_child_b", + Version: v, + LocalPath: ".terraform/modules/acctest_child_b/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b", + }, + + // acctest_root + { + Name: "Download", + ModuleAddr: "acctest_root", + PackageAddr: "hashicorp/module-installer-acctest/aws", + Version: v, + }, + { + Name: "Install", + ModuleAddr: "acctest_root", + Version: v, + LocalPath: ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038", + }, + + // acctest_root.child_a + // (no download because it's a relative path inside acctest_root) + { + Name: "Install", + ModuleAddr: "acctest_root.child_a", + LocalPath: ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_a", + }, + + // acctest_root.child_a.child_b + // (no download because it's a relative path inside acctest_root, via acctest_root.child_a) + { + Name: "Install", + ModuleAddr: "acctest_root.child_a.child_b", + LocalPath: ".terraform/modules/acctest_root/hashicorp-terraform-aws-module-installer-acctest-853d038/modules/child_b", + }, + } + + if assertResultDeepEqual(t, hooks.Calls, wantCalls) { + return + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + _, loadDiags := loader.LoadConfig(".") + assertNoDiagnostics(t, loadDiags) +} + +func TestLoaderInstallModules_goGetter(t *testing.T) { + if os.Getenv("TF_ACC") == "" { + t.Skip("this test accesses github.com; set TF_ACC=1 to run it") + } + + fixtureDir := filepath.Clean("test-fixtures/go-getter-modules") + loader, done := tempChdirLoader(t, fixtureDir) + defer done() + + hooks := &testInstallHooks{} + + diags := loader.InstallModules(".", false, hooks) + assertNoDiagnostics(t, diags) + + wantCalls := []testInstallHookCall{ + // the configuration builder visits each level of calls in lexicographical + // order by name, so the following list is kept in the same order. + + // acctest_child_a accesses //modules/child_a directly + { + Name: "Download", + ModuleAddr: "acctest_child_a", + PackageAddr: "github.com/hashicorp/terraform-aws-module-installer-acctest?ref=v0.0.1", // intentionally excludes the subdir because we're downloading the whole repo here + }, + { + Name: "Install", + ModuleAddr: "acctest_child_a", + LocalPath: ".terraform/modules/acctest_child_a/modules/child_a", + }, + + // acctest_child_a.child_b + // (no download because it's a relative path inside acctest_child_a) + { + Name: "Install", + ModuleAddr: "acctest_child_a.child_b", + LocalPath: ".terraform/modules/acctest_child_a/modules/child_b", + }, + + // acctest_child_b accesses //modules/child_b directly + { + Name: "Download", + ModuleAddr: "acctest_child_b", + PackageAddr: "github.com/hashicorp/terraform-aws-module-installer-acctest?ref=v0.0.1", // intentionally excludes the subdir because we're downloading the whole package here + }, + { + Name: "Install", + ModuleAddr: "acctest_child_b", + LocalPath: ".terraform/modules/acctest_child_b/modules/child_b", + }, + + // acctest_root + { + Name: "Download", + ModuleAddr: "acctest_root", + PackageAddr: "github.com/hashicorp/terraform-aws-module-installer-acctest?ref=v0.0.1", + }, + { + Name: "Install", + ModuleAddr: "acctest_root", + LocalPath: ".terraform/modules/acctest_root", + }, + + // acctest_root.child_a + // (no download because it's a relative path inside acctest_root) + { + Name: "Install", + ModuleAddr: "acctest_root.child_a", + LocalPath: ".terraform/modules/acctest_root/modules/child_a", + }, + + // acctest_root.child_a.child_b + // (no download because it's a relative path inside acctest_root, via acctest_root.child_a) + { + Name: "Install", + ModuleAddr: "acctest_root.child_a.child_b", + LocalPath: ".terraform/modules/acctest_root/modules/child_b", + }, + } + + if assertResultDeepEqual(t, hooks.Calls, wantCalls) { + return + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + _, loadDiags := loader.LoadConfig(".") + assertNoDiagnostics(t, loadDiags) } type testInstallHooks struct { diff --git a/configs/configload/loader_test.go b/configs/configload/loader_test.go index 6ab1fc8c5..239599dc1 100644 --- a/configs/configload/loader_test.go +++ b/configs/configload/loader_test.go @@ -1,43 +1,90 @@ package configload import ( - "reflect" + "fmt" + "io/ioutil" + "os" + "path/filepath" "testing" - "github.com/spf13/afero" - - "github.com/davecgh/go-spew/spew" + "github.com/go-test/deep" "github.com/hashicorp/hcl2/hcl" - "github.com/hashicorp/terraform/configs" - "github.com/hashicorp/terraform/registry" "github.com/zclconf/go-cty/cty" ) -// newTestLoader is like NewLoader but it uses a copy-on-write overlay filesystem -// over the real filesystem so that any files that are created cannot persist -// between test runs. +// tempChdir copies the contents of the given directory to a temporary +// directory and changes the test process's current working directory to +// point to that directory. Also returned is a function that should be +// called at the end of the test (e.g. via "defer") to restore the previous +// working directory. // -// It will also panic if there are any errors creating the loader, since -// these should never happen in a testing scenario. -func newTestLoader(dir string) *Loader { - realFS := afero.NewOsFs() - overlayFS := afero.NewMemMapFs() - fs := afero.NewCopyOnWriteFs(realFS, overlayFS) - parser := configs.NewParser(fs) - reg := registry.NewClient(nil, nil, nil) - ret := &Loader{ - parser: parser, - modules: moduleMgr{ - FS: afero.Afero{fs}, - Dir: dir, - Registry: reg, - }, - } - err := ret.modules.readModuleManifestSnapshot() +// Tests using this helper cannot safely be run in parallel with other tests. +func tempChdir(t *testing.T, sourceDir string) (string, func()) { + t.Helper() + + tmpDir, err := ioutil.TempDir("", "terraform-configload") if err != nil { - panic(err) + t.Fatalf("failed to create temporary directory: %s", err) + return "", nil } - return ret + + if err := copyDir(tmpDir, sourceDir); err != nil { + t.Fatalf("failed to copy fixture to temporary directory: %s", err) + return "", nil + } + + oldDir, err := os.Getwd() + if err != nil { + t.Fatalf("failed to determine current working directory: %s", err) + return "", nil + } + + err = os.Chdir(tmpDir) + if err != nil { + t.Fatalf("failed to switch to temp dir %s: %s", tmpDir, err) + return "", nil + } + + t.Logf("tempChdir switched to %s after copying from %s", tmpDir, sourceDir) + + return tmpDir, func() { + err := os.Chdir(oldDir) + if err != nil { + panic(fmt.Errorf("failed to restore previous working directory %s: %s", oldDir, err)) + } + + if os.Getenv("TF_CONFIGLOAD_TEST_KEEP_TMP") == "" { + os.RemoveAll(tmpDir) + } + } +} + +// tempChdirLoader is a wrapper around tempChdir that also returns a Loader +// whose modules directory is at the conventional location within the +// created temporary directory. +func tempChdirLoader(t *testing.T, sourceDir string) (*Loader, func()) { + t.Helper() + + _, done := tempChdir(t, sourceDir) + modulesDir := filepath.Clean(".terraform/modules") + + err := os.MkdirAll(modulesDir, os.ModePerm) + if err != nil { + done() // undo the chdir in tempChdir so we can safely run other tests + t.Fatalf("failed to create modules directory: %s", err) + return nil, nil + } + + loader, err := NewLoader(&Config{ + ModulesDir: modulesDir, + }) + if err != nil { + done() // undo the chdir in tempChdir so we can safely run other tests + t.Fatalf("failed to create loader: %s", err) + return nil, nil + } + + return loader, done } func assertNoDiagnostics(t *testing.T, diags hcl.Diagnostics) bool { @@ -75,8 +122,10 @@ func assertDiagnosticSummary(t *testing.T, diags hcl.Diagnostics, want string) b func assertResultDeepEqual(t *testing.T, got, want interface{}) bool { t.Helper() - if !reflect.DeepEqual(got, want) { - t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want)) + if diff := deep.Equal(got, want); diff != nil { + for _, problem := range diff { + t.Errorf("%s", problem) + } return true } return false diff --git a/configs/configload/source_addr.go b/configs/configload/source_addr.go index 7767859c5..594cf6406 100644 --- a/configs/configload/source_addr.go +++ b/configs/configload/source_addr.go @@ -3,6 +3,8 @@ package configload import ( "strings" + "github.com/hashicorp/go-getter" + "github.com/hashicorp/terraform/registry/regsrc" ) @@ -26,3 +28,18 @@ func isRegistrySourceAddr(addr string) bool { _, err := regsrc.ParseModuleSource(addr) return err == nil } + +// splitAddrSubdir splits the given address (which is assumed to be a +// registry address or go-getter-style address) into a package portion +// and a sub-directory portion. +// +// The package portion defines what should be downloaded and then the +// sub-directory portion, if present, specifies a sub-directory within +// the downloaded object (an archive, VCS repository, etc) that contains +// the module's configuration files. +// +// The subDir portion will be returned as empty if no subdir separator +// ("//") is present in the address. +func splitAddrSubdir(addr string) (packageAddr, subDir string) { + return getter.SourceDirSubdir(addr) +} diff --git a/configs/configload/test-fixtures/go-getter-modules/.gitignore b/configs/configload/test-fixtures/go-getter-modules/.gitignore new file mode 100644 index 000000000..6e0db03a8 --- /dev/null +++ b/configs/configload/test-fixtures/go-getter-modules/.gitignore @@ -0,0 +1 @@ +.terraform/* diff --git a/configs/configload/test-fixtures/go-getter-modules/root.tf b/configs/configload/test-fixtures/go-getter-modules/root.tf new file mode 100644 index 000000000..3c92ef1db --- /dev/null +++ b/configs/configload/test-fixtures/go-getter-modules/root.tf @@ -0,0 +1,16 @@ +# This fixture depends on a github repo at: +# https://github.com/hashicorp/terraform-aws-module-installer-acctest +# ...and expects its v0.0.1 tag to be pointing at the following commit: +# d676ab2559d4e0621d59e3c3c4cbb33958ac4608 + +module "acctest_root" { + source = "github.com/hashicorp/terraform-aws-module-installer-acctest?ref=v0.0.1" +} + +module "acctest_child_a" { + source = "github.com/hashicorp/terraform-aws-module-installer-acctest//modules/child_a?ref=v0.0.1" +} + +module "acctest_child_b" { + source = "github.com/hashicorp/terraform-aws-module-installer-acctest//modules/child_b?ref=v0.0.1" +} diff --git a/configs/configload/test-fixtures/registry-modules/.gitignore b/configs/configload/test-fixtures/registry-modules/.gitignore new file mode 100644 index 000000000..6e0db03a8 --- /dev/null +++ b/configs/configload/test-fixtures/registry-modules/.gitignore @@ -0,0 +1 @@ +.terraform/* diff --git a/configs/configload/test-fixtures/registry-modules/root.tf b/configs/configload/test-fixtures/registry-modules/root.tf new file mode 100644 index 000000000..786966494 --- /dev/null +++ b/configs/configload/test-fixtures/registry-modules/root.tf @@ -0,0 +1,27 @@ +# This fixture indirectly depends on a github repo at: +# https://github.com/hashicorp/terraform-aws-module-installer-acctest +# ...and expects its v0.0.1 tag to be pointing at the following commit: +# d676ab2559d4e0621d59e3c3c4cbb33958ac4608 +# +# This repository is accessed indirectly via: +# https://registry.terraform.io/modules/hashicorp/module-installer-acctest/aws/0.0.1 +# +# Since the tag's id is included in a downloaded archive, it is expected to +# have the following id: +# 853d03855b3290a3ca491d4c3a7684572dd42237 +# (this particular assumption is encoded in the tests that use this fixture) + +module "acctest_root" { + source = "hashicorp/module-installer-acctest/aws" + version = "0.0.1" +} + +module "acctest_child_a" { + source = "hashicorp/module-installer-acctest/aws//modules/child_a" + version = "0.0.1" +} + +module "acctest_child_b" { + source = "hashicorp/module-installer-acctest/aws//modules/child_b" + version = "0.0.1" +} diff --git a/registry/client.go b/registry/client.go index c69d31b15..6525c5410 100644 --- a/registry/client.go +++ b/registry/client.go @@ -115,7 +115,7 @@ func (c *Client) Versions(module *regsrc.Module) (*response.ModuleVersions, erro case http.StatusOK: // OK case http.StatusNotFound: - return nil, fmt.Errorf("module %q not found", module.String()) + return nil, &errModuleNotFound{addr: module} default: return nil, fmt.Errorf("error looking up module versions: %s", resp.Status) } diff --git a/registry/errors.go b/registry/errors.go new file mode 100644 index 000000000..b8dcd31e3 --- /dev/null +++ b/registry/errors.go @@ -0,0 +1,23 @@ +package registry + +import ( + "fmt" + + "github.com/hashicorp/terraform/registry/regsrc" +) + +type errModuleNotFound struct { + addr *regsrc.Module +} + +func (e *errModuleNotFound) Error() string { + return fmt.Sprintf("module %s not found", e.addr) +} + +// IsModuleNotFound returns true only if the given error is a "module not found" +// error. This allows callers to recognize this particular error condition +// as distinct from operational errors such as poor network connectivity. +func IsModuleNotFound(err error) bool { + _, ok := err.(*errModuleNotFound) + return ok +}