From f76a3467dcfaab6fa7cc25c843ef92cfbf602fea Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 28 Jul 2021 12:10:23 -0700 Subject: [PATCH] core: Call into refactoring.ValidateMoves after creating a plan As of this commit, refactoring.ValidateMoves doesn't actually do anything yet (always returns nil) but the goal here is to wire in the set of all declared instances so that refactoring.ValidateMoves will then have all of the information it needs to encapsulate our validation rules. The actual implementation of refactoring.ValidateMoves will follow in subsequent commits. --- internal/refactoring/move_validate.go | 2 +- internal/terraform/context.go | 72 +++++++++++++++------------ 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/internal/refactoring/move_validate.go b/internal/refactoring/move_validate.go index 40ee5f6543..692a3cc8ba 100644 --- a/internal/refactoring/move_validate.go +++ b/internal/refactoring/move_validate.go @@ -24,7 +24,7 @@ import ( // construct in incorrect plan (because it'll be starting from the wrong // prior state) but ValidateMoves will block actually showing that invalid // plan to the user. -func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, expander *instances.Expander) tfdiags.Diagnostics { +func ValidateMoves(stmts []MoveStatement, rootCfg *configs.Config, declaredInsts instances.Set) tfdiags.Diagnostics { var diags tfdiags.Diagnostics g := buildMoveStatementGraph(stmts) diff --git a/internal/terraform/context.go b/internal/terraform/context.go index 3ae479eed3..0a092c33b0 100644 --- a/internal/terraform/context.go +++ b/internal/terraform/context.go @@ -609,33 +609,6 @@ The -target option is not for routine use, and is provided only for exceptional )) } - moveStmts := refactoring.FindMoveStatements(c.config) - if len(moveStmts) != 0 { - // TEMP: we haven't fully implemented moving yet, so we'll just - // reject it outright for now to reduce confusion. - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Moves are not yet supported", - "There is currently no handling of \"moved\" blocks in the configuration.", - )) - } - moveResults := refactoring.ApplyMoves(moveStmts, c.prevRunState) - if len(c.targets) > 0 { - for _, result := range moveResults { - matchesTarget := false - for _, targetAddr := range c.targets { - if targetAddr.TargetContains(result.From) { - matchesTarget = true - break - } - } - if !matchesTarget { - // TODO: Return an error stating that a targeted plan is - // only valid if it includes this address that was moved. - } - } - } - var plan *plans.Plan var planDiags tfdiags.Diagnostics switch c.planMode { @@ -653,10 +626,6 @@ The -target option is not for routine use, and is provided only for exceptional return nil, diags } - // TODO: Call refactoring.ValidateMoves, but need to figure out how to - // get hold of the plan's expander here, or somehow otherwise export - // the necessary subset of its data for ValidateMoves to do its work. - // convert the variables into the format expected for the plan varVals := make(map[string]plans.DynamicValue, len(c.variables)) for k, iv := range c.variables { @@ -687,6 +656,8 @@ The -target option is not for routine use, and is provided only for exceptional func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + moveStmts, _ := c.prePlanFindAndApplyMoves() + graph, graphDiags := c.Graph(GraphTypePlan, nil) diags = diags.Append(graphDiags) if graphDiags.HasErrors() { @@ -716,6 +687,9 @@ func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) { // to Apply work as expected. c.state = refreshedState + // TODO: Record the move results in the plan + diags = diags.Append(c.postPlanValidateMoves(moveStmts, walker.InstanceExpander.AllInstances())) + return plan, diags } @@ -726,6 +700,8 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) { } c.changes = plans.NewChanges() + moveStmts, _ := c.prePlanFindAndApplyMoves() + // A destroy plan starts by running Refresh to read any pending data // sources, and remove missing managed resources. This is required because // a "destroy plan" is only creating delete changes, and is essentially a @@ -778,12 +754,18 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) { destroyPlan.UIMode = plans.DestroyMode destroyPlan.Changes = c.changes + + // TODO: Record the move results in the plan + diags = diags.Append(c.postPlanValidateMoves(moveStmts, walker.InstanceExpander.AllInstances())) + return destroyPlan, diags } func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + moveStmts, _ := c.prePlanFindAndApplyMoves() + graph, graphDiags := c.Graph(GraphTypePlanRefreshOnly, nil) diags = diags.Append(graphDiags) if graphDiags.HasErrors() { @@ -834,9 +816,37 @@ func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) { // mutate c.state = refreshedState + // TODO: Record the move results in the plan + diags = diags.Append(c.postPlanValidateMoves(moveStmts, walker.InstanceExpander.AllInstances())) + return plan, diags } +func (c *Context) prePlanFindAndApplyMoves() ([]refactoring.MoveStatement, map[addrs.UniqueKey]refactoring.MoveResult) { + moveStmts := refactoring.FindMoveStatements(c.config) + moveResults := refactoring.ApplyMoves(moveStmts, c.prevRunState) + if len(c.targets) > 0 { + for _, result := range moveResults { + matchesTarget := false + for _, targetAddr := range c.targets { + if targetAddr.TargetContains(result.From) { + matchesTarget = true + break + } + } + if !matchesTarget { + // TODO: Return an error stating that a targeted plan is + // only valid if it includes this address that was moved. + } + } + } + return moveStmts, moveResults +} + +func (c *Context) postPlanValidateMoves(stmts []refactoring.MoveStatement, allInsts instances.Set) tfdiags.Diagnostics { + return refactoring.ValidateMoves(stmts, c.config, allInsts) +} + // Refresh goes through all the resources in the state and refreshes them // to their latest state. This is done by executing a plan, and retaining the // state while discarding the change set.