Skip to content

Commit

Permalink
configs: add decodeMovedBlock behind a locked gate. (#28973)
Browse files Browse the repository at this point in the history
This PR adds decoding for the upcoming "moved" blocks in configuration. This code is gated behind an experiment called EverythingIsAPlan, but the experiment is not registered as an active experiment, so it will never run (there is a test in place which will fail if the experiment is ever registered).

This also adds a new function to the Targetable interface, AddrType, to simplifying comparing two addrs.Targetable.

There is some validation missing still: this does not (yet) descend into resources to see if the actual resource types are the same (I've put this off in part because we will eventually need the provider schema to verify aliased resources, so I suspect this validation will have to happen later on).
  • Loading branch information
mildwonkey committed Jun 21, 2021
1 parent bb86860 commit 3acb5e2
Show file tree
Hide file tree
Showing 10 changed files with 319 additions and 0 deletions.
4 changes: 4 additions & 0 deletions internal/addrs/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ func (m Module) TargetContains(other Targetable) bool {
}
}

func (m Module) AddrType() TargetableAddrType {
return ModuleAddrType
}

// Child returns the address of a child call in the receiver, identified by the
// given name.
func (m Module) Child(name string) Module {
Expand Down
4 changes: 4 additions & 0 deletions internal/addrs/module_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,10 @@ func (m ModuleInstance) Module() Module {
return ret
}

func (m ModuleInstance) AddrType() TargetableAddrType {
return ModuleInstanceAddrType
}

func (m ModuleInstance) targetableSigil() {
// ModuleInstance is targetable
}
Expand Down
12 changes: 12 additions & 0 deletions internal/addrs/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ func (r AbsResource) TargetContains(other Targetable) bool {
}
}

func (r AbsResource) AddrType() TargetableAddrType {
return AbsResourceAddrType
}

func (r AbsResource) String() string {
if len(r.Module) == 0 {
return r.Resource.String()
Expand Down Expand Up @@ -228,6 +232,10 @@ func (r AbsResourceInstance) TargetContains(other Targetable) bool {
}
}

func (r AbsResourceInstance) AddrType() TargetableAddrType {
return AbsResourceInstanceAddrType
}

func (r AbsResourceInstance) String() string {
if len(r.Module) == 0 {
return r.Resource.String()
Expand Down Expand Up @@ -312,6 +320,10 @@ func (r ConfigResource) TargetContains(other Targetable) bool {
}
}

func (r ConfigResource) AddrType() TargetableAddrType {
return ConfigResourceAddrType
}

func (r ConfigResource) String() string {
if len(r.Module) == 0 {
return r.Resource.String()
Expand Down
14 changes: 14 additions & 0 deletions internal/addrs/targetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ type Targetable interface {
// A targetable address always contains at least itself.
TargetContains(other Targetable) bool

// AddrType returns the address type for comparison with other Targetable
// addresses.
AddrType() TargetableAddrType

// String produces a string representation of the address that could be
// parsed as a HCL traversal and passed to ParseTarget to produce an
// identical result.
Expand All @@ -24,3 +28,13 @@ type targetable struct {

func (r targetable) targetableSigil() {
}

type TargetableAddrType int

const (
ConfigResourceAddrType TargetableAddrType = iota
AbsResourceInstanceAddrType
AbsResourceAddrType
ModuleAddrType
ModuleInstanceAddrType
)
2 changes: 2 additions & 0 deletions internal/configs/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ type File struct {

ManagedResources []*Resource
DataResources []*Resource

Moved []*Moved
}

// NewModule takes a list of primary files and a list of override files and
Expand Down
70 changes: 70 additions & 0 deletions internal/configs/moved.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package configs

import (
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/addrs"
)

type Moved struct {
From *addrs.Target
To *addrs.Target

DeclRange hcl.Range
}

func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) {
var diags hcl.Diagnostics
moved := &Moved{
DeclRange: block.DefRange,
}

content, moreDiags := block.Body.Content(movedBlockSchema)
diags = append(diags, moreDiags...)

if attr, exists := content.Attributes["from"]; exists {
from, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr)
diags = append(diags, traversalDiags...)
if !traversalDiags.HasErrors() {
from, fromDiags := addrs.ParseTarget(from)
diags = append(diags, fromDiags.ToHCL()...)
moved.From = from
}
}

if attr, exists := content.Attributes["to"]; exists {
to, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr)
diags = append(diags, traversalDiags...)
if !traversalDiags.HasErrors() {
to, toDiags := addrs.ParseTarget(to)
diags = append(diags, toDiags.ToHCL()...)
moved.To = to
}
}

// 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() {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid \"moved\" targets",
Detail: "The \"from\" and \"to\" targets must be the same address type",
Subject: &moved.DeclRange,
})
}
}

return moved, diags
}

var movedBlockSchema = &hcl.BodySchema{
Attributes: []hcl.AttributeSchema{
{
Name: "from",
Required: true,
},
{
Name: "to",
Required: true,
},
},
}
184 changes: 184 additions & 0 deletions internal/configs/moved_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
package configs

import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcltest"
"github.com/hashicorp/terraform/internal/addrs"
)

