diff --git a/internal/addrs/module_package.go b/internal/addrs/module_package.go index e21ed6fc2..dc5a6621c 100644 --- a/internal/addrs/module_package.go +++ b/internal/addrs/module_package.go @@ -1,5 +1,11 @@ package addrs +import ( + "strings" + + svchost "github.com/hashicorp/terraform-svchost" +) + // A ModulePackage represents a physical location where Terraform can retrieve // a module package, which is an archive, repository, or other similar // container which delivers the source code for one or more Terraform modules. @@ -28,3 +34,56 @@ type ModulePackage string func (p ModulePackage) String() string { return string(p) } + +// A ModuleRegistryPackage is an extra indirection over a ModulePackage where +// we use a module registry to translate a more symbolic address (and +// associated version constraint given out of band) into a physical source +// location. +// +// ModuleRegistryPackage is distinct from ModulePackage because they have +// disjoint use-cases: registry package addresses are only used to query a +// registry in order to find a real module package address. These being +// distinct is intended to help future maintainers more easily follow the +// series of steps in the module installer, with the help of the type checker. +type ModuleRegistryPackage struct { + Host svchost.Hostname + Namespace string + Name string + TargetSystem string +} + +func (s ModuleRegistryPackage) String() string { + var buf strings.Builder + // Note: we're using the "display" form of the hostname here because + // for our service hostnames "for display" means something different: + // it means to render non-ASCII characters directly as Unicode + // characters, rather than using the "punycode" representation we + // use for internal processing, and so the "display" representation + // is actually what users would write in their configurations. + return s.Host.ForDisplay() + "/" + s.ForRegistryProtocol() + return buf.String() +} + +func (s ModuleRegistryPackage) ForDisplay() string { + if s.Host == DefaultModuleRegistryHost { + return s.ForRegistryProtocol() + } + return s.Host.ForDisplay() + "/" + s.ForRegistryProtocol() +} + +// ForRegistryProtocol returns a string representation of just the namespace, +// name, and target system portions of the address, always omitting the +// registry hostname and the subdirectory portion, if any. +// +// This is primarily intended for generating addresses to send to the +// registry in question via the registry protocol, since the protocol +// skips sending the registry its own hostname as part of identifiers. +func (s ModuleRegistryPackage) ForRegistryProtocol() string { + var buf strings.Builder + buf.WriteString(s.Namespace) + buf.WriteByte('/') + buf.WriteString(s.Name) + buf.WriteByte('/') + buf.WriteString(s.TargetSystem) + return buf.String() +} diff --git a/internal/addrs/module_source.go b/internal/addrs/module_source.go index 0f2c4aff2..bad32ba38 100644 --- a/internal/addrs/module_source.go +++ b/internal/addrs/module_source.go @@ -171,10 +171,11 @@ func (s ModuleSourceLocal) ForDisplay() string { // a concrete ModuleSourceRemote that Terraform will then download and // install. type ModuleSourceRegistry struct { - Host svchost.Hostname - Namespace string - Name string - TargetSystem string + // PackageAddr is the registry package that the target module belongs to. + // The module installer must translate this into a ModuleSourceRemote + // using the registry API and then take that underlying address's + // PackageAddr in order to find the actual package location. + PackageAddr ModuleRegistryPackage // If Subdir is non-empty then it represents a sub-directory within the // remote package that the registry address eventually resolves to. @@ -233,7 +234,9 @@ func parseModuleSourceRegistry(raw string) (ModuleSourceRegistry, error) { } ret := ModuleSourceRegistry{ - Host: host, + PackageAddr: ModuleRegistryPackage{ + Host: host, + }, Subdir: subDir, } @@ -242,7 +245,7 @@ func parseModuleSourceRegistry(raw string) (ModuleSourceRegistry, error) { return ret, fmt.Errorf("can't use %q as a module registry host, because it's reserved for installing directly from version control repositories", host) } - if ret.Namespace, err = parseModuleRegistryName(parts[0]); err != nil { + if ret.PackageAddr.Namespace, err = parseModuleRegistryName(parts[0]); err != nil { if strings.Contains(parts[0], ".") { // Seems like the user omitted one of the latter components in // an address with an explicit hostname. @@ -250,10 +253,10 @@ func parseModuleSourceRegistry(raw string) (ModuleSourceRegistry, error) { } return ret, fmt.Errorf("invalid namespace %q: %s", parts[0], err) } - if ret.Name, err = parseModuleRegistryName(parts[1]); err != nil { + if ret.PackageAddr.Name, err = parseModuleRegistryName(parts[1]); err != nil { return ret, fmt.Errorf("invalid module name %q: %s", parts[1], err) } - if ret.TargetSystem, err = parseModuleRegistryTargetSystem(parts[2]); err != nil { + if ret.PackageAddr.TargetSystem, err = parseModuleRegistryTargetSystem(parts[2]); err != nil { if strings.Contains(parts[2], "?") { // The user was trying to include a query string, probably? return ret, fmt.Errorf("module registry addresses may not include a query string portion") @@ -314,52 +317,17 @@ func parseModuleRegistryTargetSystem(given string) (string, error) { func (s ModuleSourceRegistry) moduleSource() {} func (s ModuleSourceRegistry) String() string { - var buf strings.Builder - // Note: we're using the "display" form of the hostname here because - // for our service hostnames "for display" means something different: - // it means to render non-ASCII characters directly as Unicode - // characters, rather than using the "punycode" representation we - // use for internal processing, and so the "display" representation - // is actually what users would write in their configurations. - buf.WriteString(s.Host.ForDisplay()) - buf.WriteByte('/') - buf.WriteString(s.ForRegistryProtocol()) if s.Subdir != "" { - buf.WriteString("//") - buf.WriteString(s.Subdir) + return s.PackageAddr.String() + "//" + s.Subdir } - return buf.String() + return s.PackageAddr.String() } func (s ModuleSourceRegistry) ForDisplay() string { - var buf strings.Builder - if s.Host != DefaultModuleRegistryHost { - buf.WriteString(s.Host.ForDisplay()) - buf.WriteByte('/') - } - buf.WriteString(s.ForRegistryProtocol()) if s.Subdir != "" { - buf.WriteString("//") - buf.WriteString(s.Subdir) + return s.PackageAddr.ForDisplay() + "//" + s.Subdir } - return buf.String() -} - -// ForRegistryProtocol returns a string representation of just the namespace, -// name, and target system portions of the address, always omitting the -// registry hostname and the subdirectory portion, if any. -// -// This is primarily intended for generating addresses to send to the -// registry in question via the registry protocol, since the protocol -// skips sending the registry its own hostname as part of identifiers. -func (s ModuleSourceRegistry) ForRegistryProtocol() string { - var buf strings.Builder - buf.WriteString(s.Namespace) - buf.WriteByte('/') - buf.WriteString(s.Name) - buf.WriteByte('/') - buf.WriteString(s.TargetSystem) - return buf.String() + return s.PackageAddr.ForDisplay() } // ModuleSourceRemote is a ModuleSource representing a remote location from diff --git a/internal/addrs/module_source_test.go b/internal/addrs/module_source_test.go index 6303ff20c..480ca50fb 100644 --- a/internal/addrs/module_source_test.go +++ b/internal/addrs/module_source_test.go @@ -59,21 +59,25 @@ func TestParseModuleSource(t *testing.T) { "main registry implied": { input: "hashicorp/subnets/cidr", want: ModuleSourceRegistry{ - Host: svchost.Hostname("registry.terraform.io"), - Namespace: "hashicorp", - Name: "subnets", - TargetSystem: "cidr", - Subdir: "", + PackageAddr: ModuleRegistryPackage{ + Host: svchost.Hostname("registry.terraform.io"), + Namespace: "hashicorp", + Name: "subnets", + TargetSystem: "cidr", + }, + Subdir: "", }, }, "main registry implied, subdir": { input: "hashicorp/subnets/cidr//examples/foo", want: ModuleSourceRegistry{ - Host: svchost.Hostname("registry.terraform.io"), - Namespace: "hashicorp", - Name: "subnets", - TargetSystem: "cidr", - Subdir: "examples/foo", + PackageAddr: ModuleRegistryPackage{ + Host: svchost.Hostname("registry.terraform.io"), + Namespace: "hashicorp", + Name: "subnets", + TargetSystem: "cidr", + }, + Subdir: "examples/foo", }, }, "main registry implied, escaping subdir": { @@ -88,21 +92,25 @@ func TestParseModuleSource(t *testing.T) { "custom registry": { input: "example.com/awesomecorp/network/happycloud", want: ModuleSourceRegistry{ - Host: svchost.Hostname("example.com"), - Namespace: "awesomecorp", - Name: "network", - TargetSystem: "happycloud", - Subdir: "", + PackageAddr: ModuleRegistryPackage{ + Host: svchost.Hostname("example.com"), + Namespace: "awesomecorp", + Name: "network", + TargetSystem: "happycloud", + }, + Subdir: "", }, }, "custom registry, subdir": { input: "example.com/awesomecorp/network/happycloud//examples/foo", want: ModuleSourceRegistry{ - Host: svchost.Hostname("example.com"), - Namespace: "awesomecorp", - Name: "network", - TargetSystem: "happycloud", - Subdir: "examples/foo", + PackageAddr: ModuleRegistryPackage{ + Host: svchost.Hostname("example.com"), + Namespace: "awesomecorp", + Name: "network", + TargetSystem: "happycloud", + }, + Subdir: "examples/foo", }, }, @@ -536,7 +544,7 @@ func TestParseModuleSourceRegistry(t *testing.T) { if got, want := addr.ForDisplay(), test.wantForDisplay; got != want { t.Errorf("wrong ForDisplay() result\ngot: %s\nwant: %s", got, want) } - if got, want := addr.ForRegistryProtocol(), test.wantForProtocol; got != want { + if got, want := addr.PackageAddr.ForRegistryProtocol(), test.wantForProtocol; got != want { t.Errorf("wrong ForRegistryProtocol() result\ngot: %s\nwant: %s", got, want) } }) diff --git a/internal/configs/module_call_test.go b/internal/configs/module_call_test.go index eaf923739..9c607f8e3 100644 --- a/internal/configs/module_call_test.go +++ b/internal/configs/module_call_test.go @@ -45,10 +45,12 @@ func TestLoadModuleCall(t *testing.T) { { Name: "bar", SourceAddr: addrs.ModuleSourceRegistry{ - Host: addrs.DefaultModuleRegistryHost, - Namespace: "hashicorp", - Name: "bar", - TargetSystem: "aws", + PackageAddr: addrs.ModuleRegistryPackage{ + Host: addrs.DefaultModuleRegistryHost, + Namespace: "hashicorp", + Name: "bar", + TargetSystem: "aws", + }, }, SourceAddrRaw: "hashicorp/bar/aws", SourceSet: true, diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go index b640ae93f..83f4ed2bb 100644 --- a/internal/initwd/module_install.go +++ b/internal/initwd/module_install.go @@ -26,24 +26,24 @@ type ModuleInstaller struct { // The keys in moduleVersions are resolved and trimmed registry source // addresses and the values are the registry response. - moduleVersions map[string]*response.ModuleVersions + registryPackageVersions map[addrs.ModuleRegistryPackage]*response.ModuleVersions // The keys in moduleVersionsUrl are the moduleVersion struct below and // addresses and the values are underlying remote source addresses. - moduleVersionsUrl map[moduleVersion]addrs.ModuleSourceRemote + registryPackageSources map[moduleVersion]addrs.ModuleSourceRemote } type moduleVersion struct { - module string + module addrs.ModuleRegistryPackage version string } func NewModuleInstaller(modsDir string, reg *registry.Client) *ModuleInstaller { return &ModuleInstaller{ - modsDir: modsDir, - reg: reg, - moduleVersions: make(map[string]*response.ModuleVersions), - moduleVersionsUrl: make(map[moduleVersion]addrs.ModuleSourceRemote), + modsDir: modsDir, + reg: reg, + registryPackageVersions: make(map[addrs.ModuleRegistryPackage]*response.ModuleVersions), + registryPackageSources: make(map[moduleVersion]addrs.ModuleSourceRemote), } } @@ -304,7 +304,7 @@ func (i *ModuleInstaller) installLocalModule(req *earlyconfig.ModuleRequest, key 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) { var diags tfdiags.Diagnostics - hostname := addr.Host + hostname := addr.PackageAddr.Host reg := i.reg var resp *response.ModuleVersions var exists bool @@ -312,15 +312,14 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, // A registry entry isn't _really_ a module package, but we'll pretend it's // one for the sake of this reporting by just trimming off any source // directory. - packageAddr := addr // shallow copy - packageAddr.Subdir = "" + packageAddr := addr.PackageAddr // Our registry client is still using the legacy model of addresses, so // we'll shim it here for now. - regsrcAddr := regsrc.ModuleFromModuleSourceAddr(packageAddr) + regsrcAddr := regsrc.ModuleFromRegistryPackageAddr(packageAddr) // check if we've already looked up this module from the registry - if resp, exists = i.moduleVersions[packageAddr.String()]; exists { + if resp, exists = i.registryPackageVersions[packageAddr]; exists { log.Printf("[TRACE] %s using already found available versions of %s at %s", key, addr, hostname) } else { var err error @@ -342,7 +341,7 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, } return nil, nil, diags } - i.moduleVersions[packageAddr.String()] = resp + i.registryPackageVersions[packageAddr] = resp } // The response might contain information about dependencies to allow us @@ -422,8 +421,8 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, // The response to this is a go-getter-style address string. // first check the cache for the download URL - moduleAddr := moduleVersion{module: packageAddr.String(), version: latestMatch.String()} - if _, exists := i.moduleVersionsUrl[moduleAddr]; !exists { + moduleAddr := moduleVersion{module: packageAddr, version: latestMatch.String()} + if _, exists := i.registryPackageSources[moduleAddr]; !exists { realAddrRaw, err := reg.ModuleLocation(regsrcAddr, latestMatch.String()) if err != nil { log.Printf("[ERROR] %s from %s %s: %s", key, addr, latestMatch, err) @@ -449,7 +448,7 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, // its being called from) and we also don't allow recursively pointing // at another registry source for simplicity's sake. case addrs.ModuleSourceRemote: - i.moduleVersionsUrl[moduleAddr] = realAddr + i.registryPackageSources[moduleAddr] = realAddr default: diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -460,7 +459,7 @@ func (i *ModuleInstaller) installRegistryModule(req *earlyconfig.ModuleRequest, } } - dlAddr := i.moduleVersionsUrl[moduleAddr] + dlAddr := i.registryPackageSources[moduleAddr] log.Printf("[TRACE] ModuleInstaller: %s %s %s is available at %q", key, packageAddr, latestMatch, dlAddr.PackageAddr) diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index 25905d90e..223ca1ca7 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -14,6 +14,8 @@ import ( "github.com/go-test/deep" "github.com/google/go-cmp/cmp" version "github.com/hashicorp/go-version" + svchost "github.com/hashicorp/terraform-svchost" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/copy" @@ -403,11 +405,17 @@ func TestLoaderInstallModules_registry(t *testing.T) { } //check that the registry reponses were cached - if _, ok := inst.moduleVersions["registry.terraform.io/hashicorp/module-installer-acctest/aws"]; !ok { - t.Errorf("module versions cache was not populated\ngot: %s\nwant: key hashicorp/module-installer-acctest/aws", spew.Sdump(inst.moduleVersions)) + packageAddr := addrs.ModuleRegistryPackage{ + Host: svchost.Hostname("registry.terraform.io"), + Namespace: "hashicorp", + Name: "module-installer-acctest", + TargetSystem: "aws", } - if _, ok := inst.moduleVersionsUrl[moduleVersion{module: "registry.terraform.io/hashicorp/module-installer-acctest/aws", version: "0.0.1"}]; !ok { - t.Errorf("module download url cache was not populated\ngot: %s", spew.Sdump(inst.moduleVersionsUrl)) + if _, ok := inst.registryPackageVersions[packageAddr]; !ok { + t.Errorf("module versions cache was not populated\ngot: %s\nwant: key hashicorp/module-installer-acctest/aws", spew.Sdump(inst.registryPackageVersions)) + } + if _, ok := inst.registryPackageSources[moduleVersion{module: packageAddr, version: "0.0.1"}]; !ok { + t.Errorf("module download url cache was not populated\ngot: %s", spew.Sdump(inst.registryPackageSources)) } loader, err := configload.NewLoader(&configload.Config{ diff --git a/internal/registry/regsrc/module.go b/internal/registry/regsrc/module.go index 9ce6b282a..9f9999d34 100644 --- a/internal/registry/regsrc/module.go +++ b/internal/registry/regsrc/module.go @@ -105,12 +105,27 @@ func NewModule(host, namespace, name, provider, submodule string) (*Module, erro // use addrs.ModuleSourceRegistry instead, and then package regsrc can be // removed altogether. func ModuleFromModuleSourceAddr(addr addrs.ModuleSourceRegistry) *Module { + ret := ModuleFromRegistryPackageAddr(addr.PackageAddr) + ret.RawSubmodule = addr.Subdir + return ret +} + +// ModuleFromRegistryPackageAddr is similar to ModuleFromModuleSourceAddr, but +// it works with just the isolated registry package address, and not the +// full source address. +// +// The practical implication of that is that RawSubmodule will always be +// the empty string in results from this function, because "Submodule" maps +// to "Subdir" and that's a module source address concept, not a module +// package concept. In practice this typically doesn't matter because the +// registry client ignores the RawSubmodule field anyway; that's a concern +// for the higher-level module installer to deal with. +func ModuleFromRegistryPackageAddr(addr addrs.ModuleRegistryPackage) *Module { return &Module{ RawHost: NewFriendlyHost(addr.Host.String()), RawNamespace: addr.Namespace, RawName: addr.Name, RawProvider: addr.TargetSystem, // this field was never actually enforced to be a provider address, so now has a more general name - RawSubmodule: addr.Subdir, } }