From 4045a6e5d0bae04762566ce176535e5354be41df Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Tue, 7 Jan 2020 15:03:23 -0500 Subject: [PATCH] initwd: cache registry responses for module versions and download URL (#23727) * initwd: cache registry responses for module versions and download URL Closes #23544 --- internal/initwd/module_install.go | 86 ++++++++++++++++++-------- internal/initwd/module_install_test.go | 8 +++ 2 files changed, 67 insertions(+), 27 deletions(-) diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go index 531310ab8d..cbfd8e98cb 100644 --- a/internal/initwd/module_install.go +++ b/internal/initwd/module_install.go @@ -14,18 +14,34 @@ import ( "github.com/hashicorp/terraform/internal/modsdir" "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/registry/regsrc" + "github.com/hashicorp/terraform/registry/response" "github.com/hashicorp/terraform/tfdiags" ) type ModuleInstaller struct { modsDir string reg *registry.Client + + // The keys in moduleVersions are resolved and trimmed registry source + // addresses and the values are the registry response. + moduleVersions map[string]*response.ModuleVersions + + // The keys in moduleVersionsUrl are the moduleVersion struct below and + // addresses and the values are the download URLs. + moduleVersionsUrl map[moduleVersion]string +} + +type moduleVersion struct { + module string + version string } func NewModuleInstaller(modsDir string, reg *registry.Client) *ModuleInstaller { return &ModuleInstaller{ - modsDir: modsDir, - reg: reg, + modsDir: modsDir, + reg: reg, + moduleVersions: make(map[string]*response.ModuleVersions), + moduleVersionsUrl: make(map[moduleVersion]string), } } @@ -309,24 +325,32 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, } reg := i.reg + var resp *response.ModuleVersions + var exists bool - log.Printf("[DEBUG] %s listing available versions of %s at %s", key, addr, hostname) - resp, err := reg.ModuleVersions(addr) - if err != nil { - if registry.IsModuleNotFound(err) { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "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 { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Error accessing remote module registry", - fmt.Sprintf("Failed to retrieve available versions for module %q (%s:%d) from %s: %s.", req.Name, req.CallPos.Filename, req.CallPos.Line, hostname, err), - )) + // check if we've already looked up this module from the registry + if resp, exists = i.moduleVersions[addr.String()]; exists { + log.Printf("[TRACE] %s using already found available versions of %s at %s", key, addr, hostname) + } else { + log.Printf("[DEBUG] %s listing available versions of %s at %s", key, addr, hostname) + resp, err = reg.ModuleVersions(addr) + if err != nil { + if registry.IsModuleNotFound(err) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "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 { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error accessing remote module registry", + fmt.Sprintf("Failed to retrieve available versions for module %q (%s:%d) from %s: %s.", req.Name, req.CallPos.Filename, req.CallPos.Line, hostname, err), + )) + } + return nil, nil, diags } - return nil, nil, diags + i.moduleVersions[addr.String()] = resp } // The response might contain information about dependencies to allow us @@ -405,17 +429,25 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, // 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.ModuleLocation(addr, latestMatch.String()) - if err != nil { - log.Printf("[ERROR] %s from %s %s: %s", key, addr, latestMatch, err) - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid response from remote module registry", - fmt.Sprintf("The remote registry at %s failed to return a download URL for %s %s.", hostname, addr, latestMatch), - )) - return nil, nil, diags + + // first check the cache for the download URL + moduleAddr := moduleVersion{module: addr.String(), version: latestMatch.String()} + if _, exists := i.moduleVersionsUrl[moduleAddr]; !exists { + url, err := reg.ModuleLocation(addr, latestMatch.String()) + if err != nil { + log.Printf("[ERROR] %s from %s %s: %s", key, addr, latestMatch, err) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid response from remote module registry", + fmt.Sprintf("The remote registry at %s failed to return a download URL for %s %s.", hostname, addr, latestMatch), + )) + return nil, nil, diags + } + i.moduleVersionsUrl[moduleVersion{module: addr.String(), version: latestMatch.String()}] = url } + dlAddr := i.moduleVersionsUrl[moduleAddr] + log.Printf("[TRACE] ModuleInstaller: %s %s %s is available at %q", key, addr, latestMatch, dlAddr) modDir, err := getter.getWithGoGetter(instPath, dlAddr) diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index 4449681690..239014990b 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -327,6 +327,14 @@ func TestLoaderInstallModules_registry(t *testing.T) { return } + //check that the registry reponses were cached + if _, ok := inst.moduleVersions["hashicorp/module-installer-acctest/aws"]; !ok { + t.Fatal("module versions cache was not populated") + } + if _, ok := inst.moduleVersionsUrl[moduleVersion{module: "hashicorp/module-installer-acctest/aws", version: "0.0.1"}]; !ok { + t.Fatal("module download url cache was not populated") + } + loader, err := configload.NewLoader(&configload.Config{ ModulesDir: modulesDir, })