func TestDecodeMovedBlock(t *testing.T) {
blockRange := hcl.Range{
Filename: "mock.tf",
Start: hcl.Pos{Line: 3, Column: 12, Byte: 27},
End: hcl.Pos{Line: 3, Column: 19, Byte: 34},
}

foo_expr := hcltest.MockExprTraversalSrc("test_instance.foo")
bar_expr := hcltest.MockExprTraversalSrc("test_instance.bar")

foo_index_expr := hcltest.MockExprTraversalSrc("test_instance.foo[1]")
bar_index_expr := hcltest.MockExprTraversalSrc("test_instance.bar[\"one\"]")

mod_foo_expr := hcltest.MockExprTraversalSrc("module.foo")
mod_bar_expr := hcltest.MockExprTraversalSrc("module.bar")

tests := map[string]struct {
input *hcl.Block
want *Moved
err string
}{
"success": {
&hcl.Block{
Type: "moved",
Body: hcltest.MockBody(&hcl.BodyContent{
Attributes: hcl.Attributes{
"from": {
Name: "from",
Expr: foo_expr,
},
"to": {
Name: "to",
Expr: bar_expr,
},
},
}),
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(foo_expr),
To: mustTargetFromExpr(bar_expr),
DeclRange: blockRange,
},
``,
},
"indexed resources": {
&hcl.Block{
Type: "moved",
Body: hcltest.MockBody(&hcl.BodyContent{
Attributes: hcl.Attributes{
"from": {
Name: "from",
Expr: foo_index_expr,
},
"to": {
Name: "to",
Expr: bar_index_expr,
},
},
}),
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(foo_index_expr),
To: mustTargetFromExpr(bar_index_expr),
DeclRange: blockRange,
},
``,
},
"modules": {
&hcl.Block{
Type: "moved",
Body: hcltest.MockBody(&hcl.BodyContent{
Attributes: hcl.Attributes{
"from": {
Name: "from",
Expr: mod_foo_expr,
},
"to": {
Name: "to",
Expr: mod_bar_expr,
},
},
}),
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(mod_foo_expr),
To: mustTargetFromExpr(mod_bar_expr),
DeclRange: blockRange,
},
``,
},
"error: missing argument": {
&hcl.Block{
Type: "moved",
Body: hcltest.MockBody(&hcl.BodyContent{
Attributes: hcl.Attributes{
"from": {
Name: "from",
Expr: foo_expr,
},
},
}),
DefRange: blockRange,
},
&Moved{
From: mustTargetFromExpr(foo_expr),
DeclRange: blockRange,
},
"Missing required argument",
},
"error: type mismatch": {
&hcl.Block{
Type: "moved",
Body: hcltest.MockBody(&hcl.BodyContent{
Attributes: hcl.Attributes{
"to": {
Name: "to",
Expr: foo_expr,
},
"from": {
Name: "from",
Expr: mod_foo_expr,
},
},
}),
DefRange: blockRange,
},
&Moved{
To: mustTargetFromExpr(foo_expr),
From: mustTargetFromExpr(mod_foo_expr),
DeclRange: blockRange,
},
"Invalid \"moved\" targets",
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
got, diags := decodeMovedBlock(test.input)

if diags.HasErrors() {
if test.err == "" {
t.Fatalf("unexpected error: %s", diags.Errs())
}
if gotErr := diags[0].Summary; gotErr != test.err {
t.Errorf("wrong error, got %q, want %q", gotErr, test.err)
}
} else if test.err != "" {
t.Fatal("expected error")
}

if !cmp.Equal(got, test.want) {
t.Fatalf("wrong result: %s", cmp.Diff(got, test.want))
}
})
}
}

func mustTargetFromExpr(expr hcl.Expression) *addrs.Target {
traversal, hcldiags := hcl.AbsTraversalForExpr(expr)
if hcldiags.HasErrors() {
panic(hcldiags.Errs())
}

target, diags := addrs.ParseTarget(traversal)
if diags.HasErrors() {
panic(diags.Err())
}

return target
}
17 changes: 17 additions & 0 deletions internal/configs/parser_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package configs

import (
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/experiments"
)

// LoadConfigFile reads the file at the given path and parses it as a config
Expand Down Expand Up @@ -148,6 +149,19 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost
file.DataResources = append(file.DataResources, cfg)
}

case "moved":
// This is not quite the usual usage of the experiments package.
// EverythingIsAPlan is not registered as an active experiment, so
// this block will not be decoded until either the experiment is
// registered, or this check is dropped altogether.
if file.ActiveExperiments.Has(experiments.EverythingIsAPlan) {
cfg, cfgDiags := decodeMovedBlock(block)
diags = append(diags, cfgDiags...)
if cfg != nil {
file.Moved = append(file.Moved, cfg)
}
}

default:
// Should never happen because the above cases should be exhaustive
// for all block type names in our schema.
Expand Down Expand Up @@ -235,6 +249,9 @@ var configFileSchema = &hcl.BodySchema{
Type: "data",
LabelNames: []string{"type", "name"},
},
{
Type: "moved",
},
},
}

Expand Down
11 changes: 11 additions & 0 deletions internal/configs/testdata/invalid-files/everything-is-a-plan.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# experiments.EverythingIsAPlan exists but is not registered as an active (or
# concluded) experiment, so this should fail until the experiment "gate" is
# removed.
terraform {
experiments = [everything_is_a_plan]
}

moved {
from = test_instance.foo
to = test_instance.bar
}

0 comments on commit 3acb5e2

Please sign in to comment.