From e92ea32cd843671a149ecbaf8770425018249180 Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Tue, 2 Aug 2022 10:03:08 +0100 Subject: [PATCH] allow cross-package move statements --- internal/addrs/move_endpoint_module.go | 6 +-- internal/refactoring/move_validate.go | 44 +++------------------- internal/refactoring/move_validate_test.go | 8 ++-- 3 files changed, 13 insertions(+), 45 deletions(-) diff --git a/internal/addrs/move_endpoint_module.go b/internal/addrs/move_endpoint_module.go index fdc8a5c25da0..f2c1408d66b1 100644 --- 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 index 7d00686aecea..96c611543091 100644 --- 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 ), }) } + } } diff --git a/internal/refactoring/move_validate_test.go b/internal/refactoring/move_validate_test.go index aa4ec4f3b800..2213a36133ba 100644 --- 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{