diff --git a/internal/configs/config.go b/internal/configs/config.go index 93632612a..cc3f06e79 100644 --- a/internal/configs/config.go +++ b/internal/configs/config.go @@ -177,6 +177,23 @@ func (c *Config) DescendentForInstance(path addrs.ModuleInstance) *Config { return current } +// EntersNewPackage returns true if this call is to an external module, either +// directly via a remote source address or indirectly via a registry source +// address. +// +// Other behaviors in Terraform may treat package crossings as a special +// situation, because that indicates that the caller and callee can change +// independently of one another and thus we should disallow using any features +// where the caller assumes anything about the callee other than its input +// variables, required provider configurations, and output values. +// +// It's not meaningful to ask if the Config representing the root module enters +// a new package because the root module is always outside of all module +// packages, and so this function will arbitrarily return false in that case. +func (c *Config) EntersNewPackage() bool { + return moduleSourceAddrEntersNewPackage(c.SourceAddr) +} + // ProviderRequirements searches the full tree of modules under the receiver // for both explicit and implicit dependencies on providers. // diff --git a/internal/configs/module_call.go b/internal/configs/module_call.go index 7213bea58..5d887dac0 100644 --- a/internal/configs/module_call.go +++ b/internal/configs/module_call.go @@ -60,41 +60,46 @@ func decodeModuleBlock(block *hcl.Block, override bool) (*ModuleCall, hcl.Diagno } if attr, exists := content.Attributes["source"]; exists { + mc.SourceSet = true + mc.SourceAddrRange = attr.Expr.Range() valDiags := gohcl.DecodeExpression(attr.Expr, nil, &mc.SourceAddrRaw) diags = append(diags, valDiags...) - mc.SourceAddrRange = attr.Expr.Range() - mc.SourceSet = true - - addr, err := addrs.ParseModuleSource(mc.SourceAddrRaw) - mc.SourceAddr = addr - if err != nil { - // NOTE: In practice it's actually very unlikely to end up here, - // because our source address parser can turn just about any string - // into some sort of remote package address, and so for most errors - // we'll detect them only during module installation. There are - // still a _few_ purely-syntax errors we can catch at parsing time, - // though, mostly related to remote package sub-paths and local - // paths. - switch err := err.(type) { - case *getmodules.MaybeRelativePathErr: - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid module source address", - Detail: fmt.Sprintf( - "Terraform failed to determine your intended installation method for remote module package %q.\n\nIf you intended this as a path relative to the current module, use \"./%s\" instead. The \"./\" prefix indicates that the address is a relative filesystem path.", - err.Addr, err.Addr, - ), - Subject: mc.SourceAddrRange.Ptr(), - }) - default: - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid module source address", - Detail: fmt.Sprintf("Failed to parse module source address: %s.", err), - Subject: mc.SourceAddrRange.Ptr(), - }) + if !valDiags.HasErrors() { + addr, err := addrs.ParseModuleSource(mc.SourceAddrRaw) + mc.SourceAddr = addr + if err != nil { + // NOTE: In practice it's actually very unlikely to end up here, + // because our source address parser can turn just about any string + // into some sort of remote package address, and so for most errors + // we'll detect them only during module installation. There are + // still a _few_ purely-syntax errors we can catch at parsing time, + // though, mostly related to remote package sub-paths and local + // paths. + switch err := err.(type) { + case *getmodules.MaybeRelativePathErr: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf( + "Terraform failed to determine your intended installation method for remote module package %q.\n\nIf you intended this as a path relative to the current module, use \"./%s\" instead. The \"./\" prefix indicates that the address is a relative filesystem path.", + err.Addr, err.Addr, + ), + Subject: mc.SourceAddrRange.Ptr(), + }) + default: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf("Failed to parse module source address: %s.", err), + Subject: mc.SourceAddrRange.Ptr(), + }) + } } } + // NOTE: We leave mc.SourceAddr as nil for any situation where the + // source attribute is invalid, so any code which tries to carefully + // use the partial result of a failed config decode must be + // resilient to that. } if attr, exists := content.Attributes["version"]; exists { @@ -196,6 +201,19 @@ func decodeModuleBlock(block *hcl.Block, override bool) (*ModuleCall, hcl.Diagno return mc, diags } +// EntersNewPackage returns true if this call is to an external module, either +// directly via a remote source address or indirectly via a registry source +// address. +// +// Other behaviors in Terraform may treat package crossings as a special +// situation, because that indicates that the caller and callee can change +// independently of one another and thus we should disallow using any features +// where the caller assumes anything about the callee other than its input +// variables, required provider configurations, and output values. +func (mc *ModuleCall) EntersNewPackage() bool { + return moduleSourceAddrEntersNewPackage(mc.SourceAddr) +} + // PassedProviderConfig represents a provider config explicitly passed down to // a child module, possibly giving it a new local address in the process. type PassedProviderConfig struct { @@ -234,3 +252,27 @@ var moduleBlockSchema = &hcl.BodySchema{ {Type: "provider", LabelNames: []string{"type"}}, }, } + +func moduleSourceAddrEntersNewPackage(addr addrs.ModuleSource) bool { + switch addr.(type) { + case nil: + // There are only two situations where we should get here: + // - We've been asked about the source address of the root module, + // which is always nil. + // - We've been asked about a ModuleCall that is part of the partial + // result of a failed decode. + // The root module exists outside of all module packages, so we'll + // just return false for that case. For the error case it doesn't + // really matter what we return as long as we don't panic, because + // we only make a best-effort to allow careful inspection of objects + // representing invalid configuration. + return false + case addrs.ModuleSourceLocal: + // Local source addresses are the only address type that remains within + // the same package. + return false + default: + // All other address types enter a new package. + return true + } +} diff --git a/internal/configs/module_call_test.go b/internal/configs/module_call_test.go index a652bebb6..eaf923739 100644 --- a/internal/configs/module_call_test.go +++ b/internal/configs/module_call_test.go @@ -142,3 +142,49 @@ func TestLoadModuleCall(t *testing.T) { t.Error(problem) } } + +func TestModuleSourceAddrEntersNewPackage(t *testing.T) { + tests := []struct { + Addr string + Want bool + }{ + { + "./", + false, + }, + { + "../bork", + false, + }, + { + "/absolute/path", + true, + }, + { + "github.com/example/foo", + true, + }, + { + "hashicorp/subnets/cidr", // registry module + true, + }, + { + "registry.terraform.io/hashicorp/subnets/cidr", // registry module + true, + }, + } + + for _, test := range tests { + t.Run(test.Addr, func(t *testing.T) { + addr, err := addrs.ParseModuleSource(test.Addr) + if err != nil { + t.Fatalf("parsing failed for %q: %s", test.Addr, err) + } + + got := moduleSourceAddrEntersNewPackage(addr) + if got != test.Want { + t.Errorf("wrong result for %q\ngot: %#v\nwant: %#v", addr, got, test.Want) + } + }) + } +}