From d97ef10bb8cc01ab6975590ea6a410a6f586a64e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 23 Sep 2021 11:27:44 -0700 Subject: [PATCH] core: Don't return other errors if move statements are invalid Because our validation rules depend on some dynamic results produced by actually running the plan, we deal with moves in a "backwards" order where we try to apply them first -- ignoring anything strange we might find -- and then validate the original statements only after planning. An unfortunate consequence of that approach is that when the move statements are invalid it's likely that move execution will not fully complete, and so the generated plan is likely to be incorrect and might well include errors resulting from the unresolved moves. To mitigate that, here we let any move validation errors supersede all other diagnostics that the plan phase might've generated, in the hope that it'll help the user focus on fixing the incorrect move statements without creating confusing by reporting errors that only appeared as a quick of how Terraform worked around the invalid move statements earlier. --- internal/terraform/context_plan.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 084a8ab8a..001a2507a 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -417,7 +417,18 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r }) diags = diags.Append(walker.NonFatalDiagnostics) diags = diags.Append(walkDiags) - diags = diags.Append(c.postPlanValidateMoves(config, moveStmts, walker.InstanceExpander.AllInstances())) + moveValidateDiags := c.postPlanValidateMoves(config, moveStmts, walker.InstanceExpander.AllInstances()) + if moveValidateDiags.HasErrors() { + // If any of the move statements are invalid then those errors take + // precedence over any other errors because an incomplete move graph + // is quite likely to be the _cause_ of various errors. This oddity + // comes from the fact that we need to apply the moves before we + // actually validate them, because validation depends on the result + // of first trying to plan. + return nil, moveValidateDiags + } + diags = diags.Append(moveValidateDiags) // might just contain warnings + if len(moveResults.Blocked) > 0 && !diags.HasErrors() { // If we had blocked moves and we're not going to be returning errors // then we'll report the blockers as a warning. We do this only in the