From 1a1ace593047f40008f551754481f29f60e96c39 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 2 Apr 2020 13:26:53 -0400 Subject: [PATCH] prune unused values based on behavior remove the hard-coded types from PruneUnusedValuesTransformer --- terraform/node_local.go | 12 +++++++ terraform/node_module_variable.go | 10 ++++++ terraform/node_output.go | 12 +++++++ terraform/node_value.go | 10 ++++++ terraform/transform_reference.go | 53 +++++++++---------------------- 5 files changed, 59 insertions(+), 38 deletions(-) create mode 100644 terraform/node_value.go diff --git a/terraform/node_local.go b/terraform/node_local.go index 50cff018a..9fdeedd0d 100644 --- a/terraform/node_local.go +++ b/terraform/node_local.go @@ -22,8 +22,14 @@ var ( _ GraphNodeReferenceable = (*NodePlannableLocal)(nil) _ GraphNodeReferencer = (*NodePlannableLocal)(nil) _ GraphNodeDynamicExpandable = (*NodePlannableLocal)(nil) + _ graphNodeTemporaryValue = (*NodePlannableLocal)(nil) ) +// graphNodeTemporaryValue +func (n *NodePlannableLocal) temporaryValue() bool { + return true +} + func (n *NodePlannableLocal) Name() string { path := n.Module.String() addr := n.Addr.String() @@ -83,9 +89,15 @@ var ( _ GraphNodeReferenceable = (*NodeLocal)(nil) _ GraphNodeReferencer = (*NodeLocal)(nil) _ GraphNodeEvalable = (*NodeLocal)(nil) + _ graphNodeTemporaryValue = (*NodeLocal)(nil) _ dag.GraphNodeDotter = (*NodeLocal)(nil) ) +// graphNodeTemporaryValue +func (n *NodeLocal) temporaryValue() bool { + return true +} + func (n *NodeLocal) Name() string { return n.Addr.String() } diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index a320cb373..103a91944 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -25,9 +25,14 @@ var ( _ GraphNodeReferenceOutside = (*NodePlannableModuleVariable)(nil) _ GraphNodeReferenceable = (*NodePlannableModuleVariable)(nil) _ GraphNodeReferencer = (*NodePlannableModuleVariable)(nil) + _ graphNodeTemporaryValue = (*NodePlannableModuleVariable)(nil) _ RemovableIfNotTargeted = (*NodePlannableModuleVariable)(nil) ) +func (n *NodePlannableModuleVariable) temporaryValue() bool { + return true +} + func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) { var g Graph expander := ctx.InstanceExpander() @@ -120,9 +125,14 @@ var ( _ GraphNodeReferenceable = (*NodeApplyableModuleVariable)(nil) _ GraphNodeReferencer = (*NodeApplyableModuleVariable)(nil) _ GraphNodeEvalable = (*NodeApplyableModuleVariable)(nil) + _ graphNodeTemporaryValue = (*NodeApplyableModuleVariable)(nil) _ dag.GraphNodeDotter = (*NodeApplyableModuleVariable)(nil) ) +func (n *NodeApplyableModuleVariable) temporaryValue() bool { + return true +} + func (n *NodeApplyableModuleVariable) Name() string { return n.Addr.String() } diff --git a/terraform/node_output.go b/terraform/node_output.go index 3d3c80a91..203a78ab9 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -23,8 +23,14 @@ var ( _ GraphNodeReferenceable = (*NodePlannableOutput)(nil) _ GraphNodeReferencer = (*NodePlannableOutput)(nil) _ GraphNodeDynamicExpandable = (*NodePlannableOutput)(nil) + _ graphNodeTemporaryValue = (*NodeApplyableOutput)(nil) ) +func (n *NodePlannableOutput) temporaryValue() bool { + // this must always be evaluated if it is a root module output + return !n.Module.IsRoot() +} + func (n *NodePlannableOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { var g Graph expander := ctx.InstanceExpander() @@ -115,9 +121,15 @@ var ( _ GraphNodeReferencer = (*NodeApplyableOutput)(nil) _ GraphNodeReferenceOutside = (*NodeApplyableOutput)(nil) _ GraphNodeEvalable = (*NodeApplyableOutput)(nil) + _ graphNodeTemporaryValue = (*NodeApplyableOutput)(nil) _ dag.GraphNodeDotter = (*NodeApplyableOutput)(nil) ) +func (n *NodeApplyableOutput) temporaryValue() bool { + // this must always be evaluated if it is a root module output + return !n.Addr.Module.IsRoot() +} + func (n *NodeApplyableOutput) Name() string { return n.Addr.String() } diff --git a/terraform/node_value.go b/terraform/node_value.go new file mode 100644 index 000000000..62a6e6ae8 --- /dev/null +++ b/terraform/node_value.go @@ -0,0 +1,10 @@ +package terraform + +// graphNodeTemporaryValue is implemented by nodes that may represent temporary +// values, which are those not saved to the state file. This includes locals, +// variables, and non-root outputs. +// A boolean return value allows a node which may need to be saved to +// conditionally do so. +type graphNodeTemporaryValue interface { + temporaryValue() bool +} diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index cf03cda00..d92783a32 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -185,63 +185,40 @@ type PruneUnusedValuesTransformer struct { Destroy bool } -// FIXME: outputs will now have module expansion root nodes dependent on them, -// so we need to filter those out too. Might be a good time to decouple this -// from concrete types, and implement some interfaces. func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error { // Pruning a value can effect previously checked edges, so loop until there // are no more changes. for removed := 0; ; removed = 0 { for _, v := range g.Vertices() { + // we're only concerned with values that don't need to be saved in state switch v := v.(type) { - case *NodeApplyableOutput: - // If we're not certain this is a full destroy, we need to keep any - // root module outputs - if v.Addr.Module.IsRoot() && !t.Destroy { + case graphNodeTemporaryValue: + if !v.temporaryValue() { continue } - case *NodePlannableOutput: - // Have similar guardrails for plannable outputs as applyable above - if v.Module.IsRoot() && !t.Destroy { - continue - } - case *NodeLocal, *NodeApplyableModuleVariable, *NodePlannableModuleVariable: - // OK default: - // We're only concerned with variables, locals and outputs continue } dependants := g.UpEdges(v) - switch dependants.Len() { - case 0: - // nothing at all depends on this + // any referencers in the dependents means we need to keep this + // value for evaluation + removable := true + for _, d := range dependants.List() { + if _, ok := d.(GraphNodeReferencer); ok { + removable = false + break + } + } + + if removable { log.Printf("[TRACE] PruneUnusedValuesTransformer: removing unused value %s", dag.VertexName(v)) g.Remove(v) removed++ - default: - // because an output's destroy node always depends on the output, - // we need to check for the case of a single destroy node. - removable := true - SEARCH: - for _, d := range dependants.List() { - switch d.(type) { - case *NodeDestroyableOutput, *nodeCloseModule: - //pass - default: - removable = false - break SEARCH - } - } - - if removable { - log.Printf("[TRACE] PruneUnusedValuesTransformer: removing unused value %s", dag.VertexName(v)) - g.Remove(v) - removed++ - } } } + if removed == 0 { break }