From 3425e1b4dc65bf6c96b3fb1b1c7fb9554ab99749 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Wed, 20 Apr 2022 17:06:12 +0200 Subject: [PATCH 1/4] add module source addr file This enabled parsing of module source addresses for local and registry modules. --- module.go | 416 ++++++++++++++++++++++++++++++++++++++++++++++ module_package.go | 87 ++++++++++ module_test.go | 322 +++++++++++++++++++++++++++++++++++ 3 files changed, 825 insertions(+) create mode 100644 module.go create mode 100644 module_package.go create mode 100644 module_test.go diff --git a/module.go b/module.go new file mode 100644 index 0000000..2566b4e --- /dev/null +++ b/module.go @@ -0,0 +1,416 @@ +package tfaddr + +import ( + "fmt" + "path" + "regexp" + "strings" + + svchost "github.com/hashicorp/terraform-svchost" +) + +// ModuleSource is the general type for two of the possible module source +// address types. The concrete implementations of this are ModuleSourceLocal +// and ModuleSourceRegistry. +type ModuleSource interface { + // String returns a full representation of the address, including any + // additional components that are typically implied by omission in + // user-written addresses. + // + // We typically use this longer representation in error message, in case + // the inclusion of normally-omitted components is helpful in debugging + // unexpected behavior. + String() string + + // ForDisplay is similar to String but instead returns a representation of + // the idiomatic way to write the address in configuration, omitting + // components that are commonly just implied in addresses written by + // users. + // + // We typically use this shorter representation in informational messages, + // such as the note that we're about to start downloading a package. + ForDisplay() string + + moduleSource() +} + +var _ ModuleSource = ModuleSourceLocal("") +var _ ModuleSource = ModuleSourceRegistry{} + +var moduleSourceLocalPrefixes = []string{ + "./", + "../", + ".\\", + "..\\", +} + +// ParseRawModuleSource 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 two +// 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. +// +// If you know that you're expecting a registry address in particular, use +// ParseRawModuleSourceRegistry instead, which can therefore expose more +// detailed error messages about registry address parsing in particular. +func ParseRawModuleSource(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 := ParseRawModuleSourceRegistry(raw); err == nil { + return ret, nil + } + + return nil, fmt.Errorf("unsupported module source %q", raw) +} + +// ModuleSourceLocal is a ModuleSource representing a local path reference +// from the caller's directory to the callee's directory within the same +// module package. +// +// A "module package" here means a set of modules distributed together in +// the same archive, repository, or similar. That's a significant distinction +// because we always download and cache entire module packages at once, +// and then create relative references within the same directory in order +// to ensure all modules in the package are looking at a consistent filesystem +// layout. We also assume that modules within a package are maintained together, +// which means that cross-cutting maintainance across all of them would be +// possible. +// +// The actual value of a ModuleSourceLocal is a normalized relative path using +// forward slashes, even on operating systems that have other conventions, +// because we're representing traversal within the logical filesystem +// represented by the containing package, not actually within the physical +// filesystem we unpacked the package into. We should typically not construct +// ModuleSourceLocal values directly, except in tests where we can ensure +// the value meets our assumptions. Use ParseRawModuleSource instead if the +// input string is not hard-coded in the program. +type ModuleSourceLocal string + +func parseModuleSourceLocal(raw string) (ModuleSourceLocal, error) { + // As long as we have a suitable prefix (detected by ParseRawModuleSource) + // 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 { + // We assume that our underlying string was already normalized at + // construction, so we just return it verbatim. + return string(s) +} + +func (s ModuleSourceLocal) ForDisplay() string { + return string(s) +} + +// ModuleSourceRegistry is a ModuleSource representing a module listed in a +// Terraform module registry. +// +// A registry source isn't a direct source location but rather an indirection +// over a ModuleSourceRemote. The job of a registry is to translate the +// combination of a ModuleSourceRegistry and a module version number into +// a concrete ModuleSourceRemote that Terraform will then download and +// install. +type ModuleSourceRegistry struct { + // 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. + // This will ultimately become the suffix of the Subdir of the + // ModuleSourceRemote that the registry address translates to. + // + // Subdir uses a normalized forward-slash-based path syntax within the + // virtual filesystem represented by the final package. It will never + // include `../` or `./` sequences. + Subdir string +} + +// DefaultModuleRegistryHost is the hostname used for registry-based module +// source addresses that do not have an explicit hostname. +const DefaultModuleRegistryHost = svchost.Hostname("registry.terraform.io") + +var moduleRegistryNamePattern = regexp.MustCompile("^[0-9A-Za-z](?:[0-9A-Za-z-_]{0,62}[0-9A-Za-z])?$") +var moduleRegistryTargetSystemPattern = regexp.MustCompile("^[0-9a-z]{1,64}$") + +// ParseRawModuleSourceRegistry is a variant of ParseRawModuleSource which only +// accepts module registry addresses, and will reject any other address type. +// +// Use this instead of ParseRawModuleSource 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 ParseRawModuleSourceRegistry(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) + } + + ret, err := parseModuleSourceRegistry(raw) + if err != nil { + // This is to make sure we return a nil ModuleSource, rather than + // a non-nil ModuleSource containing a zero-value ModuleSourceRegistry. + return nil, err + } + return ret, nil +} + +func parseModuleSourceRegistry(raw string) (ModuleSourceRegistry, error) { + var err error + + var subDir string + raw, subDir = splitPackageSubdir(raw) + if strings.HasPrefix(subDir, "../") { + return ModuleSourceRegistry{}, fmt.Errorf("subdirectory path %q leads outside of the module package", subDir) + } + + parts := strings.Split(raw, "/") + // A valid registry address has either three or four parts, because the + // leading hostname part is optional. + if len(parts) != 3 && len(parts) != 4 { + return ModuleSourceRegistry{}, fmt.Errorf("a module registry source address must have either three or four slash-separated components") + } + + host := DefaultModuleRegistryHost + if len(parts) == 4 { + host, err = svchost.ForComparison(parts[0]) + if err != nil { + // The svchost library doesn't produce very good error messages to + // return to an end-user, so we'll use some custom ones here. + switch { + case strings.Contains(parts[0], "--"): + // Looks like possibly punycode, which we don't allow here + // to ensure that source addresses are written readably. + return ModuleSourceRegistry{}, fmt.Errorf("invalid module registry hostname %q; internationalized domain names must be given as direct unicode characters, not in punycode", parts[0]) + default: + return ModuleSourceRegistry{}, fmt.Errorf("invalid module registry hostname %q", parts[0]) + } + } + if !strings.Contains(host.String(), ".") { + return ModuleSourceRegistry{}, fmt.Errorf("invalid module registry hostname: must contain at least one dot") + } + // Discard the hostname prefix now that we've processed it + parts = parts[1:] + } + + ret := ModuleSourceRegistry{ + PackageAddr: ModuleRegistryPackage{ + Host: host, + }, + + Subdir: subDir, + } + + if host == svchost.Hostname("github.com") || host == svchost.Hostname("bitbucket.org") { + 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.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. + return ret, fmt.Errorf("source address must have three more components after the hostname: the namespace, the name, and the target system") + } + return ret, fmt.Errorf("invalid namespace %q: %s", parts[0], err) + } + if ret.PackageAddr.Name, err = parseModuleRegistryName(parts[1]); err != nil { + return ret, fmt.Errorf("invalid module name %q: %s", parts[1], err) + } + 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") + } + return ret, fmt.Errorf("invalid target system %q: %s", parts[2], err) + } + + return ret, nil +} + +// parseModuleRegistryName validates and normalizes a string in either the +// "namespace" or "name" position of a module registry source address. +func parseModuleRegistryName(given string) (string, error) { + // Similar to the names in provider source addresses, we defined these + // to be compatible with what filesystems and typical remote systems + // like GitHub allow in names. Unfortunately we didn't end up defining + // these exactly equivalently: provider names can only use dashes as + // punctuation, whereas module names can use underscores. So here we're + // using some regular expressions from the original module source + // implementation, rather than using the IDNA rules as we do in + // ParseProviderPart. + + if !moduleRegistryNamePattern.MatchString(given) { + return "", fmt.Errorf("must be between one and 64 characters, including ASCII letters, digits, dashes, and underscores, where dashes and underscores may not be the prefix or suffix") + } + + // We also skip normalizing the name to lowercase, because we historically + // didn't do that and so existing module registries might be doing + // case-sensitive matching. + return given, nil +} + +// parseModuleRegistryTargetSystem validates and normalizes a string in the +// "target system" position of a module registry source address. This is +// what we historically called "provider" but never actually enforced as +// being a provider address, and now _cannot_ be a provider address because +// provider addresses have three slash-separated components of their own. +func parseModuleRegistryTargetSystem(given string) (string, error) { + // Similar to the names in provider source addresses, we defined these + // to be compatible with what filesystems and typical remote systems + // like GitHub allow in names. Unfortunately we didn't end up defining + // these exactly equivalently: provider names can't use dashes or + // underscores. So here we're using some regular expressions from the + // original module source implementation, rather than using the IDNA rules + // as we do in ParseProviderPart. + + if !moduleRegistryTargetSystemPattern.MatchString(given) { + return "", fmt.Errorf("must be between one and 64 ASCII letters or digits") + } + + // We also skip normalizing the name to lowercase, because we historically + // didn't do that and so existing module registries might be doing + // case-sensitive matching. + return given, nil +} + +func (s ModuleSourceRegistry) moduleSource() {} + +func (s ModuleSourceRegistry) String() string { + if s.Subdir != "" { + return s.PackageAddr.String() + "//" + s.Subdir + } + return s.PackageAddr.String() +} + +func (s ModuleSourceRegistry) ForDisplay() string { + if s.Subdir != "" { + return s.PackageAddr.ForDisplay() + "//" + s.Subdir + } + return s.PackageAddr.ForDisplay() +} + +// splitPackageSubdir detects whether the given address string has a +// subdirectory portion, and if so returns a non-empty subDir string +// along with the trimmed package address. +// +// If the given string doesn't have a subdirectory portion then it'll +// just be returned verbatim in packageAddr, with an empty subDir value. +func splitPackageSubdir(given string) (packageAddr, subDir string) { + packageAddr, subDir = sourceDirSubdir(given) + if subDir != "" { + subDir = path.Clean(subDir) + } + return packageAddr, subDir +} + +// sourceDirSubdir takes a source URL and returns a tuple of the URL without +// the subdir and the subdir. +// +// ex: +// dom.com/path/?q=p => dom.com/path/?q=p, "" +// proto://dom.com/path//*?q=p => proto://dom.com/path?q=p, "*" +// proto://dom.com/path//path2?q=p => proto://dom.com/path?q=p, "path2" +func sourceDirSubdir(src string) (string, string) { + // URL might contains another url in query parameters + stop := len(src) + if idx := strings.Index(src, "?"); idx > -1 { + stop = idx + } + + // Calculate an offset to avoid accidentally marking the scheme + // as the dir. + var offset int + if idx := strings.Index(src[:stop], "://"); idx > -1 { + offset = idx + 3 + } + + // First see if we even have an explicit subdir + idx := strings.Index(src[offset:stop], "//") + if idx == -1 { + return src, "" + } + + idx += offset + subdir := src[idx+2:] + src = src[:idx] + + // Next, check if we have query parameters and push them onto the + // URL. + if idx = strings.Index(subdir, "?"); idx > -1 { + query := subdir[idx:] + subdir = subdir[:idx] + src += query + } + + return src, subdir +} diff --git a/module_package.go b/module_package.go new file mode 100644 index 0000000..cba03a7 --- /dev/null +++ b/module_package.go @@ -0,0 +1,87 @@ +package tfaddr + +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. +// +// A ModulePackage is a string in go-getter's address syntax. By convention, +// we use ModulePackage-typed values only for the result of successfully +// running the go-getter "detectors", which produces an address string which +// includes an explicit installation method prefix along with an address +// string in the format expected by that installation method. +// +// Note that although the "detector" phase of go-getter does do some simple +// normalization in certain cases, it isn't generally possible to compare +// two ModulePackage values to decide if they refer to the same package. Two +// equal ModulePackage values represent the same package, but there might be +// other non-equal ModulePackage values that also refer to that package, and +// there is no reliable way to determine that. +// +// Don't convert a user-provided string directly to ModulePackage. Instead, +// use ParseModuleSource with a remote module address and then access the +// ModulePackage value from the result, making sure to also handle the +// selected subdirectory if any. You should convert directly to ModulePackage +// only for a string that is hard-coded into the program (e.g. in a unit test) +// where you've ensured that it's already in the expected syntax. +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 { + // 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() +} + +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/module_test.go b/module_test.go new file mode 100644 index 0000000..3649c9e --- /dev/null +++ b/module_test.go @@ -0,0 +1,322 @@ +package tfaddr + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + svchost "github.com/hashicorp/terraform-svchost" +) + +func TestParseModuleSource(t *testing.T) { + tests := map[string]struct { + input string + want ModuleSource + wantErr string + }{ + // Local paths + "local in subdirectory": { + input: "./child", + want: ModuleSourceLocal("./child"), + }, + "local in subdirectory non-normalized": { + input: "./nope/../child", + want: ModuleSourceLocal("./child"), + }, + "local in sibling directory": { + input: "../sibling", + want: ModuleSourceLocal("../sibling"), + }, + "local in sibling directory non-normalized": { + input: "./nope/../../sibling", + want: ModuleSourceLocal("../sibling"), + }, + "Windows-style local in subdirectory": { + input: `.\child`, + want: ModuleSourceLocal("./child"), + }, + "Windows-style local in subdirectory non-normalized": { + input: `.\nope\..\child`, + want: ModuleSourceLocal("./child"), + }, + "Windows-style local in sibling directory": { + input: `..\sibling`, + want: ModuleSourceLocal("../sibling"), + }, + "Windows-style local in sibling directory non-normalized": { + input: `.\nope\..\..\sibling`, + want: ModuleSourceLocal("../sibling"), + }, + "an abominable mix of different slashes": { + input: `./nope\nope/why\./please\don't`, + want: ModuleSourceLocal("./nope/nope/why/please/don't"), + }, + + // Registry addresses + // (NOTE: There is another test function TestParseModuleSourceRegistry + // 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{ + 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{ + PackageAddr: ModuleRegistryPackage{ + Host: svchost.Hostname("registry.terraform.io"), + Namespace: "hashicorp", + Name: "subnets", + TargetSystem: "cidr", + }, + Subdir: "examples/foo", + }, + }, + "main registry implied, escaping subdir": { + input: "hashicorp/subnets/cidr//../nope", + wantErr: `unsupported module source "hashicorp/subnets/cidr//../nope"`, + }, + "custom registry": { + input: "example.com/awesomecorp/network/happycloud", + want: ModuleSourceRegistry{ + 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{ + PackageAddr: ModuleRegistryPackage{ + Host: svchost.Hostname("example.com"), + Namespace: "awesomecorp", + Name: "network", + TargetSystem: "happycloud", + }, + Subdir: "examples/foo", + }, + }, + + "relative path without the needed prefix": { + input: "boop/bloop", + // For this case we return a generic error message from the addrs + // layer, but using a specialized error type which our module + // installer checks for and produces an extra hint for users who + // were intending to write a local path which then got + // misinterpreted as a remote source due to the missing prefix. + // However, the main message is generic here because this is really + // just a general "this string doesn't match any of our source + // address patterns" situation, not _necessarily_ about relative + // local paths. + wantErr: `unsupported module source "boop/bloop"`, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + addr, err := ParseRawModuleSource(test.input) + + if test.wantErr != "" { + switch { + case err == nil: + t.Errorf("unexpected success\nwant error: %s", test.wantErr) + case err.Error() != test.wantErr: + t.Errorf("wrong error messages\ngot: %s\nwant: %s", err.Error(), test.wantErr) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %s", err.Error()) + } + + if diff := cmp.Diff(addr, test.want); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + } + +} + +func TestParseModuleSourceRegistry(t *testing.T) { + // We test parseModuleSourceRegistry alone here, in addition to testing + // it indirectly as part of TestParseModuleSource, 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 + // 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. + // + // That package also had some behaviors to turn the namespace, name, and + // remote system portions into lowercase, but apparently we didn't + // actually make use of that in the end and were preserving the case + // the user provided in the input, and so for backward compatibility + // we're continuing to do that here, at the expense of now making the + // "ForDisplay" output case-preserving where its predecessor in the + // old package wasn't. The main Terraform Registry at registry.terraform.io + // is itself case-insensitive anyway, so our case-preserving here is + // entirely for the benefit of existing third-party registry + // implementations that might be case-sensitive, which we must remain + // compatible with now. + + tests := map[string]struct { + input string + wantString string + wantForDisplay string + wantForProtocol string + wantErr string + }{ + "public registry": { + input: `hashicorp/consul/aws`, + wantString: `registry.terraform.io/hashicorp/consul/aws`, + wantForDisplay: `hashicorp/consul/aws`, + wantForProtocol: `hashicorp/consul/aws`, + }, + "public registry with subdir": { + input: `hashicorp/consul/aws//foo`, + wantString: `registry.terraform.io/hashicorp/consul/aws//foo`, + wantForDisplay: `hashicorp/consul/aws//foo`, + wantForProtocol: `hashicorp/consul/aws`, + }, + "public registry using explicit hostname": { + input: `registry.terraform.io/hashicorp/consul/aws`, + wantString: `registry.terraform.io/hashicorp/consul/aws`, + wantForDisplay: `hashicorp/consul/aws`, + wantForProtocol: `hashicorp/consul/aws`, + }, + "public registry with mixed case names": { + input: `HashiCorp/Consul/aws`, + wantString: `registry.terraform.io/HashiCorp/Consul/aws`, + wantForDisplay: `HashiCorp/Consul/aws`, + wantForProtocol: `HashiCorp/Consul/aws`, + }, + "private registry with non-standard port": { + input: `Example.com:1234/HashiCorp/Consul/aws`, + wantString: `example.com:1234/HashiCorp/Consul/aws`, + wantForDisplay: `example.com:1234/HashiCorp/Consul/aws`, + wantForProtocol: `HashiCorp/Consul/aws`, + }, + "private registry with IDN hostname": { + input: `Испытание.com/HashiCorp/Consul/aws`, + wantString: `испытание.com/HashiCorp/Consul/aws`, + wantForDisplay: `испытание.com/HashiCorp/Consul/aws`, + wantForProtocol: `HashiCorp/Consul/aws`, + }, + "private registry with IDN hostname and non-standard port": { + input: `Испытание.com:1234/HashiCorp/Consul/aws//Foo`, + wantString: `испытание.com:1234/HashiCorp/Consul/aws//Foo`, + wantForDisplay: `испытание.com:1234/HashiCorp/Consul/aws//Foo`, + wantForProtocol: `HashiCorp/Consul/aws`, + }, + "invalid hostname": { + input: `---.com/HashiCorp/Consul/aws`, + wantErr: `invalid module registry hostname "---.com"; internationalized domain names must be given as direct unicode characters, not in punycode`, + }, + "hostname with only one label": { + // This was historically forbidden in our initial implementation, + // so we keep it forbidden to avoid newly interpreting such + // addresses as registry addresses rather than remote source + // addresses. + input: `foo/var/baz/qux`, + wantErr: `invalid module registry hostname: must contain at least one dot`, + }, + "invalid target system characters": { + input: `foo/var/no-no-no`, + wantErr: `invalid target system "no-no-no": must be between one and 64 ASCII letters or digits`, + }, + "invalid target system length": { + input: `foo/var/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaah`, + wantErr: `invalid target system "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaah": must be between one and 64 ASCII letters or digits`, + }, + "invalid namespace": { + input: `boop!/var/baz`, + wantErr: `invalid namespace "boop!": must be between one and 64 characters, including ASCII letters, digits, dashes, and underscores, where dashes and underscores may not be the prefix or suffix`, + }, + "missing part with explicit hostname": { + input: `foo.com/var/baz`, + wantErr: `source address must have three more components after the hostname: the namespace, the name, and the target system`, + }, + "errant query string": { + input: `foo/var/baz?otherthing`, + wantErr: `module registry addresses may not include a query string portion`, + }, + "github.com": { + // We don't allow using github.com like a module registry because + // that conflicts with the historically-supported shorthand for + // installing directly from GitHub-hosted git repositories. + input: `github.com/HashiCorp/Consul/aws`, + wantErr: `can't use "github.com" as a module registry host, because it's reserved for installing directly from version control repositories`, + }, + "bitbucket.org": { + // We don't allow using bitbucket.org like a module registry because + // that conflicts with the historically-supported shorthand for + // installing directly from BitBucket-hosted git repositories. + input: `bitbucket.org/HashiCorp/Consul/aws`, + wantErr: `can't use "bitbucket.org" as a module registry host, because it's reserved for installing directly from version control repositories`, + }, + "local path from current dir": { + // Can't use a local path when we're specifically trying to parse + // a _registry_ source address. + input: `./boop`, + wantErr: `can't use local directory "./boop" as a module registry address`, + }, + "local path from parent dir": { + // Can't use a local path when we're specifically trying to parse + // a _registry_ source address. + input: `../boop`, + wantErr: `can't use local directory "../boop" as a module registry address`, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + addrI, err := ParseRawModuleSourceRegistry(test.input) + + if test.wantErr != "" { + switch { + case err == nil: + t.Errorf("unexpected success\nwant error: %s", test.wantErr) + case err.Error() != test.wantErr: + t.Errorf("wrong error messages\ngot: %s\nwant: %s", err.Error(), test.wantErr) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %s", err.Error()) + } + + addr, ok := addrI.(ModuleSourceRegistry) + if !ok { + t.Fatalf("wrong address type %T; want %T", addrI, addr) + } + + if got, want := addr.String(), test.wantString; got != want { + t.Errorf("wrong String() result\ngot: %s\nwant: %s", got, want) + } + if got, want := addr.ForDisplay(), test.wantForDisplay; got != want { + t.Errorf("wrong ForDisplay() result\ngot: %s\nwant: %s", got, want) + } + if got, want := addr.PackageAddr.ForRegistryProtocol(), test.wantForProtocol; got != want { + t.Errorf("wrong ForRegistryProtocol() result\ngot: %s\nwant: %s", got, want) + } + }) + } +} From 6d9c659e28337cef5ea28ed8f399d7ad1f0f5d38 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Wed, 20 Apr 2022 17:20:08 +0200 Subject: [PATCH 2/4] add module source example to readme --- README.md | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2ed8398..f2c2998 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,9 @@ The most common source of these addresses outside of Terraform Core is JSON representation of state, plan, or schemas as obtained via [`hashicorp/terraform-exec`](https://github.com/hashicorp/terraform-exec). -## Example +## Parsing Provider Addresses + +### Example ```go p, err := ParseRawProviderSourceString("hashicorp/aws") @@ -23,7 +25,7 @@ if err != nil { // } ``` -## Legacy address +### Legacy address A legacy address is by itself (without more context) ambiguous. For example `aws` may represent either the official `hashicorp/aws` @@ -36,7 +38,7 @@ the address was produced by an affected version. If you do not have that context you should parse the string via `ParseRawProviderSourceString` and then check `addr.IsLegacy()`. -### What to do with a legacy address? +#### What to do with a legacy address? Ask the Registry API whether and where the provider was moved to @@ -70,7 +72,7 @@ If you cache results (which you should), ensure you have invalidation mechanism in place because target (migrated) namespace may change. Hard-coding migrations anywhere in code is strongly discouraged. -### `terraform` provider +#### `terraform` provider Like any other legacy address `terraform` is also ambiguous. Such address may (most unlikely) represent a custom-built provider called `terraform`, @@ -86,3 +88,31 @@ i.e. assume all of its logic including schema is contained within Terraform Core. In such case you should just use `NewBuiltInProvider("terraform")`. + +## Parsing Module Addresses + +### Example + +```go +local, err := ParseRawModuleSource("./local") +if err != nil { + // deal with error +} + +// local == ModuleSourceLocal("./local") + +registry, err := ParseRawModuleSource("hashicorp/subnets/cidr") +if err != nil { + // deal with error +} + +// registry == ModuleSourceRegistry{ +// PackageAddr: ModuleRegistryPackage{ +// Host: svchost.Hostname("registry.terraform.io"), +// Namespace: "hashicorp", +// Name: "subnets", +// TargetSystem: "cidr", +// }, +// Subdir: "", +// }, +``` From 429a0be6ab8ceee4a2f83ed816b3ca54d677fb12 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 22 Apr 2022 10:41:18 +0200 Subject: [PATCH 3/4] remove ParseRawModuleSource and local source parsing --- module.go | 188 ++++++------------------------------------------- module_test.go | 109 ++++------------------------ 2 files changed, 33 insertions(+), 264 deletions(-) diff --git a/module.go b/module.go index 2566b4e..afbc6eb 100644 --- a/module.go +++ b/module.go @@ -9,34 +9,6 @@ import ( svchost "github.com/hashicorp/terraform-svchost" ) -// ModuleSource is the general type for two of the possible module source -// address types. The concrete implementations of this are ModuleSourceLocal -// and ModuleSourceRegistry. -type ModuleSource interface { - // String returns a full representation of the address, including any - // additional components that are typically implied by omission in - // user-written addresses. - // - // We typically use this longer representation in error message, in case - // the inclusion of normally-omitted components is helpful in debugging - // unexpected behavior. - String() string - - // ForDisplay is similar to String but instead returns a representation of - // the idiomatic way to write the address in configuration, omitting - // components that are commonly just implied in addresses written by - // users. - // - // We typically use this shorter representation in informational messages, - // such as the note that we're about to start downloading a package. - ForDisplay() string - - moduleSource() -} - -var _ ModuleSource = ModuleSourceLocal("") -var _ ModuleSource = ModuleSourceRegistry{} - var moduleSourceLocalPrefixes = []string{ "./", "../", @@ -44,109 +16,6 @@ var moduleSourceLocalPrefixes = []string{ "..\\", } -// ParseRawModuleSource 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 two -// 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. -// -// If you know that you're expecting a registry address in particular, use -// ParseRawModuleSourceRegistry instead, which can therefore expose more -// detailed error messages about registry address parsing in particular. -func ParseRawModuleSource(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 := ParseRawModuleSourceRegistry(raw); err == nil { - return ret, nil - } - - return nil, fmt.Errorf("unsupported module source %q", raw) -} - -// ModuleSourceLocal is a ModuleSource representing a local path reference -// from the caller's directory to the callee's directory within the same -// module package. -// -// A "module package" here means a set of modules distributed together in -// the same archive, repository, or similar. That's a significant distinction -// because we always download and cache entire module packages at once, -// and then create relative references within the same directory in order -// to ensure all modules in the package are looking at a consistent filesystem -// layout. We also assume that modules within a package are maintained together, -// which means that cross-cutting maintainance across all of them would be -// possible. -// -// The actual value of a ModuleSourceLocal is a normalized relative path using -// forward slashes, even on operating systems that have other conventions, -// because we're representing traversal within the logical filesystem -// represented by the containing package, not actually within the physical -// filesystem we unpacked the package into. We should typically not construct -// ModuleSourceLocal values directly, except in tests where we can ensure -// the value meets our assumptions. Use ParseRawModuleSource instead if the -// input string is not hard-coded in the program. -type ModuleSourceLocal string - -func parseModuleSourceLocal(raw string) (ModuleSourceLocal, error) { - // As long as we have a suitable prefix (detected by ParseRawModuleSource) - // 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) { @@ -156,26 +25,8 @@ func isModuleSourceLocal(raw string) bool { return false } -func (s ModuleSourceLocal) moduleSource() {} - -func (s ModuleSourceLocal) String() string { - // We assume that our underlying string was already normalized at - // construction, so we just return it verbatim. - return string(s) -} - -func (s ModuleSourceLocal) ForDisplay() string { - return string(s) -} - -// ModuleSourceRegistry is a ModuleSource representing a module listed in a -// Terraform module registry. -// -// A registry source isn't a direct source location but rather an indirection -// over a ModuleSourceRemote. The job of a registry is to translate the -// combination of a ModuleSourceRegistry and a module version number into -// a concrete ModuleSourceRemote that Terraform will then download and -// install. +// ModuleSourceRegistry is representing a module listed in a Terraform module +// registry. type ModuleSourceRegistry struct { // PackageAddr is the registry package that the target module belongs to. // The module installer must translate this into a ModuleSourceRemote @@ -201,14 +52,9 @@ const DefaultModuleRegistryHost = svchost.Hostname("registry.terraform.io") var moduleRegistryNamePattern = regexp.MustCompile("^[0-9A-Za-z](?:[0-9A-Za-z-_]{0,62}[0-9A-Za-z])?$") var moduleRegistryTargetSystemPattern = regexp.MustCompile("^[0-9a-z]{1,64}$") -// ParseRawModuleSourceRegistry is a variant of ParseRawModuleSource which only -// accepts module registry addresses, and will reject any other address type. -// -// Use this instead of ParseRawModuleSource 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 ParseRawModuleSourceRegistry(raw string) (ModuleSource, error) { +// ParseRawModuleSourceRegistry only accepts module registry addresses, and +// will reject any other address type. +func ParseRawModuleSourceRegistry(raw string) (ModuleSourceRegistry, 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. @@ -216,13 +62,7 @@ func ParseRawModuleSourceRegistry(raw string) (ModuleSource, error) { return ModuleSourceRegistry{}, fmt.Errorf("can't use local directory %q as a module registry address", raw) } - ret, err := parseModuleSourceRegistry(raw) - if err != nil { - // This is to make sure we return a nil ModuleSource, rather than - // a non-nil ModuleSource containing a zero-value ModuleSourceRegistry. - return nil, err - } - return ret, nil + return parseModuleSourceRegistry(raw) } func parseModuleSourceRegistry(raw string) (ModuleSourceRegistry, error) { @@ -343,8 +183,13 @@ func parseModuleRegistryTargetSystem(given string) (string, error) { return given, nil } -func (s ModuleSourceRegistry) moduleSource() {} - +// String returns a full representation of the address, including any +// additional components that are typically implied by omission in +// user-written addresses. +// +// We typically use this longer representation in error message, in case +// the inclusion of normally-omitted components is helpful in debugging +// unexpected behavior. func (s ModuleSourceRegistry) String() string { if s.Subdir != "" { return s.PackageAddr.String() + "//" + s.Subdir @@ -352,6 +197,13 @@ func (s ModuleSourceRegistry) String() string { return s.PackageAddr.String() } +// ForDisplay is similar to String but instead returns a representation of +// the idiomatic way to write the address in configuration, omitting +// components that are commonly just implied in addresses written by +// users. +// +// We typically use this shorter representation in informational messages, +// such as the note that we're about to start downloading a package. func (s ModuleSourceRegistry) ForDisplay() string { if s.Subdir != "" { return s.PackageAddr.ForDisplay() + "//" + s.Subdir diff --git a/module_test.go b/module_test.go index 3649c9e..5bcfb1f 100644 --- a/module_test.go +++ b/module_test.go @@ -7,55 +7,12 @@ import ( svchost "github.com/hashicorp/terraform-svchost" ) -func TestParseModuleSource(t *testing.T) { +func TestParseRawModuleSourceRegistry_Simple(t *testing.T) { tests := map[string]struct { input string - want ModuleSource + want ModuleSourceRegistry wantErr string }{ - // Local paths - "local in subdirectory": { - input: "./child", - want: ModuleSourceLocal("./child"), - }, - "local in subdirectory non-normalized": { - input: "./nope/../child", - want: ModuleSourceLocal("./child"), - }, - "local in sibling directory": { - input: "../sibling", - want: ModuleSourceLocal("../sibling"), - }, - "local in sibling directory non-normalized": { - input: "./nope/../../sibling", - want: ModuleSourceLocal("../sibling"), - }, - "Windows-style local in subdirectory": { - input: `.\child`, - want: ModuleSourceLocal("./child"), - }, - "Windows-style local in subdirectory non-normalized": { - input: `.\nope\..\child`, - want: ModuleSourceLocal("./child"), - }, - "Windows-style local in sibling directory": { - input: `..\sibling`, - want: ModuleSourceLocal("../sibling"), - }, - "Windows-style local in sibling directory non-normalized": { - input: `.\nope\..\..\sibling`, - want: ModuleSourceLocal("../sibling"), - }, - "an abominable mix of different slashes": { - input: `./nope\nope/why\./please\don't`, - want: ModuleSourceLocal("./nope/nope/why/please/don't"), - }, - - // Registry addresses - // (NOTE: There is another test function TestParseModuleSourceRegistry - // 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{ @@ -80,10 +37,6 @@ func TestParseModuleSource(t *testing.T) { Subdir: "examples/foo", }, }, - "main registry implied, escaping subdir": { - input: "hashicorp/subnets/cidr//../nope", - wantErr: `unsupported module source "hashicorp/subnets/cidr//../nope"`, - }, "custom registry": { input: "example.com/awesomecorp/network/happycloud", want: ModuleSourceRegistry{ @@ -108,25 +61,11 @@ func TestParseModuleSource(t *testing.T) { Subdir: "examples/foo", }, }, - - "relative path without the needed prefix": { - input: "boop/bloop", - // For this case we return a generic error message from the addrs - // layer, but using a specialized error type which our module - // installer checks for and produces an extra hint for users who - // were intending to write a local path which then got - // misinterpreted as a remote source due to the missing prefix. - // However, the main message is generic here because this is really - // just a general "this string doesn't match any of our source - // address patterns" situation, not _necessarily_ about relative - // local paths. - wantErr: `unsupported module source "boop/bloop"`, - }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - addr, err := ParseRawModuleSource(test.input) + addr, err := ParseRawModuleSourceRegistry(test.input) if test.wantErr != "" { switch { @@ -150,32 +89,7 @@ func TestParseModuleSource(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 - // 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 - // 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. - // - // That package also had some behaviors to turn the namespace, name, and - // remote system portions into lowercase, but apparently we didn't - // actually make use of that in the end and were preserving the case - // the user provided in the input, and so for backward compatibility - // we're continuing to do that here, at the expense of now making the - // "ForDisplay" output case-preserving where its predecessor in the - // old package wasn't. The main Terraform Registry at registry.terraform.io - // is itself case-insensitive anyway, so our case-preserving here is - // entirely for the benefit of existing third-party registry - // implementations that might be case-sensitive, which we must remain - // compatible with now. - +func TestParseRawModuleSourceRegistry(t *testing.T) { tests := map[string]struct { input string wantString string @@ -283,11 +197,19 @@ func TestParseModuleSourceRegistry(t *testing.T) { input: `../boop`, wantErr: `can't use local directory "../boop" as a module registry address`, }, + "main registry implied, escaping subdir": { + input: "hashicorp/subnets/cidr//../nope", + wantErr: `subdirectory path "../nope" leads outside of the module package`, + }, + "relative path without the needed prefix": { + input: "boop/bloop", + wantErr: "a module registry source address must have either three or four slash-separated components", + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - addrI, err := ParseRawModuleSourceRegistry(test.input) + addr, err := ParseRawModuleSourceRegistry(test.input) if test.wantErr != "" { switch { @@ -303,11 +225,6 @@ func TestParseModuleSourceRegistry(t *testing.T) { t.Fatalf("unexpected error: %s", err.Error()) } - addr, ok := addrI.(ModuleSourceRegistry) - if !ok { - t.Fatalf("wrong address type %T; want %T", addrI, addr) - } - if got, want := addr.String(), test.wantString; got != want { t.Errorf("wrong String() result\ngot: %s\nwant: %s", got, want) } From acfa784aa8107cf50574c552ccb1b5f0803357e0 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 22 Apr 2022 11:14:12 +0200 Subject: [PATCH 4/4] don't check for local module addresses, update readme --- README.md | 9 +-------- module.go | 27 --------------------------- module_test.go | 4 ++-- 3 files changed, 3 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index f2c2998..091ff70 100644 --- a/README.md +++ b/README.md @@ -94,14 +94,7 @@ In such case you should just use `NewBuiltInProvider("terraform")`. ### Example ```go -local, err := ParseRawModuleSource("./local") -if err != nil { - // deal with error -} - -// local == ModuleSourceLocal("./local") - -registry, err := ParseRawModuleSource("hashicorp/subnets/cidr") +registry, err := ParseRawModuleSourceRegistry("hashicorp/subnets/cidr") if err != nil { // deal with error } diff --git a/module.go b/module.go index afbc6eb..f90279e 100644 --- a/module.go +++ b/module.go @@ -9,22 +9,6 @@ import ( svchost "github.com/hashicorp/terraform-svchost" ) -var moduleSourceLocalPrefixes = []string{ - "./", - "../", - ".\\", - "..\\", -} - -func isModuleSourceLocal(raw string) bool { - for _, prefix := range moduleSourceLocalPrefixes { - if strings.HasPrefix(raw, prefix) { - return true - } - } - return false -} - // ModuleSourceRegistry is representing a module listed in a Terraform module // registry. type ModuleSourceRegistry struct { @@ -55,17 +39,6 @@ var moduleRegistryTargetSystemPattern = regexp.MustCompile("^[0-9a-z]{1,64}$") // ParseRawModuleSourceRegistry only accepts module registry addresses, and // will reject any other address type. func ParseRawModuleSourceRegistry(raw string) (ModuleSourceRegistry, 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) - } - - return parseModuleSourceRegistry(raw) -} - -func parseModuleSourceRegistry(raw string) (ModuleSourceRegistry, error) { var err error var subDir string diff --git a/module_test.go b/module_test.go index 5bcfb1f..0ca9155 100644 --- a/module_test.go +++ b/module_test.go @@ -189,13 +189,13 @@ func TestParseRawModuleSourceRegistry(t *testing.T) { // Can't use a local path when we're specifically trying to parse // a _registry_ source address. input: `./boop`, - wantErr: `can't use local directory "./boop" as a module registry address`, + wantErr: `a module registry source address must have either three or four slash-separated components`, }, "local path from parent dir": { // Can't use a local path when we're specifically trying to parse // a _registry_ source address. input: `../boop`, - wantErr: `can't use local directory "../boop" as a module registry address`, + wantErr: `a module registry source address must have either three or four slash-separated components`, }, "main registry implied, escaping subdir": { input: "hashicorp/subnets/cidr//../nope",