From 0bd3417e729955d0950b6446c045afb8043d2fc4 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 5 Apr 2021 17:05:57 -0700 Subject: [PATCH] command+core: Minimal initial implementation of -refresh-only mode This only includes the internal mechanisms to make it work, and not any of the necessary UI changes to "terraform plan" and "terraform apply" to make their output make sense in this mode. At the lowest level of abstraction inside the graph nodes themselves, this effectively mirrors the existing option to disable refreshing with a new option to disable change-planning, so that either "half" of the process can be disabled. As far as the nodes are concerned it would be possible in principle to disable _both_, but the higher-level representation of these modes prevents that combination from reaching Terraform Core in practice, because we block using -refresh-only and -refresh=false at the same time. --- command/arguments/extended.go | 21 ++++++- plans/plan.go | 8 +++ terraform/context.go | 76 +++++++++++++++++++++++- terraform/context_graph_type.go | 12 ++-- terraform/graph_builder_plan.go | 8 +++ terraform/graphtype_string.go | 11 ++-- terraform/node_resource_plan.go | 10 ++++ terraform/node_resource_plan_instance.go | 62 ++++++++++--------- terraform/node_resource_plan_orphan.go | 44 ++++++++------ 9 files changed, 193 insertions(+), 59 deletions(-) diff --git a/command/arguments/extended.go b/command/arguments/extended.go index 000704a38c70..03149b460b98 100644 --- a/command/arguments/extended.go +++ b/command/arguments/extended.go @@ -66,8 +66,9 @@ type Operation struct { // These private fields are used only temporarily during decoding. Use // method Parse to populate the exported fields from these, validating // the raw values in the process. - targetsRaw []string - destroyRaw bool + targetsRaw []string + destroyRaw bool + refreshOnlyRaw bool } // Parse must be called on Operation after initial flag parse. This processes @@ -105,8 +106,23 @@ func (o *Operation) Parse() tfdiags.Diagnostics { // If you add a new possible value for o.PlanMode here, consider also // adding a specialized error message for it in ParseApplyDestroy. switch { + case o.destroyRaw && o.refreshOnlyRaw: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Incompatible plan mode options", + "The -destroy and -refresh-only options are mutually-exclusive.", + )) case o.destroyRaw: o.PlanMode = plans.DestroyMode + case o.refreshOnlyRaw: + o.PlanMode = plans.RefreshOnlyMode + if !o.Refresh { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Incompatible refresh options", + "It doesn't make sense to use -refresh-only at the same time as -refresh=false, because Terraform would have nothing to do.", + )) + } default: o.PlanMode = plans.NormalMode } @@ -160,6 +176,7 @@ func extendedFlagSet(name string, state *State, operation *Operation, vars *Vars f.IntVar(&operation.Parallelism, "parallelism", DefaultParallelism, "parallelism") f.BoolVar(&operation.Refresh, "refresh", true, "refresh") f.BoolVar(&operation.destroyRaw, "destroy", false, "destroy") + f.BoolVar(&operation.refreshOnlyRaw, "refresh-only", false, "refresh-only") f.Var((*flagStringSlice)(&operation.targetsRaw), "target", "target") } diff --git a/plans/plan.go b/plans/plan.go index 92778e1e7808..d3f1145fb7e9 100644 --- a/plans/plan.go +++ b/plans/plan.go @@ -21,6 +21,14 @@ import ( // since the plan does not itself include all of the information required to // make the changes indicated. type Plan struct { + // Mode is the mode under which this plan was created. + // + // This is only recorded to allow for UI differences when presenting plans + // to the end-user, and so it must not be used to influence apply-time + // behavior. The actions during apply must be described entirely by + // the Changes field, regardless of how the plan was created. + Mode Mode + VariableValues map[string]DynamicValue Changes *Changes TargetAddrs []addrs.Targetable diff --git a/terraform/context.go b/terraform/context.go index 27ce63682074..ffa89c1d353f 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -256,6 +256,17 @@ func NewContext(opts *ContextOpts) (*Context, tfdiags.Diagnostics) { switch opts.PlanMode { case plans.NormalMode, plans.DestroyMode: // OK + case plans.RefreshOnlyMode: + if opts.SkipRefresh { + // The CLI layer (and other similar callers) should prevent this + // combination of options. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Incompatible plan options", + "Cannot skip refreshing in refresh-only mode. This is a bug in Terraform.", + )) + return nil, diags + } default: // The CLI layer (and other similar callers) should not try to // create a context for a mode that Terraform Core doesn't support. @@ -370,6 +381,20 @@ func (c *Context) Graph(typ GraphType, opts *ContextGraphOpts) (*Graph, tfdiags. Validate: opts.Validate, }).Build(addrs.RootModuleInstance) + case GraphTypePlanRefreshOnly: + // Create the plan graph builder, with skipPlanChanges set to + // activate the "refresh only" mode. + return (&PlanGraphBuilder{ + Config: c.config, + State: c.state, + Components: c.components, + Schemas: c.schemas, + Targets: c.targets, + Validate: opts.Validate, + skipRefresh: c.skipRefresh, + skipPlanChanges: true, // this activates "refresh only" mode. + }).Build(addrs.RootModuleInstance) + case GraphTypeEval: return (&EvalGraphBuilder{ Config: c.config, @@ -551,8 +576,10 @@ The -target option is not for routine use, and is provided only for exceptional plan, planDiags = c.plan() case plans.DestroyMode: plan, planDiags = c.destroyPlan() + case plans.RefreshOnlyMode: + plan, planDiags = c.refreshOnlyPlan() default: - panic(fmt.Sprintf("nsupported plan mode %s", c.planMode)) + panic(fmt.Sprintf("unsupported plan mode %s", c.planMode)) } diags = diags.Append(planDiags) if diags.HasErrors() { @@ -603,6 +630,7 @@ func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) { return nil, diags } plan := &plans.Plan{ + Mode: plans.NormalMode, Changes: c.changes, } @@ -656,10 +684,56 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) { return nil, diags } + destroyPlan.Mode = plans.DestroyMode destroyPlan.Changes = c.changes return destroyPlan, diags } +func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + graph, graphDiags := c.Graph(GraphTypePlanRefreshOnly, nil) + diags = diags.Append(graphDiags) + if graphDiags.HasErrors() { + return nil, diags + } + + // Do the walk + walker, walkDiags := c.walk(graph, walkPlan) + diags = diags.Append(walker.NonFatalDiagnostics) + diags = diags.Append(walkDiags) + if walkDiags.HasErrors() { + return nil, diags + } + plan := &plans.Plan{ + Mode: plans.RefreshOnlyMode, + Changes: c.changes, + } + + // If the graph builder and graph nodes correctly obeyed our directive + // to refresh only, the set of resource changes should always be empty. + // We'll safety-check that here so we can return a clear message about it, + // rather than probably just generating confusing output at the UI layer. + if len(plan.Changes.Resources) != 0 { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid refresh-only plan", + "Terraform generated planned resource changes in a refresh-only plan. This is a bug in Terraform.", + )) + } + + c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects() + + refreshedState := c.refreshState.DeepCopy() + plan.State = refreshedState + + // replace the working state with the updated state, so that immediate calls + // to Apply work as expected. + c.state = refreshedState + + return plan, diags +} + // 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. diff --git a/terraform/context_graph_type.go b/terraform/context_graph_type.go index aefa17f77b03..658779e6ae39 100644 --- a/terraform/context_graph_type.go +++ b/terraform/context_graph_type.go @@ -11,6 +11,7 @@ const ( GraphTypeInvalid GraphType = iota GraphTypePlan GraphTypePlanDestroy + GraphTypePlanRefreshOnly GraphTypeApply GraphTypeValidate GraphTypeEval // only visits in-memory elements such as variables, locals, and outputs. @@ -20,9 +21,10 @@ const ( // is useful to use as the mechanism for human input for configurable // graph types. var GraphTypeMap = map[string]GraphType{ - "apply": GraphTypeApply, - "plan": GraphTypePlan, - "plan-destroy": GraphTypePlanDestroy, - "validate": GraphTypeValidate, - "eval": GraphTypeEval, + "apply": GraphTypeApply, + "plan": GraphTypePlan, + "plan-destroy": GraphTypePlanDestroy, + "plan-refresh-only": GraphTypePlanRefreshOnly, + "validate": GraphTypeValidate, + "eval": GraphTypeEval, } diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index d9e92606b0c2..ff03cccea441 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -45,6 +45,12 @@ type PlanGraphBuilder struct { // skipRefresh indicates that we should skip refreshing managed resources skipRefresh bool + // skipPlanChanges indicates that we should skip the step of comparing + // prior state with configuration and generating planned changes to + // resource instances. (This is for the "refresh only" planning mode, + // where we _only_ do the refresh step.) + skipPlanChanges bool + // CustomConcrete can be set to customize the node types created // for various parts of the plan. This is useful in order to customize // the plan behavior. @@ -181,6 +187,7 @@ func (b *PlanGraphBuilder) init() { return &nodeExpandPlannableResource{ NodeAbstractResource: a, skipRefresh: b.skipRefresh, + skipPlanChanges: b.skipPlanChanges, } } @@ -188,6 +195,7 @@ func (b *PlanGraphBuilder) init() { return &NodePlannableResourceInstanceOrphan{ NodeAbstractResourceInstance: a, skipRefresh: b.skipRefresh, + skipPlanChanges: b.skipPlanChanges, } } } diff --git a/terraform/graphtype_string.go b/terraform/graphtype_string.go index 2d4026e46865..ac605ccab3a6 100644 --- a/terraform/graphtype_string.go +++ b/terraform/graphtype_string.go @@ -11,14 +11,15 @@ func _() { _ = x[GraphTypeInvalid-0] _ = x[GraphTypePlan-1] _ = x[GraphTypePlanDestroy-2] - _ = x[GraphTypeApply-3] - _ = x[GraphTypeValidate-4] - _ = x[GraphTypeEval-5] + _ = x[GraphTypePlanRefreshOnly-3] + _ = x[GraphTypeApply-4] + _ = x[GraphTypeValidate-5] + _ = x[GraphTypeEval-6] } -const _GraphType_name = "GraphTypeInvalidGraphTypePlanGraphTypePlanDestroyGraphTypeApplyGraphTypeValidateGraphTypeEval" +const _GraphType_name = "GraphTypeInvalidGraphTypePlanGraphTypePlanDestroyGraphTypePlanRefreshOnlyGraphTypeApplyGraphTypeValidateGraphTypeEval" -var _GraphType_index = [...]uint8{0, 16, 29, 49, 63, 80, 93} +var _GraphType_index = [...]uint8{0, 16, 29, 49, 73, 87, 104, 117} func (i GraphType) String() string { if i >= GraphType(len(_GraphType_index)-1) { diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 15e97bc36d18..a7ba7650cfc4 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -24,6 +24,10 @@ type nodeExpandPlannableResource struct { // skipRefresh indicates that we should skip refreshing individual instances skipRefresh bool + // skipPlanChanges indicates we should skip trying to plan change actions + // for any instances. + skipPlanChanges bool + // We attach dependencies to the Resource during refresh, since the // instances are instantiated during DynamicExpand. dependencies []addrs.ConfigResource @@ -84,6 +88,7 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, er ForceCreateBeforeDestroy: n.ForceCreateBeforeDestroy, dependencies: n.dependencies, skipRefresh: n.skipRefresh, + skipPlanChanges: n.skipPlanChanges, }) } @@ -149,6 +154,10 @@ type NodePlannableResource struct { // skipRefresh indicates that we should skip refreshing individual instances skipRefresh bool + // skipPlanChanges indicates we should skip trying to plan change actions + // for any instances. + skipPlanChanges bool + dependencies []addrs.ConfigResource } @@ -239,6 +248,7 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // nodes that have it. ForceCreateBeforeDestroy: n.CreateBeforeDestroy(), skipRefresh: n.skipRefresh, + skipPlanChanges: n.skipPlanChanges, } } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index a9582437709c..d8e2f75c8730 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -18,7 +18,13 @@ import ( type NodePlannableResourceInstance struct { *NodeAbstractResourceInstance ForceCreateBeforeDestroy bool - skipRefresh bool + + // skipRefresh indicates that we should skip refreshing individual instances + skipRefresh bool + + // skipPlanChanges indicates we should skip trying to plan change actions + // for any instances. + skipPlanChanges bool } var ( @@ -98,7 +104,6 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) var change *plans.ResourceInstanceChange var instanceRefreshState *states.ResourceInstanceObject - var instancePlanState *states.ResourceInstanceObject _, providerSchema, err := getProvider(ctx, n.ResolvedProvider) diags = diags.Append(err) @@ -152,38 +157,41 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) } } - // Plan the instance - change, instancePlanState, planDiags := n.plan(ctx, change, instanceRefreshState, n.ForceCreateBeforeDestroy) - diags = diags.Append(planDiags) - if diags.HasErrors() { - return diags - } - - diags = diags.Append(n.checkPreventDestroy(change)) - if diags.HasErrors() { - return diags - } + // Plan the instance, unless we're in the refresh-only mode + if !n.skipPlanChanges { + change, instancePlanState, planDiags := n.plan(ctx, change, instanceRefreshState, n.ForceCreateBeforeDestroy) + diags = diags.Append(planDiags) + if diags.HasErrors() { + return diags + } - diags = diags.Append(n.writeResourceInstanceState(ctx, instancePlanState, workingState)) - if diags.HasErrors() { - return diags - } + diags = diags.Append(n.checkPreventDestroy(change)) + if diags.HasErrors() { + return diags + } - // If this plan resulted in a NoOp, then apply won't have a chance to make - // any changes to the stored dependencies. Since this is a NoOp we know - // that the stored dependencies will have no effect during apply, and we can - // write them out now. - if change.Action == plans.NoOp && !depsEqual(instanceRefreshState.Dependencies, n.Dependencies) { - // the refresh state will be the final state for this resource, so - // finalize the dependencies here if they need to be updated. - instanceRefreshState.Dependencies = n.Dependencies - diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, refreshState)) + diags = diags.Append(n.writeResourceInstanceState(ctx, instancePlanState, workingState)) if diags.HasErrors() { return diags } + + // If this plan resulted in a NoOp, then apply won't have a chance to make + // any changes to the stored dependencies. Since this is a NoOp we know + // that the stored dependencies will have no effect during apply, and we can + // write them out now. + if change.Action == plans.NoOp && !depsEqual(instanceRefreshState.Dependencies, n.Dependencies) { + // the refresh state will be the final state for this resource, so + // finalize the dependencies here if they need to be updated. + instanceRefreshState.Dependencies = n.Dependencies + diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, refreshState)) + if diags.HasErrors() { + return diags + } + } + + diags = diags.Append(n.writeChange(ctx, change, "")) } - diags = diags.Append(n.writeChange(ctx, change, "")) return diags } diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index 77cdffb161c9..8e9f70865095 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -6,7 +6,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -15,7 +14,12 @@ import ( type NodePlannableResourceInstanceOrphan struct { *NodeAbstractResourceInstance + // skipRefresh indicates that we should skip refreshing individual instances skipRefresh bool + + // skipPlanChanges indicates we should skip trying to plan change actions + // for any instances. + skipPlanChanges bool } var ( @@ -76,11 +80,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon // Declare a bunch of variables that are used for state during // evaluation. These are written to by-address below. - var change *plans.ResourceInstanceChange - var state *states.ResourceInstanceObject var err error - state, err = n.readResourceInstanceState(ctx, addr) + oldState, err := n.readResourceInstanceState(ctx, addr) diags = diags.Append(err) if diags.HasErrors() { return diags @@ -93,34 +95,38 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon // plan before apply, and may not handle a missing resource during // Delete correctly. If this is a simple refresh, Terraform is // expected to remove the missing resource from the state entirely - state, refreshDiags := n.refresh(ctx, state) + refreshedState, refreshDiags := n.refresh(ctx, oldState) diags = diags.Append(refreshDiags) if diags.HasErrors() { return diags } - diags = diags.Append(n.writeResourceInstanceState(ctx, state, refreshState)) + diags = diags.Append(n.writeResourceInstanceState(ctx, refreshedState, refreshState)) if diags.HasErrors() { return diags } } - change, destroyPlanDiags := n.planDestroy(ctx, state, "") - diags = diags.Append(destroyPlanDiags) - if diags.HasErrors() { - return diags - } + if !n.skipPlanChanges { + var change *plans.ResourceInstanceChange + change, destroyPlanDiags := n.planDestroy(ctx, oldState, "") + diags = diags.Append(destroyPlanDiags) + if diags.HasErrors() { + return diags + } - diags = diags.Append(n.checkPreventDestroy(change)) - if diags.HasErrors() { - return diags - } + diags = diags.Append(n.checkPreventDestroy(change)) + if diags.HasErrors() { + return diags + } - diags = diags.Append(n.writeChange(ctx, change, "")) - if diags.HasErrors() { - return diags + diags = diags.Append(n.writeChange(ctx, change, "")) + if diags.HasErrors() { + return diags + } + + diags = diags.Append(n.writeResourceInstanceState(ctx, nil, workingState)) } - diags = diags.Append(n.writeResourceInstanceState(ctx, nil, workingState)) return diags }