From b4faa6c9ffd0fd2548c67421d7801df3c3ead8c9 Mon Sep 17 00:00:00 2001 From: Martin Atkins <mart@degeneration.co.uk> Date: Mon, 10 Sep 2018 18:31:23 -0700 Subject: [PATCH] core: start of properly planning changes to attributes --- terraform/context_apply_test.go | 1 + terraform/eval_output.go | 286 +++++++++++++++++++++++---- terraform/graph_builder_apply.go | 6 +- terraform/graph_builder_plan.go | 14 +- terraform/node_output.go | 171 ++++++++++++++-- terraform/transform_orphan_output.go | 60 ------ terraform/transform_output.go | 84 ++++---- 7 files changed, 457 insertions(+), 165 deletions(-) delete mode 100644 terraform/transform_orphan_output.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index be36e0aab0..9e88149450 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -216,6 +216,7 @@ func TestContext2Apply_resourceCountZeroList(t *testing.T) { t.Fatalf("diags: %s", diags.Err()) } + t.Logf(spew.Sdump(state)) actual := strings.TrimSpace(state.String()) expected := strings.TrimSpace(`<no state> Outputs: diff --git a/terraform/eval_output.go b/terraform/eval_output.go index 5feb95b9ef..9e1519bd8d 100644 --- a/terraform/eval_output.go +++ b/terraform/eval_output.go @@ -1,72 +1,286 @@ package terraform import ( + "fmt" "log" - "github.com/hashicorp/hcl2/hcl" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/states" ) -// EvalDeleteOutput is an EvalNode implementation that deletes an output -// from the state. -type EvalDeleteOutput struct { +// EvalReadOutputState is an EvalNode implementation that reads the state +// for a particular output value from the overall state. +type EvalReadOutputState struct { Addr addrs.OutputValue + + // Output, if non-nil, will have the current state for the requested + // output assigned to its referent. + Output **states.OutputValue } // TODO: test -func (n *EvalDeleteOutput) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalReadOutputState) Eval(ctx EvalContext) (interface{}, error) { + addr := n.Addr.Absolute(ctx.Path()) state := ctx.State() - if state == nil { - return nil, nil + + os := state.OutputValue(addr) + if os != nil { + if os.Sensitive { + log.Printf("[TRACE] EvalReadOutputState: %s is sensitive", addr) + } else { + log.Printf("[TRACE] EvalReadOutputState: %s has stored value %#v", addr, os.Value) + } + } else { + log.Printf("[TRACE] EvalReadOutputState: %s is not yet set", addr) } - state.RemoveOutputValue(n.Addr.Absolute(ctx.Path())) + if n.Output != nil { + *n.Output = os + } + + return os, nil +} + +// EvalPlanOutputChange is an EvalNode implementation that computes the +// required change (if any) for an output value. +type EvalPlanOutputChange struct { + Addr addrs.OutputValue + PriorState **states.OutputValue + Config *configs.Output + + // Output, if non-nil, will have the planned change assigned to its + // referent. + Output **plans.OutputChange + + // OutputState, if non-nil, will have written to it a synthetic "planned + // state" for the given output, which describes in as much detail as + // possible the new state we expect for this output after apply. + OutputState **states.OutputValue +} + +// TODO: test +func (n *EvalPlanOutputChange) Eval(ctx EvalContext) (interface{}, error) { + addr := n.Addr.Absolute(ctx.Path()) + config := n.Config + state := *n.PriorState + + log.Printf("[TRACE] EvalPlanOutputChange: %s", addr) + + val, diags := ctx.EvaluateExpr(config.Expr, cty.DynamicPseudoType, nil) + if diags.HasErrors() { + return nil, diags.Err() + } + + change := &plans.OutputChange{ + Addr: addr, + Sensitive: config.Sensitive, + } + + eqV := val.Equals(state.Value) + if !eqV.IsKnown() || eqV.False() { + change.After = val + switch { + case state == nil: + change.Action = plans.Create + change.Before = cty.NullVal(cty.DynamicPseudoType) + default: + change.Action = plans.Update + change.Before = state.Value + if state.Sensitive { + change.Sensitive = true // change is sensitive if either Before or After are sensitive + } + } + } else { + change.Action = plans.NoOp + change.After = state.Value + } + + log.Printf("[TRACE] EvalPlanOutputChange: %s has change action %s", addr, change.Action) + + if n.Output != nil { + *n.Output = change + } + + if n.OutputState != nil { + *n.OutputState = &states.OutputValue{ + Value: change.After, + Sensitive: config.Sensitive, + } + } + + return change, diags.ErrWithWarnings() +} + +// EvalPlanOutputDestroy is an EvalNode implementation that produces a +// destroy change for an output value. This should be used only if the +// output has been entirely removed from configuration. Setting an output +// to null should be handled by EvalPlanOutputChange. +type EvalPlanOutputDestroy struct { + Addr addrs.OutputValue + PriorState **states.OutputValue + + // Output, if non-nil, will have the planned change assigned to its + // referent. + Output **plans.OutputChange + + // OutputState, if non-nil, will have nil written to its referent, to + // represent that there will be no state for this output after the + // change is applied. + OutputState **states.OutputValue +} + +// TODO: test +func (n *EvalPlanOutputDestroy) Eval(ctx EvalContext) (interface{}, error) { + addr := n.Addr.Absolute(ctx.Path()) + state := *n.PriorState + + change := &plans.OutputChange{ + Addr: addr, + Sensitive: state.Sensitive, + Change: plans.Change{ + Action: plans.Delete, + Before: state.Value, + After: cty.NullVal(cty.DynamicPseudoType), + }, + } + + log.Printf("[TRACE] EvalPlanOutputDestroy: %s has change action %s", addr, change.Action) + + if n.Output != nil { + *n.Output = change + } + + if n.OutputState != nil { + *n.OutputState = nil + } + + return change, nil +} + +// EvalWriteOutputChange is an EvalNode implementation that appends a given +// planned output change into the current changeset. +type EvalWriteOutputChange struct { + Change **plans.OutputChange +} + +// TODO: test +func (n *EvalWriteOutputChange) Eval(ctx EvalContext) (interface{}, error) { + change := *n.Change + + src, err := change.Encode() + if err != nil { + // Should never happen, since our given change should always be valid + // having been built previously by EvalPlanOutputChange + return nil, fmt.Errorf("failed to encode %s change for plan: %s", change.Addr, err) + } + + ctx.Changes().AppendOutputChange(src) return nil, nil } -// EvalWriteOutput is an EvalNode implementation that writes the output -// for the given name to the current state. -type EvalWriteOutput struct { - Addr addrs.OutputValue - Sensitive bool - Expr hcl.Expression - // ContinueOnErr allows interpolation to fail during Input - ContinueOnErr bool +// EvalReadOutputChange is an EvalNode implementation that retrieves the +// planned change for the output of the given address from the current +// changeset. +type EvalReadOutputChange struct { + Addr addrs.OutputValue + + Output **plans.OutputChange } // TODO: test -func (n *EvalWriteOutput) Eval(ctx EvalContext) (interface{}, error) { - // This has to run before we have a state lock, since evaluation also - // reads the state - val, diags := ctx.EvaluateExpr(n.Expr, cty.DynamicPseudoType, nil) - // We'll handle errors below, after we have loaded the module. +func (n *EvalReadOutputChange) Eval(ctx EvalContext) (interface{}, error) { + addr := n.Addr.Absolute(ctx.Path()) - state := ctx.State() - if state == nil { - return nil, nil + src := ctx.Changes().GetOutputChange(addr) + oc, err := src.Decode() + if err != nil { + // Should happen only if someone's been tampering with the plan file + // manually, so we don't bother with a "pretty" error. + return nil, fmt.Errorf("failed to decode %s change from plan: %s", addr, err) } + if n.Output != nil { + *n.Output = oc + } + return oc, nil +} + +// EvalOutput is an EvalNode implementation that produces an updated value +// for an output. +type EvalOutput struct { + Addr addrs.OutputValue + Config *configs.Output + + // Output will be assigned the new state for the output value. + Output **states.OutputValue +} + +// TODO: test +func (n *EvalOutput) Eval(ctx EvalContext) (interface{}, error) { addr := n.Addr.Absolute(ctx.Path()) + config := *n.Config - // handling the interpolation error + val, diags := ctx.EvaluateExpr(config.Expr, cty.DynamicPseudoType, nil) if diags.HasErrors() { - if n.ContinueOnErr || flagWarnOutputErrors { - log.Printf("[ERROR] Output interpolation %q failed: %s", n.Addr.Name, diags.Err()) - // if we're continuing, make sure the output is included, and - // marked as unknown. If the evaluator was able to find a type - // for the value in spite of the error then we'll use it. - state.SetOutputValue(addr, cty.UnknownVal(val.Type()), n.Sensitive) - return nil, EvalEarlyExitError{} - } return nil, diags.Err() } - if val.IsKnown() && !val.IsNull() { - state.SetOutputValue(addr, val, n.Sensitive) + var os *states.OutputValue + os = &states.OutputValue{ + Value: val, + Sensitive: config.Sensitive, + } + if os.Sensitive { + log.Printf("[TRACE] EvalOutput: %s is sensitive", addr) } else { - state.RemoveOutputValue(addr) + log.Printf("[TRACE] EvalOutput: %s has new value %#v", addr, os.Value) + } + + if n.Output != nil { + *n.Output = os } + + return os, diags.ErrWithWarnings() +} + +// EvalWriteOutputState is an EvalNode implementation that updates the state +// for a given output. +type EvalWriteOutputState struct { + Addr addrs.OutputValue + State **states.OutputValue +} + +// TODO: test +func (n *EvalWriteOutputState) Eval(ctx EvalContext) (interface{}, error) { + addr := n.Addr.Absolute(ctx.Path()) + os := *n.State + + state := ctx.State() + if state == nil { + return nil, nil + } + + state.SetOutputValue(addr, os.Value, os.Sensitive) + return nil, nil +} + +// EvalDeleteOutput is an EvalNode implementation that deletes an output +// from the state. +type EvalDeleteOutput struct { + Addr addrs.OutputValue +} + +// TODO: test +func (n *EvalDeleteOutput) Eval(ctx EvalContext) (interface{}, error) { + state := ctx.State() + if state == nil { + return nil, nil + } + + state.RemoveOutputValue(n.Addr.Absolute(ctx.Path())) return nil, nil } diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index f009a10a38..e5e10867fb 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -116,7 +116,11 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &LocalTransformer{Config: b.Config}, // Add the outputs - &OutputTransformer{Config: b.Config}, + &OutputTransformer{ + Config: b.Config, + State: b.State, + NewNode: NewOutputApplyNode, + }, // Add module variables &ModuleVariableTransformer{Config: b.Config}, diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 881fa5677b..9b51d77f8c 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -79,8 +79,12 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // Add the local values &LocalTransformer{Config: b.Config}, - // Add the outputs - &OutputTransformer{Config: b.Config}, + // Add nodes for configured outputs and orphan outputs + &OutputTransformer{ + Config: b.Config, + State: b.State, + NewNode: NewOutputPlanNode, + }, // Add orphan resources &OrphanResourceTransformer{ @@ -89,12 +93,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Config: b.Config, }, - // Create orphan output nodes - &OrphanOutputTransformer{ - Config: b.Config, - State: b.State, - }, - // Attach the configuration to any resources &AttachResourceConfigTransformer{Config: b.Config}, diff --git a/terraform/node_output.go b/terraform/node_output.go index bb3d065311..26efbbe7af 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -3,14 +3,157 @@ package terraform import ( "fmt" + "github.com/hashicorp/terraform/plans" + + "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/lang" ) +// NodePlannableOutput represents an output that is "plannable": +// we want to check if the output value has changed and record that change +// in the plan if so. +// +// If the node has no attached configuration, the planned change will be to +// remove the output altogether. +type NodePlannableOutput struct { + Addr addrs.AbsOutputValue + Config *configs.Output // Config is the output in the config +} + +var ( + _ GraphNodeSubPath = (*NodePlannableOutput)(nil) + _ RemovableIfNotTargeted = (*NodePlannableOutput)(nil) + _ GraphNodeTargetDownstream = (*NodePlannableOutput)(nil) + _ GraphNodeReferenceable = (*NodePlannableOutput)(nil) + _ GraphNodeReferencer = (*NodePlannableOutput)(nil) + _ GraphNodeReferenceOutside = (*NodePlannableOutput)(nil) + _ GraphNodeEvalable = (*NodePlannableOutput)(nil) + _ dag.GraphNodeDotter = (*NodePlannableOutput)(nil) +) + +// NewOutputPlanNode constructs a new graph node that will make a plan for +// an output with the given address and configuration. +// +// The configuration may be nil, in which case the node will plan to remove +// the output from the state altogether. +func NewOutputPlanNode(addr addrs.AbsOutputValue, cfg *configs.Output) dag.Vertex { + return &NodePlannableOutput{ + Addr: addr, + Config: cfg, + } +} + +func (n *NodePlannableOutput) Name() string { + return n.Addr.String() +} + +// GraphNodeSubPath +func (n *NodePlannableOutput) Path() addrs.ModuleInstance { + return n.Addr.Module +} + +// RemovableIfNotTargeted +func (n *NodePlannableOutput) RemoveIfNotTargeted() bool { + // We need to add this so that this node will be removed if + // it isn't targeted or a dependency of a target. + return true +} + +// GraphNodeTargetDownstream +func (n *NodePlannableOutput) TargetDownstream(targetedDeps, untargetedDeps *dag.Set) bool { + // If any of the direct dependencies of an output are targeted then + // the output must always be targeted as well, so its value will always + // be up-to-date at the completion of an apply walk. + return true +} + +// GraphNodeReferenceOutside implementation +func (n *NodePlannableOutput) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) { + return referenceOutsideForOutput(n.Addr) +} + +// GraphNodeReferenceable +func (n *NodePlannableOutput) ReferenceableAddrs() []addrs.Referenceable { + return referenceableAddrsForOutput(n.Addr) +} + +// GraphNodeReferencer +func (n *NodePlannableOutput) References() []*addrs.Reference { + return appendResourceDestroyReferences(referencesForOutput(n.Config)) +} + +// GraphNodeEvalable +func (n *NodePlannableOutput) EvalTree() EvalNode { + var state *states.OutputValue + var change *plans.OutputChange + + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalReadOutputState{ + Addr: n.Addr.OutputValue, + Output: &state, + }, + &EvalIf{ + If: func(EvalContext) (bool, error) { + if state == nil && n.Config == nil { + // Nothing to do, then. (Shouldn't have created a graph node at all.) + return false, EvalEarlyExitError{} + } + return n.Config != nil, nil + }, + Then: &EvalPlanOutputChange{ + Addr: n.Addr.OutputValue, + Config: n.Config, + PriorState: &state, + Output: &change, + }, + Else: &EvalPlanOutputDestroy{ + Addr: n.Addr.OutputValue, + PriorState: &state, + Output: &change, + }, + }, + &EvalWriteOutputChange{ + Change: &change, + }, + }, + } +} + +// dag.GraphNodeDotter impl. +func (n *NodePlannableOutput) DotNode(name string, opts *dag.DotOpts) *dag.DotNode { + return &dag.DotNode{ + Name: name, + Attrs: map[string]string{ + "label": n.Name(), + "shape": "note", + }, + } +} + +// NewOutputApplyNode constructs a new graph node that will update the state +// for an output with the given address and configuration. +// +// The configuration may be nil, in which case the node will remove the output +// from the state altogether. +func NewOutputApplyNode(addr addrs.AbsOutputValue, cfg *configs.Output) dag.Vertex { + if cfg == nil { + return &NodeDestroyableOutput{ + Addr: addr, + } + } + return &NodeApplyableOutput{ + Addr: addr, + Config: cfg, + } +} + // NodeApplyableOutput represents an output that is "applyable": -// it is ready to be applied. +// it needs its state value updated to reflect its configuration. type NodeApplyableOutput struct { Addr addrs.AbsOutputValue Config *configs.Output // Config is the output in the config @@ -113,15 +256,18 @@ func (n *NodeApplyableOutput) References() []*addrs.Reference { // GraphNodeEvalable func (n *NodeApplyableOutput) EvalTree() EvalNode { + var state *states.OutputValue + return &EvalSequence{ Nodes: []EvalNode{ - &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkValidate, walkDestroy, walkPlanDestroy}, - Node: &EvalWriteOutput{ - Addr: n.Addr.OutputValue, - Sensitive: n.Config.Sensitive, - Expr: n.Config.Expr, - }, + &EvalOutput{ + Addr: n.Addr.OutputValue, + Config: n.Config, + Output: &state, + }, + &EvalWriteOutputState{ + Addr: n.Addr.OutputValue, + State: &state, }, }, } @@ -138,11 +284,10 @@ func (n *NodeApplyableOutput) DotNode(name string, opts *dag.DotOpts) *dag.DotNo } } -// NodeDestroyableOutput represents an output that is "destroybale": -// its application will remove the output from the state. +// NodeDestroyableOutput represents an output that is "destroyable": +// evaluating it will remove the output from the state altogether. type NodeDestroyableOutput struct { - Addr addrs.AbsOutputValue - Config *configs.Output // Config is the output in the config + Addr addrs.AbsOutputValue } var ( @@ -178,7 +323,7 @@ func (n *NodeDestroyableOutput) TargetDownstream(targetedDeps, untargetedDeps *d // GraphNodeReferencer func (n *NodeDestroyableOutput) References() []*addrs.Reference { - return referencesForOutput(n.Config) + return nil } // GraphNodeEvalable diff --git a/terraform/transform_orphan_output.go b/terraform/transform_orphan_output.go deleted file mode 100644 index c675409346..0000000000 --- a/terraform/transform_orphan_output.go +++ /dev/null @@ -1,60 +0,0 @@ -package terraform - -import ( - "log" - - "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/configs" - "github.com/hashicorp/terraform/states" -) - -// OrphanOutputTransformer finds the outputs that aren't present -// in the given config that are in the state and adds them to the graph -// for deletion. -type OrphanOutputTransformer struct { - Config *configs.Config // Root of config tree - State *states.State // State is the root state -} - -func (t *OrphanOutputTransformer) Transform(g *Graph) error { - if t.State == nil { - log.Printf("[DEBUG] No state, no orphan outputs") - return nil - } - - for _, ms := range t.State.Modules { - if err := t.transform(g, ms); err != nil { - return err - } - } - return nil -} - -func (t *OrphanOutputTransformer) transform(g *Graph, ms *states.Module) error { - if ms == nil { - return nil - } - - moduleAddr := ms.Addr - - // Get the config for this path, which is nil if the entire module has been - // removed. - var outputs map[string]*configs.Output - if c := t.Config.DescendentForInstance(moduleAddr); c != nil { - outputs = c.Module.Outputs - } - - // An output is "orphaned" if it's present in the state but not declared - // in the configuration. - for name := range ms.OutputValues { - if _, exists := outputs[name]; exists { - continue - } - - g.Add(&NodeOutputOrphan{ - Addr: addrs.OutputValue{Name: name}.Absolute(moduleAddr), - }) - } - - return nil -} diff --git a/terraform/transform_output.go b/terraform/transform_output.go index ed93cdb873..f349335f0a 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -1,27 +1,37 @@ package terraform import ( - "log" - + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/states" ) -// OutputTransformer is a GraphTransformer that adds all the outputs -// in the configuration to the graph. +// OutputTransformer is a GraphTransformer that adds nodes for all outputs +// that exist in the configuration or the state, allowing us to evaluate new +// values for them or remove them altogether. // -// This is done for the apply graph builder even if dependent nodes -// aren't changing since there is no downside: the state will be available -// even if the dependent items aren't changing. +// The caller must provide a factory functions for constructing both evaluate +// nodes and destroy nodes, allowing different concrete node types to be used +// during different graph walks. For destroy nodes, the given configuration +// is nil. type OutputTransformer struct { Config *configs.Config + State *states.State + + NewNode func(addr addrs.AbsOutputValue, config *configs.Output) dag.Vertex } func (t *OutputTransformer) Transform(g *Graph) error { - return t.transform(g, t.Config) + err := t.transformConfig(g, t.Config) + if err != nil { + return err + } + + return t.transformOrphans(g) } -func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { +func (t *OutputTransformer) transformConfig(g *Graph, c *configs.Config) error { // If we have no config then there can be no outputs. if c == nil { return nil @@ -31,7 +41,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { // we can reference module outputs and they must show up in the // reference map. for _, cc := range c.Children { - if err := t.transform(g, cc); err != nil { + if err := t.transformConfig(g, cc); err != nil { return err } } @@ -46,50 +56,30 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { for _, o := range c.Module.Outputs { addr := path.OutputValue(o.Name) - node := &NodeApplyableOutput{ - Addr: addr, - Config: o, - } + node := t.NewNode(addr, o) g.Add(node) } return nil } -// DestroyOutputTransformer is a GraphTransformer that adds nodes to delete -// outputs during destroy. We need to do this to ensure that no stale outputs -// are ever left in the state. -type DestroyOutputTransformer struct { -} - -func (t *DestroyOutputTransformer) Transform(g *Graph) error { - for _, v := range g.Vertices() { - output, ok := v.(*NodeApplyableOutput) - if !ok { - continue - } - - // create the destroy node for this output - node := &NodeDestroyableOutput{ - Addr: output.Addr, - Config: output.Config, - } - - log.Printf("[TRACE] creating %s", node.Name()) - g.Add(node) - - deps, err := g.Descendents(v) - if err != nil { - return err - } - - // the destroy node must depend on the eval node - deps.Add(v) - - for _, d := range deps.List() { - log.Printf("[TRACE] %s depends on %s", node.Name(), dag.VertexName(d)) - g.Connect(dag.BasicEdge(node, d)) +func (t *OutputTransformer) transformOrphans(g *Graph) error { + // "orphans" here are any outputs present in the state that are not + // present in the configuration, which we'll therefore need to remove from + // the state after apply. + + for _, ms := range t.State.Modules { + cfg := t.Config.DescendentForInstance(ms.Addr) + for name := range ms.OutputValues { + addr := addrs.OutputValue{Name: name}.Absolute(ms.Addr) + n := t.NewNode(addr, nil) + if cfg == nil { + g.Add(n) + } else if _, exists := cfg.Module.Outputs[name]; !exists { + g.Add(n) + } } } + return nil } -- GitLab