configs: For Moved blocks, use addrs.MoveEndpoint instead of addrs.Target

Although addrs.Target can in principle capture the information we need to
represent move endpoints, it's semantically confusing because
addrs.Targetable uses addrs.Abs... types which are typically for absolute
addresses, but we were using them for relative addresses here.

We now have specialized address types for representing moves and probably
other things which have similar requirements later on. These types
largely communicate the same information in the end, but aim to do so in
a way that's explicit about which addresses are relative and which are
absolute, to make it less likely that we'd inadvertently misuse these
addresses.
This commit is contained in:
Martin Atkins 2021-06-30 09:22:02 -07:00
parent 4cbe6cabfc
commit 708003b035
3 changed files with 53 additions and 21 deletions

View File

@ -47,6 +47,36 @@ func (e *MoveEndpoint) String() string {
return e.relSubject.String()
}
func (e *MoveEndpoint) Equal(other *MoveEndpoint) bool {
switch {
case (e == nil) != (other == nil):
return false
case e == nil:
return true
default:
// Since we only use ModuleInstance and AbsResourceInstance in our
// string representation, we have no ambiguity between address types
// and can safely just compare the string representations to
// compare the relSubject values.
return e.String() == other.String() && e.SourceRange == other.SourceRange
}
}
// MightUnifyWith returns true if it is possible that a later call to
// UnifyMoveEndpoints might succeed if given the reciever and the other
// given endpoint.
//
// This is intended for early static validation of obviously-wrong situations,
// although there are still various semantic errors that this cannot catch.
func (e *MoveEndpoint) MightUnifyWith(other *MoveEndpoint) bool {
// For our purposes here we'll just do a unify without a base module
// address, because the rules for whether unify can succeed depend
// only on the relative part of the addresses, not on which module
// they were declared in.
from, to := UnifyMoveEndpoints(RootModuleInstance, e, other)
return from != nil && to != nil
}
// ConfigMovable transforms the reciever into a ConfigMovable by resolving it
// relative to the given base module, which should be the module where
// the MoveEndpoint expression was found.

View File

@ -6,8 +6,8 @@ import (
)
type Moved struct {
From *addrs.Target
To *addrs.Target
From *addrs.MoveEndpoint
To *addrs.MoveEndpoint
DeclRange hcl.Range
}
@ -25,7 +25,7 @@ func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) {
from, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr)
diags = append(diags, traversalDiags...)
if !traversalDiags.HasErrors() {
from, fromDiags := addrs.ParseTarget(from)
from, fromDiags := addrs.ParseMoveEndpoint(from)
diags = append(diags, fromDiags.ToHCL()...)
moved.From = from
}
@ -35,7 +35,7 @@ func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) {
to, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr)
diags = append(diags, traversalDiags...)
if !traversalDiags.HasErrors() {
to, toDiags := addrs.ParseTarget(to)
to, toDiags := addrs.ParseMoveEndpoint(to)
diags = append(diags, toDiags.ToHCL()...)
moved.To = to
}
@ -43,11 +43,13 @@ func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) {
// we can only move from a module to a module, resource to resource, etc.
if !diags.HasErrors() {
if moved.To.Subject.AddrType() != moved.From.Subject.AddrType() {
if !moved.From.MightUnifyWith(moved.To) {
// We can catch some obviously-wrong combinations early here,
// but we still have other dynamic validation to do at runtime.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid \"moved\" targets",
Detail: "The \"from\" and \"to\" targets must be the same address type",
Summary: "Invalid \"moved\" addresses",
Detail: "The \"from\" and \"to\" addresses must either both refer to resources or both refer to modules.",
Subject: &moved.DeclRange,
})
}

View File

@ -48,8 +48,8 @@ func TestDecodeMovedBlock(t *testing.T) {
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(foo_expr),
To: mustTargetFromExpr(bar_expr),
From: mustMoveEndpointFromExpr(foo_expr),
To: mustMoveEndpointFromExpr(bar_expr),
DeclRange: blockRange,
},
``,
@ -72,8 +72,8 @@ func TestDecodeMovedBlock(t *testing.T) {
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(foo_index_expr),
To: mustTargetFromExpr(bar_index_expr),
From: mustMoveEndpointFromExpr(foo_index_expr),
To: mustMoveEndpointFromExpr(bar_index_expr),
DeclRange: blockRange,
},
``,
@ -96,8 +96,8 @@ func TestDecodeMovedBlock(t *testing.T) {
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(mod_foo_expr),
To: mustTargetFromExpr(mod_bar_expr),
From: mustMoveEndpointFromExpr(mod_foo_expr),
To: mustMoveEndpointFromExpr(mod_bar_expr),
DeclRange: blockRange,
},
``,
@ -116,7 +116,7 @@ func TestDecodeMovedBlock(t *testing.T) {
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(foo_expr),
From: mustMoveEndpointFromExpr(foo_expr),
DeclRange: blockRange,
},
"Missing required argument",
@ -139,11 +139,11 @@ func TestDecodeMovedBlock(t *testing.T) {
DefRange: blockRange,
},
&Moved{
To: mustTargetFromExpr(foo_expr),
From: mustTargetFromExpr(mod_foo_expr),
To: mustMoveEndpointFromExpr(foo_expr),
From: mustMoveEndpointFromExpr(mod_foo_expr),
DeclRange: blockRange,
},
"Invalid \"moved\" targets",
"Invalid \"moved\" addresses",
},
}
@ -162,23 +162,23 @@ func TestDecodeMovedBlock(t *testing.T) {
t.Fatal("expected error")
}
if !cmp.Equal(got, test.want) {
if !cmp.Equal(got, test.want, cmp.AllowUnexported(addrs.MoveEndpoint{})) {
t.Fatalf("wrong result: %s", cmp.Diff(got, test.want))
}
})
}
}
func mustTargetFromExpr(expr hcl.Expression) *addrs.Target {
func mustMoveEndpointFromExpr(expr hcl.Expression) *addrs.MoveEndpoint {
traversal, hcldiags := hcl.AbsTraversalForExpr(expr)
if hcldiags.HasErrors() {
panic(hcldiags.Errs())
}
target, diags := addrs.ParseTarget(traversal)
ep, diags := addrs.ParseMoveEndpoint(traversal)
if diags.HasErrors() {
panic(diags.Err())
}
return target
return ep
}