Merge pull request #16599 from hashicorp/jbardin/orphaned-module-outputs

Remove modules and module outputs from state

terraform/context_apply_test.go

@@ -342,11 +342,7 @@ func TestContext2Apply_resourceDependsOnModuleStateOnly(t *testing.T) {
 			t.Fatal("should check")
 		}
 
-		checkStateString(t, state, `
-<no state>
-module.child:
-  <no state>
-		`)
+		checkStateString(t, state, "<no state>")
 	}
 }
 
@@ -2698,10 +2694,7 @@ func TestContext2Apply_moduleOrphanInheritAlias(t *testing.T) {
 		t.Fatal("must call configure")
 	}
 
-	checkStateString(t, state, `
-module.child:
-  <no state>
-  `)
+	checkStateString(t, state, "")
 }
 
 func TestContext2Apply_moduleOrphanProvider(t *testing.T) {
@@ -4036,7 +4029,7 @@ func TestContext2Apply_outputOrphanModule(t *testing.T) {
 				"aws": testProviderFuncFixed(p),
 			},
 		),
-		State: state,
+		State: state.DeepCopy(),
 	})
 
 	if _, err := ctx.Plan(); err != nil {
@@ -4051,7 +4044,33 @@ func TestContext2Apply_outputOrphanModule(t *testing.T) {
 	actual := strings.TrimSpace(state.String())
 	expected := strings.TrimSpace(testTerraformApplyOutputOrphanModuleStr)
 	if actual != expected {
-		t.Fatalf("bad: \n%s", actual)
+		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
+	}
+
+	// now apply with no module in the config, which should remove the
+	// remaining output
+	ctx = testContext2(t, &ContextOpts{
+		Module: module.NewEmptyTree(),
+		ProviderResolver: ResourceProviderResolverFixed(
+			map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+		State: state.DeepCopy(),
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err = ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual = strings.TrimSpace(state.String())
+	if actual != "" {
+		t.Fatalf("expected no state, got:\n%s", actual)
 	}
 }
 
@@ -6100,9 +6119,8 @@ func TestContext2Apply_destroyNestedModule(t *testing.T) {
 
 	// Test that things were destroyed
 	actual := strings.TrimSpace(state.String())
-	expected := strings.TrimSpace(testTerraformApplyDestroyNestedModuleStr)
-	if actual != expected {
-		t.Fatalf("bad: \n%s", actual)
+	if actual != "" {
+		t.Fatalf("expected no state, got: %s", actual)
 	}
 }
 
@@ -6150,12 +6168,8 @@ func TestContext2Apply_destroyDeeplyNestedModule(t *testing.T) {
 
 	// Test that things were destroyed
 	actual := strings.TrimSpace(state.String())
-	expected := strings.TrimSpace(`
-module.child.subchild.subsubchild:
-  <no state>
-	`)
-	if actual != expected {
-		t.Fatalf("bad: \n%s", actual)
+	if actual != "" {
+		t.Fatalf("epected no state, got: %s", actual)
 	}
 }
 
@@ -9080,14 +9094,7 @@ func TestContext2Apply_destroyWithProviders(t *testing.T) {
 
 	got := strings.TrimSpace(state.String())
 
-	// This should fail once modules are removed from the state entirely.
-	want := strings.TrimSpace(`
-<no state>
-module.child:
-  <no state>
-module.mod.removed:
-  <no state>`)
-
+	want := strings.TrimSpace("<no state>")
 	if got != want {
 		t.Fatalf("wrong final state\ngot:\n%s\nwant:\n%s", got, want)
 	}

terraform/eval_state.go

@@ -214,37 +214,6 @@ func writeInstanceToState(
 	return nil, nil
 }
 
-// EvalClearPrimaryState is an EvalNode implementation that clears the primary
-// instance from a resource state.
-type EvalClearPrimaryState struct {
-	Name string
-}
-
-func (n *EvalClearPrimaryState) Eval(ctx EvalContext) (interface{}, error) {
-	state, lock := ctx.State()
-
-	// Get a read lock so we can access this instance
-	lock.RLock()
-	defer lock.RUnlock()
-
-	// Look for the module state. If we don't have one, then it doesn't matter.
-	mod := state.ModuleByPath(ctx.Path())
-	if mod == nil {
-		return nil, nil
-	}
-
-	// Look for the resource state. If we don't have one, then it is okay.
-	rs := mod.Resources[n.Name]
-	if rs == nil {
-		return nil, nil
-	}
-
-	// Clear primary from the resource state
-	rs.Primary = nil
-
-	return nil, nil
-}
-
 // EvalDeposeState is an EvalNode implementation that takes the primary
 // out of a state and makes it Deposed. This is done at the beginning of
 // create-before-destroy calls so that the create can create while preserving

terraform/graph_builder_apply.go

@@ -113,6 +113,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Add module variables
 		&ModuleVariableTransformer{Module: b.Module},
 
+		// Remove modules no longer present in the config
+		&RemovedModuleTransformer{Module: b.Module, State: b.State},
+
 		// Connect references so ordering is correct
 		&ReferenceTransformer{},
 

terraform/graph_builder_plan.go

@@ -84,6 +84,12 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 			Module:   b.Module,
 		},
 
+		// Create orphan output nodes
+		&OrphanOutputTransformer{
+			Module: b.Module,
+			State:  b.State,
+		},
+
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Module: b.Module},
 
@@ -109,6 +115,9 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 			Module: b.Module,
 		},
 
+		// Remove modules no longer present in the config
+		&RemovedModuleTransformer{Module: b.Module, State: b.State},
+
 		// Connect so that the references are ready for targeting. We'll
 		// have to connect again later for providers and so on.
 		&ReferenceTransformer{},

terraform/node_module_removed.go

@@ -0,0 +1,77 @@
+package terraform
+
+import (
+	"fmt"
+	"log"
+	"reflect"
+)
+
+// NodeModuleRemoved represents a module that is no longer in the
+// config.
+type NodeModuleRemoved struct {
+	PathValue []string
+}
+
+func (n *NodeModuleRemoved) Name() string {
+	return fmt.Sprintf("%s (removed)", modulePrefixStr(n.PathValue))
+}
+
+// GraphNodeSubPath
+func (n *NodeModuleRemoved) Path() []string {
+	return n.PathValue
+}
+
+// GraphNodeEvalable
+func (n *NodeModuleRemoved) EvalTree() EvalNode {
+	return &EvalOpFilter{
+		Ops: []walkOperation{walkRefresh, walkApply, walkDestroy},
+		Node: &EvalDeleteModule{
+			PathValue: n.PathValue,
+		},
+	}
+}
+
+func (n *NodeModuleRemoved) ReferenceGlobal() bool {
+	return true
+}
+
+func (n *NodeModuleRemoved) References() []string {
+	return []string{modulePrefixStr(n.PathValue)}
+}
+
+// EvalDeleteModule is an EvalNode implementation that removes an empty module
+// entry from the state.
+type EvalDeleteModule struct {
+	PathValue []string
+}
+
+func (n *EvalDeleteModule) Eval(ctx EvalContext) (interface{}, error) {
+	state, lock := ctx.State()
+	if state == nil {
+		return nil, nil
+	}
+
+	// Get a write lock so we can access this instance
+	lock.Lock()
+	defer lock.Unlock()
+
+	// Make sure we have a clean state
+	// Destroyed resources aren't deleted, they're written with an ID of "".
+	state.prune()
+
+	// find the module and delete it
+	for i, m := range state.Modules {
+		if reflect.DeepEqual(m.Path, n.PathValue) {
+			if !m.Empty() {
+				// a targeted apply may leave module resources even without a config,
+				// so just log this and return.
+				log.Printf("[DEBUG] cannot remove module %s, not empty", modulePrefixStr(n.PathValue))
+				break
+			}
+			state.Modules = append(state.Modules[:i], state.Modules[i+1:]...)
+			break
+		}
+	}
+
+	return nil, nil
+}

terraform/node_output_orphan.go

@@ -19,6 +19,11 @@ func (n *NodeOutputOrphan) Name() string {
 	return result
 }
 
+// GraphNodeReferenceable
+func (n *NodeOutputOrphan) ReferenceableName() []string {
+	return []string{"output." + n.OutputName}
+}
+
 // GraphNodeSubPath
 func (n *NodeOutputOrphan) Path() []string {
 	return n.PathValue

terraform/state.go

@@ -1089,7 +1089,7 @@ func (m *ModuleState) Orphans(c *config.Config) []string {
 	defer m.Unlock()
 
 	keys := make(map[string]struct{})
-	for k, _ := range m.Resources {
+	for k := range m.Resources {
 		keys[k] = struct{}{}
 	}
 
@@ -1097,7 +1097,7 @@ func (m *ModuleState) Orphans(c *config.Config) []string {
 		for _, r := range c.Resources {
 			delete(keys, r.Id())
 
-			for k, _ := range keys {
+			for k := range keys {
 				if strings.HasPrefix(k, r.Id()+".") {
 					delete(keys, k)
 				}
@@ -1106,7 +1106,32 @@ func (m *ModuleState) Orphans(c *config.Config) []string {
 	}
 
 	result := make([]string, 0, len(keys))
-	for k, _ := range keys {
+	for k := range keys {
+		result = append(result, k)
+	}
+
+	return result
+}
+
+// RemovedOutputs returns a list of outputs that are in the State but aren't
+// present in the configuration itself.
+func (m *ModuleState) RemovedOutputs(c *config.Config) []string {
+	m.Lock()
+	defer m.Unlock()
+
+	keys := make(map[string]struct{})
+	for k := range m.Outputs {
+		keys[k] = struct{}{}
+	}
+
+	if c != nil {
+		for _, o := range c.Outputs {
+			delete(keys, o.Name)
+		}
+	}
+
+	result := make([]string, 0, len(keys))
+	for k := range keys {
 		result = append(result, k)
 	}
 
@@ -1314,6 +1339,10 @@ func (m *ModuleState) String() string {
 	return buf.String()
 }
 
+func (m *ModuleState) Empty() bool {
+	return len(m.Locals) == 0 && len(m.Outputs) == 0 && len(m.Resources) == 0
+}
+
 // ResourceStateKey is a structured representation of the key used for the
 // ModuleState.Resources mapping
 type ResourceStateKey struct {

terraform/terraform_test.go

@@ -713,11 +713,6 @@ const testTerraformApplyDestroyStr = `
 <no state>
 `
 
-const testTerraformApplyDestroyNestedModuleStr = `
-module.child.subchild:
-  <no state>
-`
-
 const testTerraformApplyErrorStr = `
 aws_instance.bar:
   ID = bar

terraform/transform_orphan_output.go

@@ -21,43 +21,32 @@ func (t *OrphanOutputTransformer) Transform(g *Graph) error {
 		return nil
 	}
 
-	return t.transform(g, t.Module)
-}
-
-func (t *OrphanOutputTransformer) transform(g *Graph, m *module.Tree) error {
-	// Get our configuration, and recurse into children
-	var c *config.Config
-	if m != nil {
-		c = m.Config()
-		for _, child := range m.Children() {
-			if err := t.transform(g, child); err != nil {
+	for _, ms := range t.State.Modules {
+		if err := t.transform(g, ms); err != nil {
 			return err
 		}
 	}
-	}
-
-	// Get the state. If there is no state, then we have no orphans!
-	path := normalizeModulePath(m.Path())
-	state := t.State.ModuleByPath(path)
-	if state == nil {
 	return nil
 }
 
-	// Make a map of the valid outputs
-	valid := make(map[string]struct{})
-	for _, o := range c.Outputs {
-		valid[o.Name] = struct{}{}
+func (t *OrphanOutputTransformer) transform(g *Graph, ms *ModuleState) error {
+	if ms == nil {
+		return nil
 	}
 
-	// Go through the outputs and find the ones that aren't in our config.
-	for n, _ := range state.Outputs {
-		// If it is in the valid map, then ignore
-		if _, ok := valid[n]; ok {
-			continue
+	path := normalizeModulePath(ms.Path)
+
+	// Get the config for this path, which is nil if the entire module has been
+	// removed.
+	var c *config.Config
+	if m := t.Module.Child(path[1:]); m != nil {
+		c = m.Config()
 	}
 
-		// Orphan!
+	// add all the orphaned outputs to the graph
+	for _, n := range ms.RemovedOutputs(c) {
 		g.Add(&NodeOutputOrphan{OutputName: n, PathValue: path})
+
 	}
 
 	return nil

terraform/transform_reference.go

@@ -127,6 +127,7 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) {
 	var matches []dag.Vertex
 	var missing []string
 	prefix := m.prefix(v)
+
 	for _, ns := range rn.References() {
 		found := false
 		for _, n := range strings.Split(ns, "/") {
@@ -139,19 +140,14 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) {
 			// Mark that we found a match
 			found = true
 
-			// Make sure this isn't a self reference, which isn't included
-			selfRef := false
 			for _, p := range parents {
+				// don't include self-references
 				if p == v {
-					selfRef = true
-					break
-				}
-			}
-			if selfRef {
 					continue
 				}
+				matches = append(matches, p)
+			}
 
-			matches = append(matches, parents...)
 			break
 		}
 

terraform/transform_removed_modules.go

@@ -0,0 +1,32 @@
+package terraform
+
+import (
+	"log"
+
+	"github.com/hashicorp/terraform/config/module"
+)
+
+// RemoveModuleTransformer implements GraphTransformer to add nodes indicating
+// when a module was removed from the configuration.
+type RemovedModuleTransformer struct {
+	Module *module.Tree // root module
+	State  *State
+}
+
+func (t *RemovedModuleTransformer) Transform(g *Graph) error {
+	// nothing to remove if there's no state!
+	if t.State == nil {
+		return nil
+	}
+
+	for _, m := range t.State.Modules {
+		c := t.Module.Child(m.Path[1:])
+		if c != nil {
+			continue
+		}
+
+		log.Printf("[DEBUG] module %s no longer in config\n", modulePrefixStr(m.Path))
+		g.Add(&NodeModuleRemoved{PathValue: m.Path})
+	}
+	return nil
+}