diff --git a/internal/addrs/module_source.go b/internal/addrs/module_source.go index 1b155cc6a6..e2ab072097 100644 --- a/internal/addrs/module_source.go +++ b/internal/addrs/module_source.go @@ -4,17 +4,19 @@ package addrs import ( - "fmt" "path" "strings" tfaddr "github.com/hashicorp/terraform-registry-address" - "github.com/hashicorp/terraform/internal/getmodules" ) // ModuleSource is the general type for all three of the possible module source -// address types. The concrete implementations of this are ModuleSourceLocal, -// ModuleSourceRegistry, and ModuleSourceRemote. +// address types. The concrete implementations of this are [ModuleSourceLocal], +// [ModuleSourceRegistry], and [ModuleSourceRemote]. +// +// The parser for this address type lives in package moduleaddrs, because remote +// module source address parsing depends on go-getter and that's too heavy a +// dependency to impose on everything that imports this package addrs. type ModuleSource interface { // String returns a full representation of the address, including any // additional components that are typically implied by omission in @@ -41,76 +43,6 @@ var _ ModuleSource = ModuleSourceLocal("") var _ ModuleSource = ModuleSourceRegistry{} var _ ModuleSource = ModuleSourceRemote{} -var moduleSourceLocalPrefixes = []string{ - "./", - "../", - ".\\", - "..\\", -} - -// ParseModuleSource parses a module source address as given in the "source" -// argument inside a "module" block in the configuration. -// -// For historical reasons this syntax is a bit overloaded, supporting three -// different address types: -// - Local paths starting with either ./ or ../, which are special because -// Terraform considers them to belong to the same "package" as the caller. -// - Module registry addresses, given as either NAMESPACE/NAME/SYSTEM or -// HOST/NAMESPACE/NAME/SYSTEM, in which case the remote registry serves -// as an indirection over the third address type that follows. -// - Various URL-like and other heuristically-recognized strings which -// we currently delegate to the external library go-getter. -// -// There is some ambiguity between the module registry addresses and go-getter's -// very liberal heuristics and so this particular function will typically treat -// an invalid registry address as some other sort of remote source address -// rather than returning an error. If you know that you're expecting a -// registry address in particular, use ParseModuleSourceRegistry instead, which -// can therefore expose more detailed error messages about registry address -// parsing in particular. -func ParseModuleSource(raw string) (ModuleSource, error) { - if isModuleSourceLocal(raw) { - localAddr, err := parseModuleSourceLocal(raw) - if err != nil { - // This is to make sure we really return a nil ModuleSource in - // this case, rather than an interface containing the zero - // value of ModuleSourceLocal. - return nil, err - } - return localAddr, nil - } - - // For historical reasons, whether an address is a registry - // address is defined only by whether it can be successfully - // parsed as one, and anything else must fall through to be - // parsed as a direct remote source, where go-getter might - // then recognize it as a filesystem path. This is odd - // but matches behavior we've had since Terraform v0.10 which - // existing modules may be relying on. - // (Notice that this means that there's never any path where - // the registry source parse error gets returned to the caller, - // which is annoying but has been true for many releases - // without it posing a serious problem in practice.) - if ret, err := ParseModuleSourceRegistry(raw); err == nil { - return ret, nil - } - - // If we get down here then we treat everything else as a - // remote address. In practice there's very little that - // go-getter doesn't consider invalid input, so even invalid - // nonsense will probably interpreted as _something_ here - // and then fail during installation instead. We can't - // really improve this situation for historical reasons. - remoteAddr, err := parseModuleSourceRemote(raw) - if err != nil { - // This is to make sure we really return a nil ModuleSource in - // this case, rather than an interface containing the zero - // value of ModuleSourceRemote. - return nil, err - } - return remoteAddr, nil -} - // ModuleSourceLocal is a ModuleSource representing a local path reference // from the caller's directory to the callee's directory within the same // module package. @@ -132,53 +64,13 @@ func ParseModuleSource(raw string) (ModuleSource, error) { // ModuleSourceLocal values directly, except in tests where we can ensure // the value meets our assumptions. Use ParseModuleSource instead if the // input string is not hard-coded in the program. +// +// The parser for this address type lives in package moduleaddrs. It doesn't +// really need to because it doesn't have any special dependencies, but the +// remote source address parser needs to live over there and so it's clearer +// to just have all of the parsers live together in that other package. type ModuleSourceLocal string -func parseModuleSourceLocal(raw string) (ModuleSourceLocal, error) { - // As long as we have a suitable prefix (detected by ParseModuleSource) - // there is no failure case for local paths: we just use the "path" - // package's cleaning logic to remove any redundant "./" and "../" - // sequences and any duplicate slashes and accept whatever that - // produces. - - // Although using backslashes (Windows-style) is non-idiomatic, we do - // allow it and just normalize it away, so the rest of Terraform will - // only see the forward-slash form. - if strings.Contains(raw, `\`) { - // Note: We use string replacement rather than filepath.ToSlash - // here because the filepath package behavior varies by current - // platform, but we want to interpret configured paths the same - // across all platforms: these are virtual paths within a module - // package, not physical filesystem paths. - raw = strings.ReplaceAll(raw, `\`, "/") - } - - // Note that we could've historically blocked using "//" in a path here - // in order to avoid confusion with the subdir syntax in remote addresses, - // but we historically just treated that as the same as a single slash - // and so we continue to do that now for compatibility. Clean strips those - // out and reduces them to just a single slash. - clean := path.Clean(raw) - - // However, we do need to keep a single "./" on the front if it isn't - // a "../" path, or else it would be ambigous with the registry address - // syntax. - if !strings.HasPrefix(clean, "../") { - clean = "./" + clean - } - - return ModuleSourceLocal(clean), nil -} - -func isModuleSourceLocal(raw string) bool { - for _, prefix := range moduleSourceLocalPrefixes { - if strings.HasPrefix(raw, prefix) { - return true - } - } - return false -} - func (s ModuleSourceLocal) moduleSource() {} func (s ModuleSourceLocal) String() string { @@ -199,37 +91,17 @@ func (s ModuleSourceLocal) ForDisplay() string { // combination of a ModuleSourceRegistry and a module version number into // a concrete ModuleSourceRemote that Terraform will then download and // install. +// +// The parser for this address type lives in package moduleaddrs. It doesn't +// really need to because it doesn't have any special dependencies, but the +// remote source address parser needs to live over there and so it's clearer +// to just have all of the parsers live together in that other package. type ModuleSourceRegistry tfaddr.Module // DefaultModuleRegistryHost is the hostname used for registry-based module // source addresses that do not have an explicit hostname. const DefaultModuleRegistryHost = tfaddr.DefaultModuleRegistryHost -// ParseModuleSourceRegistry is a variant of ParseModuleSource which only -// accepts module registry addresses, and will reject any other address type. -// -// Use this instead of ParseModuleSource if you know from some other surrounding -// context that an address is intended to be a registry address rather than -// some other address type, which will then allow for better error reporting -// due to the additional information about user intent. -func ParseModuleSourceRegistry(raw string) (ModuleSource, error) { - // Before we delegate to the "real" function we'll just make sure this - // doesn't look like a local source address, so we can return a better - // error message for that situation. - if isModuleSourceLocal(raw) { - return ModuleSourceRegistry{}, fmt.Errorf("can't use local directory %q as a module registry address", raw) - } - - src, err := tfaddr.ParseModuleSource(raw) - if err != nil { - return nil, err - } - return ModuleSourceRegistry{ - Package: src.Package, - Subdir: src.Subdir, - }, nil -} - func (s ModuleSourceRegistry) moduleSource() {} func (s ModuleSourceRegistry) String() string { @@ -252,6 +124,10 @@ func (s ModuleSourceRegistry) ForDisplay() string { // A ModuleSourceRemote can optionally include a "subdirectory" path, which // means that it's selecting a sub-directory of the given package to use as // the entry point into the package. +// +// The parser for this address type lives in package moduleaddrs, because remote +// module source address parsing depends on go-getter and that's too heavy a +// dependency to impose on everything that imports this package addrs. type ModuleSourceRemote struct { // Package is the address of the remote package that the requested // module belongs to. @@ -266,56 +142,6 @@ type ModuleSourceRemote struct { Subdir string } -func parseModuleSourceRemote(raw string) (ModuleSourceRemote, error) { - var subDir string - raw, subDir = getmodules.SplitPackageSubdir(raw) - if strings.HasPrefix(subDir, "../") { - return ModuleSourceRemote{}, fmt.Errorf("subdirectory path %q leads outside of the module package", subDir) - } - - // A remote source address is really just a go-getter address resulting - // from go-getter's "detect" phase, which adds on the prefix specifying - // which protocol it should use and possibly also adjusts the - // protocol-specific part into different syntax. - // - // Note that for historical reasons this can potentially do network - // requests in order to disambiguate certain address types, although - // that's a legacy thing that is only for some specific, less-commonly-used - // address types. Most just do local string manipulation. We should - // aim to remove the network requests over time, if possible. - norm, moreSubDir, err := getmodules.NormalizePackageAddress(raw) - if err != nil { - // We must pass through the returned error directly here because - // the getmodules package has some special error types it uses - // for certain cases where the UI layer might want to include a - // more helpful error message. - return ModuleSourceRemote{}, err - } - - if moreSubDir != "" { - switch { - case subDir != "": - // The detector's own subdir goes first, because the - // subdir we were given is conceptually relative to - // the subdirectory that we just detected. - subDir = path.Join(moreSubDir, subDir) - default: - subDir = path.Clean(moreSubDir) - } - if strings.HasPrefix(subDir, "../") { - // This would suggest a bug in a go-getter detector, but - // we'll catch it anyway to avoid doing something confusing - // downstream. - return ModuleSourceRemote{}, fmt.Errorf("detected subdirectory path %q of %q leads outside of the module package", subDir, norm) - } - } - - return ModuleSourceRemote{ - Package: ModulePackage(norm), - Subdir: subDir, - }, nil -} - func (s ModuleSourceRemote) moduleSource() {} func (s ModuleSourceRemote) String() string { diff --git a/internal/configs/module_call.go b/internal/configs/module_call.go index 31fc31e8a9..a67464f47c 100644 --- a/internal/configs/module_call.go +++ b/internal/configs/module_call.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/getmodules" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" ) // ModuleCall represents a "module" block in a module or file. @@ -80,9 +81,9 @@ func decodeModuleBlock(block *hcl.Block, override bool) (*ModuleCall, hcl.Diagno var addr addrs.ModuleSource var err error if haveVersionArg { - addr, err = addrs.ParseModuleSourceRegistry(mc.SourceAddrRaw) + addr, err = moduleaddrs.ParseModuleSourceRegistry(mc.SourceAddrRaw) } else { - addr, err = addrs.ParseModuleSource(mc.SourceAddrRaw) + addr, err = moduleaddrs.ParseModuleSource(mc.SourceAddrRaw) } mc.SourceAddr = addr if err != nil { diff --git a/internal/configs/module_call_test.go b/internal/configs/module_call_test.go index bd474eca9f..5f64f65506 100644 --- a/internal/configs/module_call_test.go +++ b/internal/configs/module_call_test.go @@ -9,7 +9,9 @@ import ( "github.com/go-test/deep" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" ) func TestLoadModuleCall(t *testing.T) { @@ -181,7 +183,7 @@ func TestModuleSourceAddrEntersNewPackage(t *testing.T) { for _, test := range tests { t.Run(test.Addr, func(t *testing.T) { - addr, err := addrs.ParseModuleSource(test.Addr) + addr, err := moduleaddrs.ParseModuleSource(test.Addr) if err != nil { t.Fatalf("parsing failed for %q: %s", test.Addr, err) } diff --git a/internal/configs/test_file.go b/internal/configs/test_file.go index b902384adf..97306cd270 100644 --- a/internal/configs/test_file.go +++ b/internal/configs/test_file.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/getmodules" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -636,9 +637,9 @@ func decodeTestRunModuleBlock(block *hcl.Block) (*TestRunModuleCall, hcl.Diagnos if !rawDiags.HasErrors() { var err error if haveVersionArg { - module.Source, err = addrs.ParseModuleSourceRegistry(raw) + module.Source, err = moduleaddrs.ParseModuleSourceRegistry(raw) } else { - module.Source, err = addrs.ParseModuleSource(raw) + module.Source, err = moduleaddrs.ParseModuleSource(raw) } if err != nil { // NOTE: We leave mc.SourceAddr as nil for any situation where the diff --git a/internal/getmodules/moduleaddrs/source_parsing.go b/internal/getmodules/moduleaddrs/source_parsing.go new file mode 100644 index 0000000000..77e8992099 --- /dev/null +++ b/internal/getmodules/moduleaddrs/source_parsing.go @@ -0,0 +1,218 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package moduleaddrs + +import ( + "fmt" + "path" + "strings" + + tfaddr "github.com/hashicorp/terraform-registry-address" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/getmodules" +) + +// We have some of the module address parsers in here, rather than in +// package addrs, because right now our remote source address normalization +// is inextricably tied to the external go-getter library, which means any +// package that calls these functions must indirectly depend on go-getter. +// +// Package addrs is imported from almost everywhere, so any dependency it +// has becomes an indirect dependency of everything else. Only a few callers +// actually need to parse module source addresses, so it's pragmatic to have +// just those callers import this package, whereas packages that only need +// to work with addresses that were already parsed -- or don't need to interact +// with module source addresses _at all_ -- can avoid indirectly depending +// on go-getter and all of its various third-party dependencies. + +var moduleSourceLocalPrefixes = []string{ + "./", + "../", + ".\\", + "..\\", +} + +// ParseModuleSource parses a module source address as given in the "source" +// argument inside a "module" block in the configuration. +// +// For historical reasons this syntax is a bit overloaded, supporting three +// different address types: +// - Local paths starting with either ./ or ../, which are special because +// Terraform considers them to belong to the same "package" as the caller. +// - Module registry addresses, given as either NAMESPACE/NAME/SYSTEM or +// HOST/NAMESPACE/NAME/SYSTEM, in which case the remote registry serves +// as an indirection over the third address type that follows. +// - Various URL-like and other heuristically-recognized strings which +// we currently delegate to the external library go-getter. +// +// There is some ambiguity between the module registry addresses and go-getter's +// very liberal heuristics and so this particular function will typically treat +// an invalid registry address as some other sort of remote source address +// rather than returning an error. If you know that you're expecting a +// registry address in particular, use [ParseModuleSourceRegistry] instead, which +// can therefore expose more detailed error messages about registry address +// parsing in particular. +func ParseModuleSource(raw string) (addrs.ModuleSource, error) { + if isModuleSourceLocal(raw) { + localAddr, err := parseModuleSourceLocal(raw) + if err != nil { + // This is to make sure we really return a nil ModuleSource in + // this case, rather than an interface containing the zero + // value of ModuleSourceLocal. + return nil, err + } + return localAddr, nil + } + + // For historical reasons, whether an address is a registry + // address is defined only by whether it can be successfully + // parsed as one, and anything else must fall through to be + // parsed as a direct remote source, where go-getter might + // then recognize it as a filesystem path. This is odd + // but matches behavior we've had since Terraform v0.10 which + // existing modules may be relying on. + // (Notice that this means that there's never any path where + // the registry source parse error gets returned to the caller, + // which is annoying but has been true for many releases + // without it posing a serious problem in practice.) + if ret, err := ParseModuleSourceRegistry(raw); err == nil { + return ret, nil + } + + // If we get down here then we treat everything else as a + // remote address. In practice there's very little that + // go-getter doesn't consider invalid input, so even invalid + // nonsense will probably interpreted as _something_ here + // and then fail during installation instead. We can't + // really improve this situation for historical reasons. + remoteAddr, err := parseModuleSourceRemote(raw) + if err != nil { + // This is to make sure we really return a nil ModuleSource in + // this case, rather than an interface containing the zero + // value of ModuleSourceRemote. + return nil, err + } + return remoteAddr, nil +} + +func parseModuleSourceLocal(raw string) (addrs.ModuleSourceLocal, error) { + // As long as we have a suitable prefix (detected by ParseModuleSource) + // there is no failure case for local paths: we just use the "path" + // package's cleaning logic to remove any redundant "./" and "../" + // sequences and any duplicate slashes and accept whatever that + // produces. + + // Although using backslashes (Windows-style) is non-idiomatic, we do + // allow it and just normalize it away, so the rest of Terraform will + // only see the forward-slash form. + if strings.Contains(raw, `\`) { + // Note: We use string replacement rather than filepath.ToSlash + // here because the filepath package behavior varies by current + // platform, but we want to interpret configured paths the same + // across all platforms: these are virtual paths within a module + // package, not physical filesystem paths. + raw = strings.ReplaceAll(raw, `\`, "/") + } + + // Note that we could've historically blocked using "//" in a path here + // in order to avoid confusion with the subdir syntax in remote addresses, + // but we historically just treated that as the same as a single slash + // and so we continue to do that now for compatibility. Clean strips those + // out and reduces them to just a single slash. + clean := path.Clean(raw) + + // However, we do need to keep a single "./" on the front if it isn't + // a "../" path, or else it would be ambigous with the registry address + // syntax. + if !strings.HasPrefix(clean, "../") { + clean = "./" + clean + } + + return addrs.ModuleSourceLocal(clean), nil +} + +func isModuleSourceLocal(raw string) bool { + for _, prefix := range moduleSourceLocalPrefixes { + if strings.HasPrefix(raw, prefix) { + return true + } + } + return false +} + +// ParseModuleSourceRegistry is a variant of ParseModuleSource which only +// accepts module registry addresses, and will reject any other address type. +// +// Use this instead of ParseModuleSource if you know from some other surrounding +// context that an address is intended to be a registry address rather than +// some other address type, which will then allow for better error reporting +// due to the additional information about user intent. +func ParseModuleSourceRegistry(raw string) (addrs.ModuleSource, error) { + // Before we delegate to the "real" function we'll just make sure this + // doesn't look like a local source address, so we can return a better + // error message for that situation. + if isModuleSourceLocal(raw) { + return addrs.ModuleSourceRegistry{}, fmt.Errorf("can't use local directory %q as a module registry address", raw) + } + + src, err := tfaddr.ParseModuleSource(raw) + if err != nil { + return nil, err + } + return addrs.ModuleSourceRegistry{ + Package: src.Package, + Subdir: src.Subdir, + }, nil +} + +func parseModuleSourceRemote(raw string) (addrs.ModuleSourceRemote, error) { + var subDir string + raw, subDir = getmodules.SplitPackageSubdir(raw) + if strings.HasPrefix(subDir, "../") { + return addrs.ModuleSourceRemote{}, fmt.Errorf("subdirectory path %q leads outside of the module package", subDir) + } + + // A remote source address is really just a go-getter address resulting + // from go-getter's "detect" phase, which adds on the prefix specifying + // which protocol it should use and possibly also adjusts the + // protocol-specific part into different syntax. + // + // Note that for historical reasons this can potentially do network + // requests in order to disambiguate certain address types, although + // that's a legacy thing that is only for some specific, less-commonly-used + // address types. Most just do local string manipulation. We should + // aim to remove the network requests over time, if possible. + norm, moreSubDir, err := getmodules.NormalizePackageAddress(raw) + if err != nil { + // We must pass through the returned error directly here because + // the getmodules package has some special error types it uses + // for certain cases where the UI layer might want to include a + // more helpful error message. + return addrs.ModuleSourceRemote{}, err + } + + if moreSubDir != "" { + switch { + case subDir != "": + // The detector's own subdir goes first, because the + // subdir we were given is conceptually relative to + // the subdirectory that we just detected. + subDir = path.Join(moreSubDir, subDir) + default: + subDir = path.Clean(moreSubDir) + } + if strings.HasPrefix(subDir, "../") { + // This would suggest a bug in a go-getter detector, but + // we'll catch it anyway to avoid doing something confusing + // downstream. + return addrs.ModuleSourceRemote{}, fmt.Errorf("detected subdirectory path %q of %q leads outside of the module package", subDir, norm) + } + } + + return addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage(norm), + Subdir: subDir, + }, nil +} diff --git a/internal/addrs/module_source_test.go b/internal/getmodules/moduleaddrs/source_parsing_test.go similarity index 81% rename from internal/addrs/module_source_test.go rename to internal/getmodules/moduleaddrs/source_parsing_test.go index 933a1d1a93..fae56599d1 100644 --- a/internal/addrs/module_source_test.go +++ b/internal/getmodules/moduleaddrs/source_parsing_test.go @@ -1,68 +1,70 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package addrs +package moduleaddrs import ( "testing" "github.com/google/go-cmp/cmp" svchost "github.com/hashicorp/terraform-svchost" + + "github.com/hashicorp/terraform/internal/addrs" ) func TestParseModuleSource(t *testing.T) { tests := map[string]struct { input string - want ModuleSource + want addrs.ModuleSource wantErr string }{ // Local paths "local in subdirectory": { input: "./child", - want: ModuleSourceLocal("./child"), + want: addrs.ModuleSourceLocal("./child"), }, "local in subdirectory non-normalized": { input: "./nope/../child", - want: ModuleSourceLocal("./child"), + want: addrs.ModuleSourceLocal("./child"), }, "local in sibling directory": { input: "../sibling", - want: ModuleSourceLocal("../sibling"), + want: addrs.ModuleSourceLocal("../sibling"), }, "local in sibling directory non-normalized": { input: "./nope/../../sibling", - want: ModuleSourceLocal("../sibling"), + want: addrs.ModuleSourceLocal("../sibling"), }, "Windows-style local in subdirectory": { input: `.\child`, - want: ModuleSourceLocal("./child"), + want: addrs.ModuleSourceLocal("./child"), }, "Windows-style local in subdirectory non-normalized": { input: `.\nope\..\child`, - want: ModuleSourceLocal("./child"), + want: addrs.ModuleSourceLocal("./child"), }, "Windows-style local in sibling directory": { input: `..\sibling`, - want: ModuleSourceLocal("../sibling"), + want: addrs.ModuleSourceLocal("../sibling"), }, "Windows-style local in sibling directory non-normalized": { input: `.\nope\..\..\sibling`, - want: ModuleSourceLocal("../sibling"), + want: addrs.ModuleSourceLocal("../sibling"), }, "an abominable mix of different slashes": { input: `./nope\nope/why\./please\don't`, - want: ModuleSourceLocal("./nope/nope/why/please/don't"), + want: addrs.ModuleSourceLocal("./nope/nope/why/please/don't"), }, // Registry addresses - // (NOTE: There is another test function TestParseModuleSourceRegistry + // (NOTE: There is another test function TestParseaddrs.ModuleSourceRegistry // which tests this situation more exhaustively, so this is just a // token set of cases to see that we are indeed calling into the // registry address parser when appropriate.) "main registry implied": { input: "hashicorp/subnets/cidr", - want: ModuleSourceRegistry{ - Package: ModuleRegistryPackage{ + want: addrs.ModuleSourceRegistry{ + Package: addrs.ModuleRegistryPackage{ Host: svchost.Hostname("registry.terraform.io"), Namespace: "hashicorp", Name: "subnets", @@ -73,8 +75,8 @@ func TestParseModuleSource(t *testing.T) { }, "main registry implied, subdir": { input: "hashicorp/subnets/cidr//examples/foo", - want: ModuleSourceRegistry{ - Package: ModuleRegistryPackage{ + want: addrs.ModuleSourceRegistry{ + Package: addrs.ModuleRegistryPackage{ Host: svchost.Hostname("registry.terraform.io"), Namespace: "hashicorp", Name: "subnets", @@ -94,8 +96,8 @@ func TestParseModuleSource(t *testing.T) { }, "custom registry": { input: "example.com/awesomecorp/network/happycloud", - want: ModuleSourceRegistry{ - Package: ModuleRegistryPackage{ + want: addrs.ModuleSourceRegistry{ + Package: addrs.ModuleRegistryPackage{ Host: svchost.Hostname("example.com"), Namespace: "awesomecorp", Name: "network", @@ -106,8 +108,8 @@ func TestParseModuleSource(t *testing.T) { }, "custom registry, subdir": { input: "example.com/awesomecorp/network/happycloud//examples/foo", - want: ModuleSourceRegistry{ - Package: ModuleRegistryPackage{ + want: addrs.ModuleSourceRegistry{ + Package: addrs.ModuleRegistryPackage{ Host: svchost.Hostname("example.com"), Namespace: "awesomecorp", Name: "network", @@ -120,75 +122,75 @@ func TestParseModuleSource(t *testing.T) { // Remote package addresses "github.com shorthand": { input: "github.com/hashicorp/terraform-cidr-subnets", - want: ModuleSourceRemote{ - Package: ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git"), }, }, "github.com shorthand, subdir": { input: "github.com/hashicorp/terraform-cidr-subnets//example/foo", - want: ModuleSourceRemote{ - Package: ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::https://github.com/hashicorp/terraform-cidr-subnets.git"), Subdir: "example/foo", }, }, "git protocol, URL-style": { input: "git://example.com/code/baz.git", - want: ModuleSourceRemote{ - Package: ModulePackage("git://example.com/code/baz.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git://example.com/code/baz.git"), }, }, "git protocol, URL-style, subdir": { input: "git://example.com/code/baz.git//bleep/bloop", - want: ModuleSourceRemote{ - Package: ModulePackage("git://example.com/code/baz.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git://example.com/code/baz.git"), Subdir: "bleep/bloop", }, }, "git over HTTPS, URL-style": { input: "git::https://example.com/code/baz.git", - want: ModuleSourceRemote{ - Package: ModulePackage("git::https://example.com/code/baz.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::https://example.com/code/baz.git"), }, }, "git over HTTPS, URL-style, subdir": { input: "git::https://example.com/code/baz.git//bleep/bloop", - want: ModuleSourceRemote{ - Package: ModulePackage("git::https://example.com/code/baz.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::https://example.com/code/baz.git"), Subdir: "bleep/bloop", }, }, "git over HTTPS, URL-style, subdir, query parameters": { input: "git::https://example.com/code/baz.git//bleep/bloop?otherthing=blah", - want: ModuleSourceRemote{ - Package: ModulePackage("git::https://example.com/code/baz.git?otherthing=blah"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::https://example.com/code/baz.git?otherthing=blah"), Subdir: "bleep/bloop", }, }, "git over SSH, URL-style": { input: "git::ssh://git@example.com/code/baz.git", - want: ModuleSourceRemote{ - Package: ModulePackage("git::ssh://git@example.com/code/baz.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::ssh://git@example.com/code/baz.git"), }, }, "git over SSH, URL-style, subdir": { input: "git::ssh://git@example.com/code/baz.git//bleep/bloop", - want: ModuleSourceRemote{ - Package: ModulePackage("git::ssh://git@example.com/code/baz.git"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("git::ssh://git@example.com/code/baz.git"), Subdir: "bleep/bloop", }, }, "git over SSH, scp-style": { input: "git::git@example.com:code/baz.git", - want: ModuleSourceRemote{ + want: addrs.ModuleSourceRemote{ // Normalized to URL-style - Package: ModulePackage("git::ssh://git@example.com/code/baz.git"), + Package: addrs.ModulePackage("git::ssh://git@example.com/code/baz.git"), }, }, "git over SSH, scp-style, subdir": { input: "git::git@example.com:code/baz.git//bleep/bloop", - want: ModuleSourceRemote{ + want: addrs.ModuleSourceRemote{ // Normalized to URL-style - Package: ModulePackage("git::ssh://git@example.com/code/baz.git"), + Package: addrs.ModulePackage("git::ssh://git@example.com/code/baz.git"), Subdir: "bleep/bloop", }, }, @@ -199,73 +201,73 @@ func TestParseModuleSource(t *testing.T) { "Google Cloud Storage bucket implied, path prefix": { input: "www.googleapis.com/storage/v1/BUCKET_NAME/PATH_TO_MODULE", - want: ModuleSourceRemote{ - Package: ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH_TO_MODULE"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH_TO_MODULE"), }, }, "Google Cloud Storage bucket, path prefix": { input: "gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH_TO_MODULE", - want: ModuleSourceRemote{ - Package: ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH_TO_MODULE"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH_TO_MODULE"), }, }, "Google Cloud Storage bucket implied, archive object": { input: "www.googleapis.com/storage/v1/BUCKET_NAME/PATH/TO/module.zip", - want: ModuleSourceRemote{ - Package: ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH/TO/module.zip"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH/TO/module.zip"), }, }, "Google Cloud Storage bucket, archive object": { input: "gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH/TO/module.zip", - want: ModuleSourceRemote{ - Package: ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH/TO/module.zip"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("gcs::https://www.googleapis.com/storage/v1/BUCKET_NAME/PATH/TO/module.zip"), }, }, "Amazon S3 bucket implied, archive object": { input: "s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip", - want: ModuleSourceRemote{ - Package: ModulePackage("s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip"), }, }, "Amazon S3 bucket, archive object": { input: "s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip", - want: ModuleSourceRemote{ - Package: ModulePackage("s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("s3::https://s3-eu-west-1.amazonaws.com/examplecorp-terraform-modules/vpc.zip"), }, }, "HTTP URL": { input: "http://example.com/module", - want: ModuleSourceRemote{ - Package: ModulePackage("http://example.com/module"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("http://example.com/module"), }, }, "HTTPS URL": { input: "https://example.com/module", - want: ModuleSourceRemote{ - Package: ModulePackage("https://example.com/module"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("https://example.com/module"), }, }, "HTTPS URL, archive file": { input: "https://example.com/module.zip", - want: ModuleSourceRemote{ - Package: ModulePackage("https://example.com/module.zip"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("https://example.com/module.zip"), }, }, "HTTPS URL, forced archive file": { input: "https://example.com/module?archive=tar", - want: ModuleSourceRemote{ - Package: ModulePackage("https://example.com/module?archive=tar"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("https://example.com/module?archive=tar"), }, }, "HTTPS URL, forced archive file and checksum": { input: "https://example.com/module?archive=tar&checksum=blah", - want: ModuleSourceRemote{ + want: addrs.ModuleSourceRemote{ // The query string only actually gets processed when we finally // do the get, so "checksum=blah" is accepted as valid up // at this parsing layer. - Package: ModulePackage("https://example.com/module?archive=tar&checksum=blah"), + Package: addrs.ModulePackage("https://example.com/module?archive=tar&checksum=blah"), }, }, @@ -275,8 +277,8 @@ func TestParseModuleSource(t *testing.T) { // high-level steps to work with these, even though "downloading" // is replaced by a deep filesystem copy instead. input: "/tmp/foo/example", - want: ModuleSourceRemote{ - Package: ModulePackage("file:///tmp/foo/example"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("file:///tmp/foo/example"), }, }, "absolute filesystem path, subdir": { @@ -286,8 +288,8 @@ func TestParseModuleSource(t *testing.T) { // of that subtree, and so they can use the usual subdir // syntax to move the package root higher in the real filesystem. input: "/tmp/foo//example", - want: ModuleSourceRemote{ - Package: ModulePackage("file:///tmp/foo"), + want: addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("file:///tmp/foo"), Subdir: "example", }, }, @@ -316,11 +318,11 @@ func TestParseModuleSource(t *testing.T) { "go-getter will accept all sorts of garbage": { input: "dfgdfgsd:dgfhdfghdfghdfg/dfghdfghdfg", - want: ModuleSourceRemote{ + want: addrs.ModuleSourceRemote{ // Unfortunately go-getter doesn't actually reject a totally // invalid address like this until getting time, as long as // it looks somewhat like a URL. - Package: ModulePackage("dfgdfgsd:dgfhdfghdfghdfg/dfghdfghdfg"), + Package: addrs.ModulePackage("dfgdfgsd:dgfhdfghdfghdfg/dfghdfghdfg"), }, }, } @@ -353,11 +355,11 @@ func TestParseModuleSource(t *testing.T) { func TestModuleSourceRemoteFromRegistry(t *testing.T) { t.Run("both have subdir", func(t *testing.T) { - remote := ModuleSourceRemote{ - Package: ModulePackage("boop"), + remote := addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("boop"), Subdir: "foo", } - registry := ModuleSourceRegistry{ + registry := addrs.ModuleSourceRegistry{ Subdir: "bar", } gotAddr := remote.FromRegistry(registry) @@ -372,11 +374,11 @@ func TestModuleSourceRemoteFromRegistry(t *testing.T) { } }) t.Run("only remote has subdir", func(t *testing.T) { - remote := ModuleSourceRemote{ - Package: ModulePackage("boop"), + remote := addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("boop"), Subdir: "foo", } - registry := ModuleSourceRegistry{ + registry := addrs.ModuleSourceRegistry{ Subdir: "", } gotAddr := remote.FromRegistry(registry) @@ -391,11 +393,11 @@ func TestModuleSourceRemoteFromRegistry(t *testing.T) { } }) t.Run("only registry has subdir", func(t *testing.T) { - remote := ModuleSourceRemote{ - Package: ModulePackage("boop"), + remote := addrs.ModuleSourceRemote{ + Package: addrs.ModulePackage("boop"), Subdir: "", } - registry := ModuleSourceRegistry{ + registry := addrs.ModuleSourceRegistry{ Subdir: "bar", } gotAddr := remote.FromRegistry(registry) @@ -462,15 +464,15 @@ func TestParseModuleSourceRemote(t *testing.T) { } func TestParseModuleSourceRegistry(t *testing.T) { - // We test parseModuleSourceRegistry alone here, in addition to testing - // it indirectly as part of TestParseModuleSource, because general + // We test ParseModuleSourceRegistry alone here, in addition to testing + // it indirectly as part of TestParseaddrs.ModuleSource, because general // module parsing unfortunately eats all of the error situations from // registry passing by falling back to trying for a direct remote package // address. // Historical note: These test cases were originally derived from the // ones in the old internal/registry/regsrc package that the - // ModuleSourceRegistry type is replacing. That package had the notion + // addrs.ModuleSourceRegistry type is replacing. That package had the notion // of "normalized" addresses as separate from the original user input, // but this new implementation doesn't try to preserve the original // user input at all, and so the main string output is always normalized. @@ -614,7 +616,7 @@ func TestParseModuleSourceRegistry(t *testing.T) { t.Fatalf("unexpected error: %s", err.Error()) } - addr, ok := addrI.(ModuleSourceRegistry) + addr, ok := addrI.(addrs.ModuleSourceRegistry) if !ok { t.Fatalf("wrong address type %T; want %T", addrI, addr) } diff --git a/internal/initwd/from_module.go b/internal/initwd/from_module.go index 795f94b98c..c28a1b1c61 100644 --- a/internal/initwd/from_module.go +++ b/internal/initwd/from_module.go @@ -14,11 +14,11 @@ import ( "strings" "github.com/hashicorp/hcl/v2" - "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" "github.com/hashicorp/terraform/internal/getmodules" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" version "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/internal/modsdir" @@ -129,7 +129,7 @@ func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modu // Now we need to create an artificial root module that will seed our // installation process. - sourceAddr, err := addrs.ParseModuleSource(sourceAddrStr) + sourceAddr, err := moduleaddrs.ParseModuleSource(sourceAddrStr) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go index ca814bb4df..35d0812fc0 100644 --- a/internal/initwd/module_install.go +++ b/internal/initwd/module_install.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/getmodules" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" "github.com/hashicorp/terraform/internal/modsdir" "github.com/hashicorp/terraform/internal/registry" "github.com/hashicorp/terraform/internal/registry/regsrc" @@ -590,7 +591,7 @@ func (i *ModuleInstaller) installRegistryModule(ctx context.Context, req *config }) return nil, nil, diags } - realAddr, err := addrs.ParseModuleSource(realAddrRaw) + realAddr, err := moduleaddrs.ParseModuleSource(realAddrRaw) if err != nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, diff --git a/internal/modsdir/manifest.go b/internal/modsdir/manifest.go index 76bf1d65e8..a2e5387c6f 100644 --- a/internal/modsdir/manifest.go +++ b/internal/modsdir/manifest.go @@ -17,6 +17,7 @@ import ( version "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" ) // Record represents some metadata about an installed module, as part @@ -101,7 +102,7 @@ func ReadManifestSnapshot(r io.Reader) (Manifest, error) { // to normalize them back in on read so that we can just gracefully // upgrade on the next "terraform init". if record.SourceAddr != "" { - if addr, err := addrs.ParseModuleSource(record.SourceAddr); err == nil { + if addr, err := moduleaddrs.ParseModuleSource(record.SourceAddr); err == nil { // This is a best effort sort of thing. If the source // address isn't valid then we'll just leave it as-is // and let another component detect that downstream,