refactoring: Implied move statements can be cross-package
Terraform uses "implied" move statements to represent the situation where it automatically handles a switch from count to no-count on a resource. Because that situation requires targeting only a specific resource instance inside a specific module instance, implied move statements are always presented as if they had been declared in the root module and then traversed through the exact module instance path to reach the target resource. However, that means they can potentially cross a module package boundary, if the changed resource belongs to an external module. Normally we prohibit that to avoid the root module depending on implementation details of the called module, but Terraform generates these implied statements based only on information in the called module and so there's no need to apply that same restriction to implied move statements, which will always have source and destination addresses belonging to the same module instance. This change therefore fixes a misbehavior where Terraform would reject an attempt to switch from no-count to count in a called module, where previously the author of the calling configuration had no recourse to fix it because the change has actually happened upstream.
This commit is contained in:
parent
5f61140655
commit
bdc5f152d7
|
@ -55,27 +55,34 @@ func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, declaredInsts
|
||||||
_, toCallSteps := stmt.To.ModuleCallTraversals()
|
_, toCallSteps := stmt.To.ModuleCallTraversals()
|
||||||
|
|
||||||
modCfg := rootCfg.Descendent(stmtMod)
|
modCfg := rootCfg.Descendent(stmtMod)
|
||||||
if pkgAddr := callsThroughModulePackage(modCfg, fromCallSteps); pkgAddr != nil {
|
if !stmt.Implied {
|
||||||
diags = diags.Append(&hcl.Diagnostic{
|
// Implied statements can cross module boundaries because we
|
||||||
Severity: hcl.DiagError,
|
// generate them only for changing instance keys on a single
|
||||||
Summary: "Cross-package move statement",
|
// resource. They happen to be generated _as if_ they were written
|
||||||
Detail: fmt.Sprintf(
|
// in the root module, but the source and destination are always
|
||||||
"This statement declares a move from an object declared in external module package %q. Move statements can be only within a single module package.",
|
// in the same module anyway.
|
||||||
pkgAddr,
|
if pkgAddr := callsThroughModulePackage(modCfg, fromCallSteps); pkgAddr != nil {
|
||||||
),
|
diags = diags.Append(&hcl.Diagnostic{
|
||||||
Subject: stmt.DeclRange.ToHCL().Ptr(),
|
Severity: hcl.DiagError,
|
||||||
})
|
Summary: "Cross-package move statement",
|
||||||
}
|
Detail: fmt.Sprintf(
|
||||||
if pkgAddr := callsThroughModulePackage(modCfg, toCallSteps); pkgAddr != nil {
|
"This statement declares a move from an object declared in external module package %q. Move statements can be only within a single module package.",
|
||||||
diags = diags.Append(&hcl.Diagnostic{
|
pkgAddr,
|
||||||
Severity: hcl.DiagError,
|
),
|
||||||
Summary: "Cross-package move statement",
|
Subject: stmt.DeclRange.ToHCL().Ptr(),
|
||||||
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,
|
if pkgAddr := callsThroughModulePackage(modCfg, toCallSteps); pkgAddr != nil {
|
||||||
),
|
diags = diags.Append(&hcl.Diagnostic{
|
||||||
Subject: stmt.DeclRange.ToHCL().Ptr(),
|
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) {
|
for _, modInst := range declaredInsts.InstancesForModule(stmtMod) {
|
||||||
|
|
|
@ -366,6 +366,50 @@ Each resource can have moved from only one source resource.`,
|
||||||
},
|
},
|
||||||
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: `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.`,
|
||||||
},
|
},
|
||||||
|
"implied move from resource in another module package": {
|
||||||
|
Statements: []MoveStatement{
|
||||||
|
makeTestImpliedMoveStmt(t,
|
||||||
|
``,
|
||||||
|
`module.fake_external.test.thing`,
|
||||||
|
`test.thing`,
|
||||||
|
),
|
||||||
|
},
|
||||||
|
// Implied move statements are not subject to the cross-package restriction
|
||||||
|
WantError: ``,
|
||||||
|
},
|
||||||
|
"implied move to resource in another module package": {
|
||||||
|
Statements: []MoveStatement{
|
||||||
|
makeTestImpliedMoveStmt(t,
|
||||||
|
``,
|
||||||
|
`test.thing`,
|
||||||
|
`module.fake_external.test.thing`,
|
||||||
|
),
|
||||||
|
},
|
||||||
|
// Implied move statements are not subject to the cross-package restriction
|
||||||
|
WantError: ``,
|
||||||
|
},
|
||||||
|
"implied move from module call in another module package": {
|
||||||
|
Statements: []MoveStatement{
|
||||||
|
makeTestImpliedMoveStmt(t,
|
||||||
|
``,
|
||||||
|
`module.fake_external.module.a`,
|
||||||
|
`module.b`,
|
||||||
|
),
|
||||||
|
},
|
||||||
|
// Implied move statements are not subject to the cross-package restriction
|
||||||
|
WantError: ``,
|
||||||
|
},
|
||||||
|
"implied move to module call in another module package": {
|
||||||
|
Statements: []MoveStatement{
|
||||||
|
makeTestImpliedMoveStmt(t,
|
||||||
|
``,
|
||||||
|
`module.a`,
|
||||||
|
`module.fake_external.module.b`,
|
||||||
|
),
|
||||||
|
},
|
||||||
|
// Implied move statements are not subject to the cross-package restriction
|
||||||
|
WantError: ``,
|
||||||
|
},
|
||||||
"move to a call that refers to another module package": {
|
"move to a call that refers to another module package": {
|
||||||
Statements: []MoveStatement{
|
Statements: []MoveStatement{
|
||||||
makeTestMoveStmt(t,
|
makeTestMoveStmt(t,
|
||||||
|
@ -650,6 +694,13 @@ func makeTestMoveStmt(t *testing.T, moduleStr, fromStr, toStr string) MoveStatem
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func makeTestImpliedMoveStmt(t *testing.T, moduleStr, fromStr, toStr string) MoveStatement {
|
||||||
|
t.Helper()
|
||||||
|
ret := makeTestMoveStmt(t, moduleStr, fromStr, toStr)
|
||||||
|
ret.Implied = true
|
||||||
|
return ret
|
||||||
|
}
|
||||||
|
|
||||||
var fakeExternalModuleSource = addrs.ModuleSourceRemote{
|
var fakeExternalModuleSource = addrs.ModuleSourceRemote{
|
||||||
PackageAddr: addrs.ModulePackage("fake-external:///"),
|
PackageAddr: addrs.ModulePackage("fake-external:///"),
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue