allow cross-package move statements (#31556)

This commit is contained in:
kmoe 2022-08-16 16:52:57 +02:00 committed by GitHub
parent f79e912a81
commit 56a1e0d1c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 13 additions and 72 deletions

View File

@ -5,8 +5,9 @@ import (
"reflect" "reflect"
"strings" "strings"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/internal/tfdiags"
) )
// anyKeyImpl is the InstanceKey representation indicating a wildcard, which // 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. // while selecting a particular object to move.
// //
// This is a rather special-purpose function here mainly to support our // This is a rather special-purpose function here mainly to support our
// validation rule that a module can only traverse down into child modules // validation rule that a module can only traverse down into child modules.
// that belong to the same module package.
func (e *MoveEndpointInModule) ModuleCallTraversals() (Module, []ModuleCall) { func (e *MoveEndpointInModule) ModuleCallTraversals() (Module, []ModuleCall) {
// We're returning []ModuleCall rather than Module here to make it clearer // We're returning []ModuleCall rather than Module here to make it clearer
// that this is a relative sequence of calls rather than an absolute // that this is a relative sequence of calls rather than an absolute

View File

@ -50,46 +50,13 @@ func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, declaredInsts
for _, stmt := range stmts { for _, stmt := range stmts {
// Earlier code that constructs MoveStatement values should ensure that // Earlier code that constructs MoveStatement values should ensure that
// both stmt.From and stmt.To always belong to the same statement and // both stmt.From and stmt.To always belong to the same statement.
// thus to the same module. fromMod, _ := stmt.From.ModuleCallTraversals()
stmtMod, fromCallSteps := stmt.From.ModuleCallTraversals()
_, toCallSteps := stmt.To.ModuleCallTraversals()
modCfg := rootCfg.Descendent(stmtMod) for _, fromModInst := range declaredInsts.InstancesForModule(fromMod) {
if !stmt.Implied { absFrom := stmt.From.InModuleInstance(fromModInst)
// 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 _, modInst := range declaredInsts.InstancesForModule(stmtMod) { absTo := stmt.To.InModuleInstance(fromModInst)
absFrom := stmt.From.InModuleInstance(modInst)
absTo := stmt.To.InModuleInstance(modInst)
if addrs.Equivalent(absFrom, absTo) { if addrs.Equivalent(absFrom, absTo) {
diags = diags.Append(&hcl.Diagnostic{ diags = diags.Append(&hcl.Diagnostic{
@ -199,6 +166,7 @@ func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, declaredInsts
), ),
}) })
} }
} }
} }
@ -370,24 +338,3 @@ func movableObjectDeclRange(addr addrs.AbsMoveable, cfg *configs.Config) (tfdiag
panic("unsupported AbsMoveable address type") 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
}

View File

@ -335,7 +335,7 @@ Each resource can have moved from only one source resource.`,
`test.thing`, `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": { "move to resource in another module package": {
Statements: []MoveStatement{ Statements: []MoveStatement{
@ -345,7 +345,7 @@ Each resource can have moved from only one source resource.`,
`module.fake_external.test.thing`, `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": { "move from module call in another module package": {
Statements: []MoveStatement{ Statements: []MoveStatement{
@ -355,7 +355,7 @@ Each resource can have moved from only one source resource.`,
`module.b`, `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": { "move to module call in another module package": {
Statements: []MoveStatement{ Statements: []MoveStatement{
@ -365,7 +365,7 @@ Each resource can have moved from only one source resource.`,
`module.fake_external.module.b`, `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": { "implied move from resource in another module package": {
Statements: []MoveStatement{ Statements: []MoveStatement{

View File

@ -400,12 +400,6 @@ assumes that all three of these modules are maintained by the same people
and distributed together in a single and distributed together in a single
[module package](/language/modules/sources#modules-in-package-sub-directories). [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 Terraform resolves module references in `moved` blocks relative to the module
instance they are defined in. For example, if the original module above were instance they are defined in. For example, if the original module above were
already a child module named `module.original`, the reference to already a child module named `module.original`, the reference to