From 40ec62c1391f65147c5b8239d97002705dfde074 Mon Sep 17 00:00:00 2001 From: kmoe <5575356+kmoe@users.noreply.github.com> Date: Mon, 1 Nov 2021 20:09:16 +0000 Subject: [PATCH] command: make module installation interruptible Earlier work to make "terraform init" interruptible made the getproviders package context-aware in order to allow provider installation to be cancelled. Here we make a similar change for module installation, which is now also cancellable with SIGINT. This involves plumbing context through initwd and getmodules. Functions which can make network requests now include a context parameter whose cancellation cancels those requests. Since the module installation code is shared, "terraform get" is now also interruptible during module installation. --- internal/command/command_test.go | 3 +- internal/command/get.go | 6 +-- internal/command/get_test.go | 32 +++++++++++++ internal/command/init.go | 24 +++++----- internal/command/init_test.go | 40 ++++++++++++++++- internal/command/meta_config.go | 45 ++++++++++++++----- internal/command/test.go | 2 +- .../testdata/init-registry-module/main.tf | 4 ++ internal/getmodules/getter.go | 4 +- internal/getmodules/installer.go | 8 +++- internal/initwd/from_module.go | 5 ++- internal/initwd/from_module_test.go | 7 +-- internal/initwd/module_install.go | 38 +++++++++++----- internal/initwd/module_install_test.go | 21 ++++----- internal/initwd/testing.go | 3 +- internal/refactoring/move_validate_test.go | 3 +- internal/registry/client.go | 8 +++- internal/registry/client_test.go | 24 +++++----- internal/terraform/terraform_test.go | 5 ++- 19 files changed, 207 insertions(+), 75 deletions(-) create mode 100644 internal/command/testdata/init-registry-module/main.tf diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 5fab71f1f..4a56ecc73 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -2,6 +2,7 @@ package command import ( "bytes" + "context" "crypto/md5" "encoding/base64" "encoding/json" @@ -188,7 +189,7 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. inst := initwd.NewModuleInstaller(loader.ModulesDir(), registry.NewClient(nil, nil)) - _, instDiags := inst.InstallModules(dir, true, initwd.ModuleInstallHooksImpl{}) + _, instDiags := inst.InstallModules(context.Background(), dir, true, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) } diff --git a/internal/command/get.go b/internal/command/get.go index 170e19400..0f541c3b1 100644 --- a/internal/command/get.go +++ b/internal/command/get.go @@ -33,9 +33,9 @@ func (c *GetCommand) Run(args []string) int { path = c.normalizePath(path) - diags := getModules(&c.Meta, path, update) + abort, diags := getModules(&c.Meta, path, update) c.showDiagnostics(diags) - if diags.HasErrors() { + if abort || diags.HasErrors() { return 1 } @@ -73,7 +73,7 @@ func (c *GetCommand) Synopsis() string { return "Install or upgrade remote Terraform modules" } -func getModules(m *Meta, path string, upgrade bool) tfdiags.Diagnostics { +func getModules(m *Meta, path string, upgrade bool) (abort bool, diags tfdiags.Diagnostics) { hooks := uiModuleInstallHooks{ Ui: m.Ui, ShowLocalPaths: true, diff --git a/internal/command/get_test.go b/internal/command/get_test.go index b2a3ea0a7..77daeb0b4 100644 --- a/internal/command/get_test.go +++ b/internal/command/get_test.go @@ -79,3 +79,35 @@ func TestGet_update(t *testing.T) { t.Fatalf("doesn't look like get: %s", output) } } + +func TestGet_cancel(t *testing.T) { + // This test runs `terraform get` as if SIGINT (or similar on other + // platforms) were sent to it, testing that it is interruptible. + + wd := tempWorkingDirFixture(t, "init-registry-module") + defer testChdir(t, wd.RootModuleDir())() + + // Our shutdown channel is pre-closed so init will exit as soon as it + // starts a cancelable portion of the process. + shutdownCh := make(chan struct{}) + close(shutdownCh) + + ui := cli.NewMockUi() + c := &GetCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + WorkingDir: wd, + ShutdownCh: shutdownCh, + }, + } + + args := []string{} + if code := c.Run(args); code == 0 { + t.Fatalf("succeeded; wanted error\n%s", ui.OutputWriter.String()) + } + + if got, want := ui.ErrorWriter.String(), `Module installation was canceled by an interrupt signal`; !strings.Contains(got, want) { + t.Fatalf("wrong error message\nshould contain: %s\ngot:\n%s", want, got) + } +} diff --git a/internal/command/init.go b/internal/command/init.go index 39a902c96..b8ba6b9ff 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -115,9 +115,9 @@ func (c *InitCommand) Run(args []string) int { ShowLocalPaths: false, // since they are in a weird location for init } - initDiags := c.initDirFromModule(path, src, hooks) - diags = diags.Append(initDiags) - if initDiags.HasErrors() { + initDirFromModuleAbort, initDirFromModuleDiags := c.initDirFromModule(path, src, hooks) + diags = diags.Append(initDirFromModuleDiags) + if initDirFromModuleAbort || initDirFromModuleDiags.HasErrors() { c.showDiagnostics(diags) return 1 } @@ -174,9 +174,9 @@ func (c *InitCommand) Run(args []string) int { } if flagGet { - modsOutput, modsDiags := c.getModules(path, rootModEarly, flagUpgrade) + modsOutput, modsAbort, modsDiags := c.getModules(path, rootModEarly, flagUpgrade) diags = diags.Append(modsDiags) - if modsDiags.HasErrors() { + if modsAbort || modsDiags.HasErrors() { c.showDiagnostics(diags) return 1 } @@ -326,10 +326,10 @@ func (c *InitCommand) Run(args []string) int { return 0 } -func (c *InitCommand) getModules(path string, earlyRoot *tfconfig.Module, upgrade bool) (output bool, diags tfdiags.Diagnostics) { +func (c *InitCommand) getModules(path string, earlyRoot *tfconfig.Module, upgrade bool) (output bool, abort bool, diags tfdiags.Diagnostics) { if len(earlyRoot.ModuleCalls) == 0 { // Nothing to do - return false, nil + return false, false, nil } if upgrade { @@ -342,8 +342,12 @@ func (c *InitCommand) getModules(path string, earlyRoot *tfconfig.Module, upgrad Ui: c.Ui, ShowLocalPaths: true, } - instDiags := c.installModules(path, upgrade, hooks) - diags = diags.Append(instDiags) + + installAbort, installDiags := c.installModules(path, upgrade, hooks) + diags = diags.Append(installDiags) + if installAbort || installDiags.HasErrors() { + return true, true, diags + } // Since module installer has modified the module manifest on disk, we need // to refresh the cache of it in the loader. @@ -358,7 +362,7 @@ func (c *InitCommand) getModules(path string, earlyRoot *tfconfig.Module, upgrad } } - return true, diags + return true, false, diags } func (c *InitCommand) initCloud(root *configs.Module) (be backend.Backend, output bool, diags tfdiags.Diagnostics) { diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 56bc1f911..b952ea0a2 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -1417,7 +1417,45 @@ func TestInit_providerSource(t *testing.T) { } } -func TestInit_cancel(t *testing.T) { +func TestInit_cancelModules(t *testing.T) { + // This test runs `terraform init` as if SIGINT (or similar on other + // platforms) were sent to it, testing that it is interruptible. + + td := tempDir(t) + testCopyDir(t, testFixturePath("init-registry-module"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + // Our shutdown channel is pre-closed so init will exit as soon as it + // starts a cancelable portion of the process. + shutdownCh := make(chan struct{}) + close(shutdownCh) + + ui := cli.NewMockUi() + view, _ := testView(t) + m := Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + ShutdownCh: shutdownCh, + } + + c := &InitCommand{ + Meta: m, + } + + args := []string{} + + if code := c.Run(args); code == 0 { + t.Fatalf("succeeded; wanted error\n%s", ui.OutputWriter.String()) + } + + if got, want := ui.ErrorWriter.String(), `Module installation was canceled by an interrupt signal`; !strings.Contains(got, want) { + t.Fatalf("wrong error message\nshould contain: %s\ngot:\n%s", want, got) + } +} + +func TestInit_cancelProviders(t *testing.T) { // This test runs `terraform init` as if SIGINT (or similar on other // platforms) were sent to it, testing that it is interruptible. diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index 6913db594..0abaf52a7 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -164,26 +164,39 @@ func (m *Meta) loadHCLFile(filename string) (hcl.Body, tfdiags.Diagnostics) { } // installModules reads a root module from the given directory and attempts -// recursively install all of its descendent modules. +// recursively to install all of its descendent modules. // // The given hooks object will be notified of installation progress, which -// can then be relayed to the end-user. The moduleUiInstallHooks type in +// can then be relayed to the end-user. The uiModuleInstallHooks type in // this package has a reasonable implementation for displaying notifications // via a provided cli.Ui. -func (m *Meta) installModules(rootDir string, upgrade bool, hooks initwd.ModuleInstallHooks) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics +func (m *Meta) installModules(rootDir string, upgrade bool, hooks initwd.ModuleInstallHooks) (abort bool, diags tfdiags.Diagnostics) { rootDir = m.normalizePath(rootDir) err := os.MkdirAll(m.modulesDir(), os.ModePerm) if err != nil { diags = diags.Append(fmt.Errorf("failed to create local modules directory: %s", err)) - return diags + return true, diags } + // FIXME: KEM: does returning the abort here change behaviour in a particular error + // case? inst := m.moduleInstaller() - _, moreDiags := inst.InstallModules(rootDir, upgrade, hooks) + + // Installation can be aborted by interruption signals + ctx, done := m.InterruptibleContext() + defer done() + + _, moreDiags := inst.InstallModules(ctx, rootDir, upgrade, hooks) diags = diags.Append(moreDiags) - return diags + + if ctx.Err() == context.Canceled { + m.showDiagnostics(diags) + m.Ui.Error("Module installation was canceled by an interrupt signal.") + return true, diags + } + + return false, diags } // initDirFromModule initializes the given directory (which should be @@ -192,15 +205,23 @@ func (m *Meta) installModules(rootDir string, upgrade bool, hooks initwd.ModuleI // // Internally this runs similar steps to installModules. // The given hooks object will be notified of installation progress, which -// can then be relayed to the end-user. The moduleUiInstallHooks type in +// can then be relayed to the end-user. The uiModuleInstallHooks type in // this package has a reasonable implementation for displaying notifications // via a provided cli.Ui. -func (m *Meta) initDirFromModule(targetDir string, addr string, hooks initwd.ModuleInstallHooks) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics +func (m *Meta) initDirFromModule(targetDir string, addr string, hooks initwd.ModuleInstallHooks) (abort bool, diags tfdiags.Diagnostics) { + // Installation can be aborted by interruption signals + ctx, done := m.InterruptibleContext() + defer done() + targetDir = m.normalizePath(targetDir) - moreDiags := initwd.DirFromModule(targetDir, m.modulesDir(), addr, m.registryClient(), hooks) + moreDiags := initwd.DirFromModule(ctx, targetDir, m.modulesDir(), addr, m.registryClient(), hooks) diags = diags.Append(moreDiags) - return diags + if ctx.Err() == context.Canceled { + m.showDiagnostics(diags) + m.Ui.Error("Module initialization was canceled by an interrupt signal.") + return true, diags + } + return false, diags } // inputForSchema uses interactive prompts to try to populate any diff --git a/internal/command/test.go b/internal/command/test.go index eff291179..fc6d7ef70 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -249,7 +249,7 @@ func (c *TestCommand) prepareSuiteDir(ctx context.Context, suiteName string) (te os.MkdirAll(suiteDirs.ModulesDir, 0755) // if this fails then we'll ignore it and let InstallModules below fail instead reg := c.registryClient() moduleInst := initwd.NewModuleInstaller(suiteDirs.ModulesDir, reg) - _, moreDiags := moduleInst.InstallModules(configDir, true, nil) + _, moreDiags := moduleInst.InstallModules(ctx, configDir, true, nil) diags = diags.Append(moreDiags) if diags.HasErrors() { return suiteDirs, diags diff --git a/internal/command/testdata/init-registry-module/main.tf b/internal/command/testdata/init-registry-module/main.tf new file mode 100644 index 000000000..cc388d7c5 --- /dev/null +++ b/internal/command/testdata/init-registry-module/main.tf @@ -0,0 +1,4 @@ +module "foo" { + source = "registry.does.not.exist/example_corp/foo/bar" + version = "0.1.0" +} diff --git a/internal/getmodules/getter.go b/internal/getmodules/getter.go index 67b5c2760..95f334762 100644 --- a/internal/getmodules/getter.go +++ b/internal/getmodules/getter.go @@ -1,6 +1,7 @@ package getmodules import ( + "context" "fmt" "log" "os" @@ -119,7 +120,7 @@ type reusingGetter map[string]string // 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 separately recognizable. -func (g reusingGetter) getWithGoGetter(instPath, packageAddr string) error { +func (g reusingGetter) getWithGoGetter(ctx context.Context, instPath, packageAddr string) error { var err error if prevDir, exists := g[packageAddr]; exists { @@ -144,6 +145,7 @@ func (g reusingGetter) getWithGoGetter(instPath, packageAddr string) error { Detectors: goGetterNoDetectors, // our caller should've already done detection Decompressors: goGetterDecompressors, Getters: goGetterGetters, + Ctx: ctx, } err = client.Get() if err != nil { diff --git a/internal/getmodules/installer.go b/internal/getmodules/installer.go index 087c0cd3d..657c0e73a 100644 --- a/internal/getmodules/installer.go +++ b/internal/getmodules/installer.go @@ -1,5 +1,9 @@ package getmodules +import ( + "context" +) + // PackageFetcher is a low-level utility for fetching remote module packages // into local filesystem directories in preparation for use by higher-level // module installer functionality implemented elsewhere. @@ -35,6 +39,6 @@ func NewPackageFetcher() *PackageFetcher { // a module source address which includes a subdirectory portion then the // caller must resolve that itself, possibly with the help of the // getmodules.SplitPackageSubdir and getmodules.ExpandSubdirGlobs functions. -func (f *PackageFetcher) FetchPackage(instDir string, packageAddr string) error { - return f.getter.getWithGoGetter(instDir, packageAddr) +func (f *PackageFetcher) FetchPackage(ctx context.Context, instDir string, packageAddr string) error { + return f.getter.getWithGoGetter(ctx, instDir, packageAddr) } diff --git a/internal/initwd/from_module.go b/internal/initwd/from_module.go index 392370032..38a4e49b5 100644 --- a/internal/initwd/from_module.go +++ b/internal/initwd/from_module.go @@ -1,6 +1,7 @@ package initwd import ( + "context" "fmt" "io/ioutil" "log" @@ -41,7 +42,7 @@ const initFromModuleRootKeyPrefix = initFromModuleRootCallName + "." // references using ../ from that module to be unresolvable. Error diagnostics // are produced in that case, to prompt the user to rewrite the source strings // to be absolute references to the original remote module. -func DirFromModule(rootDir, modulesDir, sourceAddr string, reg *registry.Client, hooks ModuleInstallHooks) tfdiags.Diagnostics { +func DirFromModule(ctx context.Context, rootDir, modulesDir, sourceAddr string, reg *registry.Client, hooks ModuleInstallHooks) tfdiags.Diagnostics { var diags tfdiags.Diagnostics // The way this function works is pretty ugly, but we accept it because @@ -144,7 +145,7 @@ func DirFromModule(rootDir, modulesDir, sourceAddr string, reg *registry.Client, Wrapped: hooks, } fetcher := getmodules.NewPackageFetcher() - _, instDiags := inst.installDescendentModules(fakeRootModule, rootDir, instManifest, true, wrapHooks, fetcher) + _, instDiags := inst.installDescendentModules(ctx, fakeRootModule, rootDir, instManifest, true, wrapHooks, fetcher) diags = append(diags, instDiags...) if instDiags.HasErrors() { return diags diff --git a/internal/initwd/from_module_test.go b/internal/initwd/from_module_test.go index 8265f0e3a..a28ad2a7d 100644 --- a/internal/initwd/from_module_test.go +++ b/internal/initwd/from_module_test.go @@ -1,6 +1,7 @@ package initwd import ( + "context" "io/ioutil" "os" "path/filepath" @@ -38,7 +39,7 @@ func TestDirFromModule_registry(t *testing.T) { hooks := &testInstallHooks{} reg := registry.NewClient(nil, nil) - diags := DirFromModule(dir, modsDir, "hashicorp/module-installer-acctest/aws//examples/main", reg, hooks) + diags := DirFromModule(context.Background(), dir, modsDir, "hashicorp/module-installer-acctest/aws//examples/main", reg, hooks) assertNoDiagnostics(t, diags) v := version.Must(version.NewVersion("0.0.2")) @@ -154,7 +155,7 @@ func TestDirFromModule_submodules(t *testing.T) { } modInstallDir := filepath.Join(dir, ".terraform/modules") - diags := DirFromModule(dir, modInstallDir, fromModuleDir, nil, hooks) + diags := DirFromModule(context.Background(), dir, modInstallDir, fromModuleDir, nil, hooks) assertNoDiagnostics(t, diags) wantCalls := []testInstallHookCall{ { @@ -248,7 +249,7 @@ func TestDirFromModule_rel_submodules(t *testing.T) { modInstallDir := ".terraform/modules" sourceDir := "../local-modules" - diags := DirFromModule(".", modInstallDir, sourceDir, nil, hooks) + diags := DirFromModule(context.Background(), ".", modInstallDir, sourceDir, nil, hooks) assertNoDiagnostics(t, diags) wantCalls := []testInstallHookCall{ { diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go index 83f4ed2bb..c5355ee96 100644 --- a/internal/initwd/module_install.go +++ b/internal/initwd/module_install.go @@ -1,6 +1,8 @@ package initwd import ( + "context" + "errors" "fmt" "log" "os" @@ -75,7 +77,7 @@ func NewModuleInstaller(modsDir string, reg *registry.Client) *ModuleInstaller { // If successful (the returned diagnostics contains no errors) then the // first return value is the early configuration tree that was constructed by // the installation process. -func (i *ModuleInstaller) InstallModules(rootDir string, upgrade bool, hooks ModuleInstallHooks) (*earlyconfig.Config, tfdiags.Diagnostics) { +func (i *ModuleInstaller) InstallModules(ctx context.Context, rootDir string, upgrade bool, hooks ModuleInstallHooks) (*earlyconfig.Config, tfdiags.Diagnostics) { log.Printf("[TRACE] ModuleInstaller: installing child modules for %s into %s", rootDir, i.modsDir) rootMod, diags := earlyconfig.LoadModule(rootDir) @@ -94,13 +96,13 @@ func (i *ModuleInstaller) InstallModules(rootDir string, upgrade bool, hooks Mod } fetcher := getmodules.NewPackageFetcher() - cfg, instDiags := i.installDescendentModules(rootMod, rootDir, manifest, upgrade, hooks, fetcher) + cfg, instDiags := i.installDescendentModules(ctx, rootMod, rootDir, manifest, upgrade, hooks, fetcher) diags = append(diags, instDiags...) return cfg, diags } -func (i *ModuleInstaller) installDescendentModules(rootMod *tfconfig.Module, rootDir string, manifest modsdir.Manifest, upgrade bool, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*earlyconfig.Config, tfdiags.Diagnostics) { +func (i *ModuleInstaller) installDescendentModules(ctx context.Context, rootMod *tfconfig.Module, rootDir string, manifest modsdir.Manifest, upgrade bool, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*earlyconfig.Config, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics if hooks == nil { @@ -209,13 +211,13 @@ func (i *ModuleInstaller) installDescendentModules(rootMod *tfconfig.Module, roo case addrs.ModuleSourceRegistry: log.Printf("[TRACE] ModuleInstaller: %s is a registry module at %s", key, addr.String()) - mod, v, mDiags := i.installRegistryModule(req, key, instPath, addr, manifest, hooks, fetcher) + mod, v, mDiags := i.installRegistryModule(ctx, req, key, instPath, addr, manifest, hooks, fetcher) diags = append(diags, mDiags...) return mod, v, diags case addrs.ModuleSourceRemote: log.Printf("[TRACE] ModuleInstaller: %s address %q will be handled by go-getter", key, addr.String()) - mod, mDiags := i.installGoGetterModule(req, key, instPath, manifest, hooks, fetcher) + mod, mDiags := i.installGoGetterModule(ctx, req, key, instPath, manifest, hooks, fetcher) diags = append(diags, mDiags...) return mod, nil, diags @@ -301,7 +303,7 @@ func (i *ModuleInstaller) installLocalModule(req *earlyconfig.ModuleRequest, key return mod, diags } -func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, key string, instPath string, addr addrs.ModuleSourceRegistry, manifest modsdir.Manifest, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*tfconfig.Module, *version.Version, tfdiags.Diagnostics) { +func (i *ModuleInstaller) installRegistryModule(ctx context.Context, req *earlyconfig.ModuleRequest, key string, instPath string, addr addrs.ModuleSourceRegistry, manifest modsdir.Manifest, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*tfconfig.Module, *version.Version, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics hostname := addr.PackageAddr.Host @@ -324,7 +326,7 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, } else { var err error log.Printf("[DEBUG] %s listing available versions of %s at %s", key, addr, hostname) - resp, err = reg.ModuleVersions(regsrcAddr) + resp, err = reg.ModuleVersions(ctx, regsrcAddr) if err != nil { if registry.IsModuleNotFound(err) { diags = diags.Append(tfdiags.Sourceless( @@ -332,6 +334,12 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, "Module not found", fmt.Sprintf("Module %q (from %s:%d) cannot be found in the module registry at %s.", req.Name, req.CallPos.Filename, req.CallPos.Line, hostname), )) + } else if errors.Is(err, context.Canceled) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Module installation was interrupted", + fmt.Sprintf("Received interrupt signal while retrieving available versions for module %q.", req.Name), + )) } else { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -423,7 +431,7 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, // first check the cache for the download URL moduleAddr := moduleVersion{module: packageAddr, version: latestMatch.String()} if _, exists := i.registryPackageSources[moduleAddr]; !exists { - realAddrRaw, err := reg.ModuleLocation(regsrcAddr, latestMatch.String()) + realAddrRaw, err := reg.ModuleLocation(ctx, regsrcAddr, latestMatch.String()) if err != nil { log.Printf("[ERROR] %s from %s %s: %s", key, addr, latestMatch, err) diags = diags.Append(tfdiags.Sourceless( @@ -463,7 +471,15 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, log.Printf("[TRACE] ModuleInstaller: %s %s %s is available at %q", key, packageAddr, latestMatch, dlAddr.PackageAddr) - err := fetcher.FetchPackage(instPath, dlAddr.PackageAddr.String()) + err := fetcher.FetchPackage(ctx, instPath, dlAddr.PackageAddr.String()) + if errors.Is(err, context.Canceled) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Module download was interrupted", + fmt.Sprintf("Interrupt signal received when downloading module %s.", addr), + )) + return nil, nil, diags + } 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 @@ -519,7 +535,7 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, return mod, latestMatch, diags } -func (i *ModuleInstaller) installGoGetterModule(req *earlyconfig.ModuleRequest, key string, instPath string, manifest modsdir.Manifest, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*tfconfig.Module, tfdiags.Diagnostics) { +func (i *ModuleInstaller) installGoGetterModule(ctx context.Context, req *earlyconfig.ModuleRequest, key string, instPath string, manifest modsdir.Manifest, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*tfconfig.Module, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics // Report up to the caller that we're about to start downloading. @@ -536,7 +552,7 @@ func (i *ModuleInstaller) installGoGetterModule(req *earlyconfig.ModuleRequest, return nil, diags } - err := fetcher.FetchPackage(instPath, packageAddr.String()) + err := fetcher.FetchPackage(ctx, instPath, packageAddr.String()) if err != nil { // go-getter generates a poor error for an invalid relative path, so // we'll detect that case and generate a better one. diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index 223ca1ca7..534ef5840 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -2,6 +2,7 @@ package initwd import ( "bytes" + "context" "flag" "fmt" "io/ioutil" @@ -39,7 +40,7 @@ func TestModuleInstaller(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) assertNoDiagnostics(t, diags) wantCalls := []testInstallHookCall{ @@ -100,7 +101,7 @@ func TestModuleInstaller_error(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) if !diags.HasErrors() { t.Fatal("expected error") @@ -135,7 +136,7 @@ func TestModuleInstaller_packageEscapeError(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) if !diags.HasErrors() { t.Fatal("expected error") @@ -170,7 +171,7 @@ func TestModuleInstaller_explicitPackageBoundary(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) if diags.HasErrors() { t.Fatalf("unexpected errors\n%s", diags.Err().Error()) @@ -186,7 +187,7 @@ func TestModuleInstaller_invalid_version_constraint_error(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) if !diags.HasErrors() { t.Fatal("expected error") @@ -204,7 +205,7 @@ func TestModuleInstaller_invalidVersionConstraintGetter(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) if !diags.HasErrors() { t.Fatal("expected error") @@ -222,7 +223,7 @@ func TestModuleInstaller_invalidVersionConstraintLocal(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) if !diags.HasErrors() { t.Fatal("expected error") @@ -240,7 +241,7 @@ func TestModuleInstaller_symlink(t *testing.T) { modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, nil) - _, diags := inst.InstallModules(".", false, hooks) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) assertNoDiagnostics(t, diags) wantCalls := []testInstallHookCall{ @@ -313,7 +314,7 @@ func TestLoaderInstallModules_registry(t *testing.T) { hooks := &testInstallHooks{} modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, registry.NewClient(nil, nil)) - _, diags := inst.InstallModules(dir, false, hooks) + _, diags := inst.InstallModules(context.Background(), dir, false, hooks) assertNoDiagnostics(t, diags) v := version.Must(version.NewVersion("0.0.1")) @@ -473,7 +474,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { hooks := &testInstallHooks{} modulesDir := filepath.Join(dir, ".terraform/modules") inst := NewModuleInstaller(modulesDir, registry.NewClient(nil, nil)) - _, diags := inst.InstallModules(dir, false, hooks) + _, diags := inst.InstallModules(context.Background(), dir, false, hooks) assertNoDiagnostics(t, diags) wantCalls := []testInstallHookCall{ diff --git a/internal/initwd/testing.go b/internal/initwd/testing.go index 320540daa..406718159 100644 --- a/internal/initwd/testing.go +++ b/internal/initwd/testing.go @@ -1,6 +1,7 @@ package initwd import ( + "context" "testing" "github.com/hashicorp/terraform/internal/configs" @@ -35,7 +36,7 @@ func LoadConfigForTests(t *testing.T, rootDir string) (*configs.Config, *configl loader, cleanup := configload.NewLoaderForTests(t) inst := NewModuleInstaller(loader.ModulesDir(), registry.NewClient(nil, nil)) - _, moreDiags := inst.InstallModules(rootDir, true, ModuleInstallHooksImpl{}) + _, moreDiags := inst.InstallModules(context.Background(), rootDir, true, ModuleInstallHooksImpl{}) diags = diags.Append(moreDiags) if diags.HasErrors() { cleanup() diff --git a/internal/refactoring/move_validate_test.go b/internal/refactoring/move_validate_test.go index 49f8acd86..986c1d5db 100644 --- a/internal/refactoring/move_validate_test.go +++ b/internal/refactoring/move_validate_test.go @@ -1,6 +1,7 @@ package refactoring import ( + "context" "strings" "testing" @@ -419,7 +420,7 @@ func loadRefactoringFixture(t *testing.T, dir string) (*configs.Config, instance defer cleanup() inst := initwd.NewModuleInstaller(loader.ModulesDir(), registry.NewClient(nil, nil)) - _, instDiags := inst.InstallModules(dir, true, initwd.ModuleInstallHooksImpl{}) + _, instDiags := inst.InstallModules(context.Background(), dir, true, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) } diff --git a/internal/registry/client.go b/internal/registry/client.go index 495048486..0204674bb 100644 --- a/internal/registry/client.go +++ b/internal/registry/client.go @@ -1,6 +1,7 @@ package registry import ( + "context" "encoding/json" "fmt" "io/ioutil" @@ -109,7 +110,7 @@ func (c *Client) Discover(host svchost.Hostname, serviceID string) (*url.URL, er } // ModuleVersions queries the registry for a module, and returns the available versions. -func (c *Client) ModuleVersions(module *regsrc.Module) (*response.ModuleVersions, error) { +func (c *Client) ModuleVersions(ctx context.Context, module *regsrc.Module) (*response.ModuleVersions, error) { host, err := module.SvcHost() if err != nil { return nil, err @@ -133,6 +134,7 @@ func (c *Client) ModuleVersions(module *regsrc.Module) (*response.ModuleVersions if err != nil { return nil, err } + req = req.WithContext(ctx) c.addRequestCreds(host, req.Request) req.Header.Set(xTerraformVersion, tfVersion) @@ -182,7 +184,7 @@ func (c *Client) addRequestCreds(host svchost.Hostname, req *http.Request) { // ModuleLocation find the download location for a specific version module. // This returns a string, because the final location may contain special go-getter syntax. -func (c *Client) ModuleLocation(module *regsrc.Module, version string) (string, error) { +func (c *Client) ModuleLocation(ctx context.Context, module *regsrc.Module, version string) (string, error) { host, err := module.SvcHost() if err != nil { return "", err @@ -211,6 +213,8 @@ func (c *Client) ModuleLocation(module *regsrc.Module, version string) (string, return "", err } + req = req.WithContext(ctx) + c.addRequestCreds(host, req.Request) req.Header.Set(xTerraformVersion, tfVersion) diff --git a/internal/registry/client_test.go b/internal/registry/client_test.go index 765c3b001..da3055110 100644 --- a/internal/registry/client_test.go +++ b/internal/registry/client_test.go @@ -103,7 +103,7 @@ func TestLookupModuleVersions(t *testing.T) { t.Fatal(err) } - resp, err := client.ModuleVersions(modsrc) + resp, err := client.ModuleVersions(context.Background(), modsrc) if err != nil { t.Fatal(err) } @@ -143,7 +143,7 @@ func TestInvalidRegistry(t *testing.T) { t.Fatal(err) } - if _, err := client.ModuleVersions(modsrc); err == nil { + if _, err := client.ModuleVersions(context.Background(), modsrc); err == nil { t.Fatal("expected error") } } @@ -160,11 +160,11 @@ func TestRegistryAuth(t *testing.T) { t.Fatal(err) } - _, err = client.ModuleVersions(mod) + _, err = client.ModuleVersions(context.Background(), mod) if err != nil { t.Fatal(err) } - _, err = client.ModuleLocation(mod, "1.0.0") + _, err = client.ModuleLocation(context.Background(), mod, "1.0.0") if err != nil { t.Fatal(err) } @@ -173,11 +173,11 @@ func TestRegistryAuth(t *testing.T) { client.services.SetCredentialsSource(nil) // both should fail without auth - _, err = client.ModuleVersions(mod) + _, err = client.ModuleVersions(context.Background(), mod) if err == nil { t.Fatal("expected error") } - _, err = client.ModuleLocation(mod, "1.0.0") + _, err = client.ModuleLocation(context.Background(), mod, "1.0.0") if err == nil { t.Fatal("expected error") } @@ -195,7 +195,7 @@ func TestLookupModuleLocationRelative(t *testing.T) { t.Fatal(err) } - got, err := client.ModuleLocation(mod, "0.2.0") + got, err := client.ModuleLocation(context.Background(), mod, "0.2.0") if err != nil { t.Fatal(err) } @@ -224,7 +224,7 @@ func TestAccLookupModuleVersions(t *testing.T) { } s := NewClient(regDisco, nil) - resp, err := s.ModuleVersions(modsrc) + resp, err := s.ModuleVersions(context.Background(), modsrc) if err != nil { t.Fatal(err) } @@ -277,7 +277,7 @@ func TestLookupLookupModuleError(t *testing.T) { return oldCheck(ctx, resp, err) } - _, err = client.ModuleLocation(mod, "0.2.0") + _, err = client.ModuleLocation(context.Background(), mod, "0.2.0") if err == nil { t.Fatal("expected error") } @@ -299,7 +299,7 @@ func TestLookupModuleRetryError(t *testing.T) { if err != nil { t.Fatal(err) } - resp, err := client.ModuleVersions(modsrc) + resp, err := client.ModuleVersions(context.Background(), modsrc) if err == nil { t.Fatal("expected requests to exceed retry", err) } @@ -328,7 +328,7 @@ func TestLookupModuleNoRetryError(t *testing.T) { if err != nil { t.Fatal(err) } - resp, err := client.ModuleVersions(modsrc) + resp, err := client.ModuleVersions(context.Background(), modsrc) if err == nil { t.Fatal("expected request to fail", err) } @@ -354,7 +354,7 @@ func TestLookupModuleNetworkError(t *testing.T) { if err != nil { t.Fatal(err) } - resp, err := client.ModuleVersions(modsrc) + resp, err := client.ModuleVersions(context.Background(), modsrc) if err == nil { t.Fatal("expected request to fail", err) } diff --git a/internal/terraform/terraform_test.go b/internal/terraform/terraform_test.go index 7eccb51c2..51c1579ec 100644 --- a/internal/terraform/terraform_test.go +++ b/internal/terraform/terraform_test.go @@ -1,6 +1,7 @@ package terraform import ( + "context" "flag" "io" "io/ioutil" @@ -61,7 +62,7 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. inst := initwd.NewModuleInstaller(loader.ModulesDir(), registry.NewClient(nil, nil)) - _, instDiags := inst.InstallModules(dir, true, initwd.ModuleInstallHooksImpl{}) + _, instDiags := inst.InstallModules(context.Background(), dir, true, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) } @@ -119,7 +120,7 @@ func testModuleInline(t *testing.T, sources map[string]string) *configs.Config { // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. inst := initwd.NewModuleInstaller(loader.ModulesDir(), registry.NewClient(nil, nil)) - _, instDiags := inst.InstallModules(cfgPath, true, initwd.ModuleInstallHooksImpl{}) + _, instDiags := inst.InstallModules(context.Background(), cfgPath, true, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) }