From 1b464e1e9ac1a0f799074bd981d33fe2abf78abf Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 5 Apr 2021 17:05:57 -0700 Subject: [PATCH] core: Minimal initial implementation of "refresh only" planning mode This only includes the core mechanisms to make it work. There's not yet any way to turn this mode on as an end-user, because we have to do some more work at the UI layer to present this well before we could include it as an end-user-visible feature in a release. 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. --- plans/plan.go | 8 +++ terraform/context.go | 74 ++++++++++++++++++++++++ terraform/context_graph_type.go | 12 ++-- terraform/context_plan2_test.go | 71 +++++++++++++++++++++++ terraform/graph_builder_plan.go | 8 +++ terraform/graphtype_string.go | 11 ++-- terraform/node_resource_plan.go | 10 ++++ terraform/node_resource_plan_instance.go | 67 +++++++++++---------- terraform/node_resource_plan_orphan.go | 47 ++++++++------- 9 files changed, 249 insertions(+), 59 deletions(-) diff --git a/plans/plan.go b/plans/plan.go index 92778e1e7..d3f1145fb 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 40688e872..ffa89c1d3 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,6 +576,8 @@ 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("unsupported plan mode %s", c.planMode)) } @@ -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 aefa17f77..658779e6a 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/context_plan2_test.go b/terraform/context_plan2_test.go index 312e971be..20faac0d5 100644 --- a/terraform/context_plan2_test.go +++ b/terraform/context_plan2_test.go @@ -1,10 +1,12 @@ package terraform import ( + "bytes" "errors" "strings" "testing" + "github.com/davecgh/go-spew/spew" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/plans" @@ -487,6 +489,75 @@ provider "test" { } } +func TestContext2Plan_refreshOnlyMode(t *testing.T) { + addr := mustResourceInstanceAddr("test_object.a") + + p := simpleMockProvider() + + // The configuration, the prior state, and the refresh result intentionally + // have different values for "test_string" so we can observe that the + // refresh took effect but the configuration change wasn't considered. + m := testModuleInline(t, map[string]string{ + "main.tf": ` + resource "test_object" "a" { + test_string = "after" + } + `, + }) + state := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(addr, &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"test_string":"before"}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`)) + }) + p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse { + newVal, err := cty.Transform(req.PriorState, func(path cty.Path, v cty.Value) (cty.Value, error) { + if len(path) == 1 && path[0] == (cty.GetAttrStep{Name: "test_string"}) { + return cty.StringVal("current"), nil + } + return v, nil + }) + if err != nil { + // shouldn't get here + t.Fatalf("ReadResourceFn transform failed") + return providers.ReadResourceResponse{} + } + return providers.ReadResourceResponse{ + NewState: newVal, + } + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + State: state, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + PlanMode: plans.RefreshOnlyMode, + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("unexpected errors\n%s", diags.Err().Error()) + } + + if got, want := len(plan.Changes.Resources), 0; got != want { + t.Fatalf("plan contains resource changes; want none\n%s", spew.Sdump(plan.Changes.Resources)) + } + + state = plan.State + instState := state.ResourceInstance(addr) + if instState == nil { + t.Fatalf("%s has no state at all after plan", addr) + } + if instState.Current == nil { + t.Fatalf("%s has no current object after plan", addr) + } + if got, want := instState.Current.AttrsJSON, `"current"`; !bytes.Contains(got, []byte(want)) { + t.Fatalf("%s has wrong prior state after plan\ngot:\n%s\n\nwant substring: %s", addr, got, want) + } +} + func TestContext2Plan_invalidSensitiveModuleOutput(t *testing.T) { m := testModuleInline(t, map[string]string{ "child/main.tf": ` diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index d9e92606b..ff03cccea 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 2d4026e46..ac605ccab 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 15e97bc36..a7ba7650c 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 e50a5cfcf..5664e4ef6 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 ( @@ -96,7 +102,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) addr := n.ResourceInstanceAddr() var change *plans.ResourceInstanceChange - var instancePlanState *states.ResourceInstanceObject + var instanceRefreshState *states.ResourceInstanceObject _, providerSchema, err := getProvider(ctx, n.ResolvedProvider) diags = diags.Append(err) @@ -150,38 +156,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 - } - - 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)) + // 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.checkPreventDestroy(change)) + if diags.HasErrors() { + return diags + } + + 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 94a4f9235..37a7a6a14 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -14,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 ( @@ -75,9 +80,7 @@ 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 - - state, readDiags := n.readResourceInstanceState(ctx, addr) + oldState, readDiags := n.readResourceInstanceState(ctx, addr) diags = diags.Append(readDiags) if diags.HasErrors() { return diags @@ -90,34 +93,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.writeChange(ctx, change, "")) + if diags.HasErrors() { + return diags + } + + diags = diags.Append(n.writeResourceInstanceState(ctx, nil, workingState)) } - 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.writeResourceInstanceState(ctx, nil, workingState)) return diags }