allow cross-package move statements

hashicorp · Aug 2, 2022 · a1fc4b8 · a1fc4b8
1 parent 1dcfabd
commit a1fc4b8
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 72 deletions.
diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go
@@ -5,8 +5,9 @@ import (
 	"reflect"
 	"strings"
 
-	"github.com/hashicorp/terraform/internal/tfdiags"
 	"github.com/zclconf/go-cty/cty"
+
+	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
 // anyKeyImpl is the InstanceKey representation indicating a wildcard, which
@@ -179,8 +180,7 @@ func (e *MoveEndpointInModule) InModuleInstance(modInst ModuleInstance) AbsMovea
 // while selecting a particular object to move.
 //
 // This is a rather special-purpose function here mainly to support our
-// validation rule that a module can only traverse down into child modules
-// that belong to the same module package.
+// validation rule that a module can only traverse down into child modules.
 func (e *MoveEndpointInModule) ModuleCallTraversals() (Module, []ModuleCall) {
 	// We're returning []ModuleCall rather than Module here to make it clearer
 	// that this is a relative sequence of calls rather than an absolute

diff --git a/internal/refactoring/move_validate.go b/internal/refactoring/move_validate.go
@@ -49,46 +49,13 @@ func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, declaredInsts
 
 	for _, stmt := range stmts {
 		// Earlier code that constructs MoveStatement values should ensure that
-		// both stmt.From and stmt.To always belong to the same statement and
-		// thus to the same module.
-		stmtMod, fromCallSteps := stmt.From.ModuleCallTraversals()
-		_, toCallSteps := stmt.To.ModuleCallTraversals()
+		// both stmt.From and stmt.To always belong to the same statement.
+		fromMod, _ := stmt.From.ModuleCallTraversals()
 
-		modCfg := rootCfg.Descendent(stmtMod)
-		if !stmt.Implied {
-			// Implied statements can cross module boundaries because we
-			// generate them only for changing instance keys on a single
-			// resource. They happen to be generated _as if_ they were written
-			// in the root module, but the source and destination are always
-			// in the same module anyway.
-			if pkgAddr := callsThroughModulePackage(modCfg, fromCallSteps); pkgAddr != nil {
-				diags = diags.Append(&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Cross-package move statement",
-					Detail: fmt.Sprintf(
-						"This statement declares a move from an object declared in external module package %q. Move statements can be only within a single module package.",
-						pkgAddr,
-					),
-					Subject: stmt.DeclRange.ToHCL().Ptr(),
-				})
-			}
-			if pkgAddr := callsThroughModulePackage(modCfg, toCallSteps); pkgAddr != nil {
-				diags = diags.Append(&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Cross-package move statement",
-					Detail: fmt.Sprintf(
-						"This statement declares a move to an object declared in external module package %q. Move statements can be only within a single module package.",
-						pkgAddr,
-					),
-					Subject: stmt.DeclRange.ToHCL().Ptr(),
-				})
-			}
-		}
+		for _, fromModInst := range declaredInsts.InstancesForModule(fromMod) {
+			absFrom := stmt.From.InModuleInstance(fromModInst)
 
-		for _, modInst := range declaredInsts.InstancesForModule(stmtMod) {
-
-			absFrom := stmt.From.InModuleInstance(modInst)
-			absTo := stmt.To.InModuleInstance(modInst)
+			absTo := stmt.To.InModuleInstance(fromModInst)
 
 			if addrs.Equivalent(absFrom, absTo) {
 				diags = diags.Append(&hcl.Diagnostic{
@@ -198,6 +165,7 @@ func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, declaredInsts
 					),
 				})
 			}
+
 		}
 	}
 
@@ -369,24 +337,3 @@ func movableObjectDeclRange(addr addrs.AbsMoveable, cfg *configs.Config) (tfdiag
 		panic("unsupported AbsMoveable address type")
 	}
 }
-
-func callsThroughModulePackage(modCfg *configs.Config, callSteps []addrs.ModuleCall) addrs.ModuleSource {
-	var sourceAddr addrs.ModuleSource
-	current := modCfg
-	for _, step := range callSteps {
-		call := current.Module.ModuleCalls[step.Name]
-		if call == nil {
-			break
-		}
-		if call.EntersNewPackage() {
-			sourceAddr = call.SourceAddr
-		}
-		current = modCfg.Children[step.Name]
-		if current == nil {
-			// Weird to have a call but not a config, but we'll tolerate
-			// it to avoid crashing here.
-			break
-		}
-	}
-	return sourceAddr
-}
diff --git a/internal/refactoring/move_validate_test.go b/internal/refactoring/move_validate_test.go
@@ -334,7 +334,7 @@ Each resource can have moved from only one source resource.`,
 					`test.thing`,
 				),
 			},
-			WantError: `Cross-package move statement: This statement declares a move from an object declared in external module package "fake-external:///". Move statements can be only within a single module package.`,
+			WantError: ``,
 		},
 		"move to resource in another module package": {
 			Statements: []MoveStatement{
@@ -344,7 +344,7 @@ Each resource can have moved from only one source resource.`,
 					`module.fake_external.test.thing`,
 				),
 			},
-			WantError: `Cross-package move statement: This statement declares a move to an object declared in external module package "fake-external:///". Move statements can be only within a single module package.`,
+			WantError: ``,
 		},
 		"move from module call in another module package": {
 			Statements: []MoveStatement{
@@ -354,7 +354,7 @@ Each resource can have moved from only one source resource.`,
 					`module.b`,
 				),
 			},
-			WantError: `Cross-package move statement: This statement declares a move from an object declared in external module package "fake-external:///". Move statements can be only within a single module package.`,
+			WantError: ``,
 		},
 		"move to module call in another module package": {
 			Statements: []MoveStatement{
@@ -364,7 +364,7 @@ Each resource can have moved from only one source resource.`,
 					`module.fake_external.module.b`,
 				),
 			},
-			WantError: `Cross-package move statement: This statement declares a move to an object declared in external module package "fake-external:///". Move statements can be only within a single module package.`,
+			WantError: ``,
 		},
 		"implied move from resource in another module package": {
 			Statements: []MoveStatement{

diff --git a/website/docs/language/modules/develop/refactoring.mdx b/website/docs/language/modules/develop/refactoring.mdx
@@ -400,12 +400,6 @@ assumes that all three of these modules are maintained by the same people
 and distributed together in a single
 [module package](/language/modules/sources#modules-in-package-sub-directories).
 
-To reduce [coupling](https://en.wikipedia.org/wiki/Coupling_\(computer_programming\))
-between separately-packaged modules, Terraform only allows declarations of
-moves between modules in the same package. In other words, Terraform would
-not have allowed moving into `module.x` above if the `source` address of
-that call had not been a [local path](/language/modules/sources#local-paths).
-
 Terraform resolves module references in `moved` blocks relative to the module
 instance they are defined in. For example, if the original module above were
 already a child module named `module.original`, the reference to