From 3acb5e284171a136c12be6e2cb19f1da50867ee4 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Mon, 21 Jun 2021 10:53:16 -0400 Subject: [PATCH] configs: add decodeMovedBlock behind a locked gate. (#28973) 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). --- internal/addrs/module.go | 4 + internal/addrs/module_instance.go | 4 + internal/addrs/resource.go | 12 ++ internal/addrs/targetable.go | 14 ++ internal/configs/module.go | 2 + internal/configs/moved.go | 70 +++++++ internal/configs/moved_test.go | 184 ++++++++++++++++++ internal/configs/parser_config.go | 17 ++ .../invalid-files/everything-is-a-plan.tf | 11 ++ internal/experiments/experiment.go | 1 + 10 files changed, 319 insertions(+) create mode 100644 internal/configs/moved.go create mode 100644 internal/configs/moved_test.go create mode 100644 internal/configs/testdata/invalid-files/everything-is-a-plan.tf diff --git a/internal/addrs/module.go b/internal/addrs/module.go index 18f3dbec8e..80f394ad75 100644 --- a/internal/addrs/module.go +++ b/internal/addrs/module.go @@ -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 { diff --git a/internal/addrs/module_instance.go b/internal/addrs/module_instance.go index adde944169..5162fb3c17 100644 --- a/internal/addrs/module_instance.go +++ b/internal/addrs/module_instance.go @@ -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 } diff --git a/internal/addrs/resource.go b/internal/addrs/resource.go index 8aa0efc05e..d34fec97cd 100644 --- a/internal/addrs/resource.go +++ b/internal/addrs/resource.go @@ -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() @@ -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() @@ -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() diff --git a/internal/addrs/targetable.go b/internal/addrs/targetable.go index 16819a5afb..1aa3ef1ff7 100644 --- a/internal/addrs/targetable.go +++ b/internal/addrs/targetable.go @@ -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. @@ -24,3 +28,13 @@ type targetable struct { func (r targetable) targetableSigil() { } + +type TargetableAddrType int + +const ( + ConfigResourceAddrType TargetableAddrType = iota + AbsResourceInstanceAddrType + AbsResourceAddrType + ModuleAddrType + ModuleInstanceAddrType +) diff --git a/internal/configs/module.go b/internal/configs/module.go index c6177f07fc..9a5625ac77 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -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 diff --git a/internal/configs/moved.go b/internal/configs/moved.go new file mode 100644 index 0000000000..891636a73c --- /dev/null +++ b/internal/configs/moved.go @@ -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, + }, + }, +} diff --git a/internal/configs/moved_test.go b/internal/configs/moved_test.go new file mode 100644 index 0000000000..c662dae953 --- /dev/null +++ b/internal/configs/moved_test.go @@ -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 +} diff --git a/internal/configs/parser_config.go b/internal/configs/parser_config.go index ec66b4cc54..6d1a701a09 100644 --- a/internal/configs/parser_config.go +++ b/internal/configs/parser_config.go @@ -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 @@ -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. @@ -235,6 +249,9 @@ var configFileSchema = &hcl.BodySchema{ Type: "data", LabelNames: []string{"type", "name"}, }, + { + Type: "moved", + }, }, } diff --git a/internal/configs/testdata/invalid-files/everything-is-a-plan.tf b/internal/configs/testdata/invalid-files/everything-is-a-plan.tf new file mode 100644 index 0000000000..e66766bea6 --- /dev/null +++ b/internal/configs/testdata/invalid-files/everything-is-a-plan.tf @@ -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 +} \ No newline at end of file diff --git a/internal/experiments/experiment.go b/internal/experiments/experiment.go index ee73ca2333..d7fd8a58c7 100644 --- a/internal/experiments/experiment.go +++ b/internal/experiments/experiment.go @@ -16,6 +16,7 @@ const ( VariableValidation = Experiment("variable_validation") ModuleVariableOptionalAttrs = Experiment("module_variable_optional_attrs") SuppressProviderSensitiveAttrs = Experiment("provider_sensitive_attrs") + EverythingIsAPlan = Experiment("everything_is_a_plan") ) func init() {