Merge pull request #4574 from hashicorp/phinze/orphan-addressing

Orphan addressing / targeting

terraform/context_apply_test.go

@@ -3359,6 +3359,60 @@ aws_instance.bar:
 	`)
 }
 
+// https://github.com/hashicorp/terraform/issues/4462
+func TestContext2Apply_targetedDestroyModule(t *testing.T) {
+	m := testModule(t, "apply-targeted-module")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: &State{
+			Modules: []*ModuleState{
+				&ModuleState{
+					Path: rootModulePath,
+					Resources: map[string]*ResourceState{
+						"aws_instance.foo": resourceState("aws_instance", "i-bcd345"),
+						"aws_instance.bar": resourceState("aws_instance", "i-abc123"),
+					},
+				},
+				&ModuleState{
+					Path: []string{"root", "child"},
+					Resources: map[string]*ResourceState{
+						"aws_instance.foo": resourceState("aws_instance", "i-bcd345"),
+						"aws_instance.bar": resourceState("aws_instance", "i-abc123"),
+					},
+				},
+			},
+		},
+		Targets: []string{"module.child.aws_instance.foo"},
+		Destroy: true,
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	checkStateString(t, state, `
+aws_instance.bar:
+  ID = i-abc123
+aws_instance.foo:
+  ID = i-bcd345
+
+module.child:
+  aws_instance.bar:
+    ID = i-abc123
+	`)
+}
+
 func TestContext2Apply_targetedDestroyCountIndex(t *testing.T) {
 	m := testModule(t, "apply-targeted-count")
 	p := testProvider("aws")

terraform/context_plan_test.go

@@ -1670,6 +1670,12 @@ func TestContext2Plan_targetedOrphan(t *testing.T) {
 								ID: "i-789xyz",
 							},
 						},
+						"aws_instance.nottargeted": &ResourceState{
+							Type: "aws_instance",
+							Primary: &InstanceState{
+								ID: "i-abc123",
+							},
+						},
 					},
 				},
 			},
@@ -1690,8 +1696,150 @@ DESTROY: aws_instance.orphan
 
 STATE:
 
+aws_instance.nottargeted:
+  ID = i-abc123
 aws_instance.orphan:
-  ID = i-789xyz`)
+  ID = i-789xyz
+`)
+	if actual != expected {
+		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
+	}
+}
+
+// https://github.com/hashicorp/terraform/issues/2538
+func TestContext2Plan_targetedModuleOrphan(t *testing.T) {
+	m := testModule(t, "plan-targeted-module-orphan")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: &State{
+			Modules: []*ModuleState{
+				&ModuleState{
+					Path: []string{"root", "child"},
+					Resources: map[string]*ResourceState{
+						"aws_instance.orphan": &ResourceState{
+							Type: "aws_instance",
+							Primary: &InstanceState{
+								ID: "i-789xyz",
+							},
+						},
+						"aws_instance.nottargeted": &ResourceState{
+							Type: "aws_instance",
+							Primary: &InstanceState{
+								ID: "i-abc123",
+							},
+						},
+					},
+				},
+			},
+		},
+		Destroy: true,
+		Targets: []string{"module.child.aws_instance.orphan"},
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(`DIFF:
+
+module.child:
+  DESTROY: aws_instance.orphan
+
+STATE:
+
+module.child:
+  aws_instance.nottargeted:
+    ID = i-abc123
+  aws_instance.orphan:
+    ID = i-789xyz
+`)
+	if actual != expected {
+		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
+	}
+}
+
+// https://github.com/hashicorp/terraform/issues/4515
+func TestContext2Plan_targetedOverTen(t *testing.T) {
+	m := testModule(t, "plan-targeted-over-ten")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+
+	resources := make(map[string]*ResourceState)
+	var expectedState []string
+	for i := 0; i < 13; i++ {
+		key := fmt.Sprintf("aws_instance.foo.%d", i)
+		id := fmt.Sprintf("i-abc%d", i)
+		resources[key] = &ResourceState{
+			Type:    "aws_instance",
+			Primary: &InstanceState{ID: id},
+		}
+		expectedState = append(expectedState,
+			fmt.Sprintf("%s:\n  ID = %s\n", key, id))
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: &State{
+			Modules: []*ModuleState{
+				&ModuleState{
+					Path:      rootModulePath,
+					Resources: resources,
+				},
+			},
+		},
+		Targets: []string{"aws_instance.foo[1]"},
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	sort.Strings(expectedState)
+	expected := strings.TrimSpace(`
+DIFF:
+
+
+
+STATE:
+
+aws_instance.foo.0:
+  ID = i-abc0
+aws_instance.foo.1:
+  ID = i-abc1
+aws_instance.foo.10:
+  ID = i-abc10
+aws_instance.foo.11:
+  ID = i-abc11
+aws_instance.foo.12:
+  ID = i-abc12
+aws_instance.foo.2:
+  ID = i-abc2
+aws_instance.foo.3:
+  ID = i-abc3
+aws_instance.foo.4:
+  ID = i-abc4
+aws_instance.foo.5:
+  ID = i-abc5
+aws_instance.foo.6:
+  ID = i-abc6
+aws_instance.foo.7:
+  ID = i-abc7
+aws_instance.foo.8:
+  ID = i-abc8
+aws_instance.foo.9:
+  ID = i-abc9
+	`)
 	if actual != expected {
 		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
 	}

terraform/graph_config_node_resource.go

@@ -163,9 +163,8 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error)
 		// expand orphans, which have all the same semantics in a destroy
 		// as a primary.
 		steps = append(steps, &OrphanTransformer{
-			State:   state,
-			View:    n.Resource.Id(),
-			Targets: n.Targets,
+			State: state,
+			View:  n.Resource.Id(),
 		})
 
 		steps = append(steps, &DeposedTransformer{
@@ -181,6 +180,12 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error)
 		})
 	}
 
+	// We always want to apply targeting
+	steps = append(steps, &TargetsTransformer{
+		ParsedTargets: n.Targets,
+		Destroy:       n.DestroyMode != DestroyNone,
+	})
+
 	// Always end with the root being added
 	steps = append(steps, &RootTransformer{})
 

terraform/resource_address_test.go

@@ -28,6 +28,15 @@ func TestParseResourceAddress(t *testing.T) {
 				Index:        2,
 			},
 		},
+		"implicit primary, explicit index over ten": {
+			Input: "aws_instance.foo[12]",
+			Expected: &ResourceAddress{
+				Type:         "aws_instance",
+				Name:         "foo",
+				InstanceType: TypePrimary,
+				Index:        12,
+			},
+		},
 		"explicit primary, explicit index": {
 			Input: "aws_instance.foo.primary[2]",
 			Expected: &ResourceAddress{
@@ -184,6 +193,21 @@ func TestResourceAddressEquals(t *testing.T) {
 			},
 			Expect: true,
 		},
+		"index over ten": {
+			Address: &ResourceAddress{
+				Type:         "aws_instance",
+				Name:         "foo",
+				InstanceType: TypePrimary,
+				Index:        1,
+			},
+			Other: &ResourceAddress{
+				Type:         "aws_instance",
+				Name:         "foo",
+				InstanceType: TypePrimary,
+				Index:        13,
+			},
+			Expect: false,
+		},
 		"different type": {
 			Address: &ResourceAddress{
 				Type:         "aws_instance",

terraform/state.go

@@ -9,6 +9,7 @@ import (
 	"log"
 	"reflect"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/hashicorp/terraform/config"
@@ -661,6 +662,65 @@ func (m *ModuleState) String() string {
 	return buf.String()
 }
 
+// ResourceStateKey is a structured representation of the key used for the
+// ModuleState.Resources mapping
+type ResourceStateKey struct {
+	Name  string
+	Type  string
+	Index int
+}
+
+// Equal determines whether two ResourceStateKeys are the same
+func (rsk *ResourceStateKey) Equal(other *ResourceStateKey) bool {
+	if rsk == nil || other == nil {
+		return false
+	}
+	if rsk.Type != other.Type {
+		return false
+	}
+	if rsk.Name != other.Name {
+		return false
+	}
+	if rsk.Index != other.Index {
+		return false
+	}
+	return true
+}
+
+func (rsk *ResourceStateKey) String() string {
+	if rsk == nil {
+		return ""
+	}
+	if rsk.Index == -1 {
+		return fmt.Sprintf("%s.%s", rsk.Type, rsk.Name)
+	}
+	return fmt.Sprintf("%s.%s.%d", rsk.Type, rsk.Name, rsk.Index)
+}
+
+// ParseResourceStateKey accepts a key in the format used by
+// ModuleState.Resources and returns a resource name and resource index. In the
+// state, a resource has the format "type.name.index" or "type.name". In the
+// latter case, the index is returned as -1.
+func ParseResourceStateKey(k string) (*ResourceStateKey, error) {
+	parts := strings.Split(k, ".")
+	if len(parts) < 2 || len(parts) > 3 {
+		return nil, fmt.Errorf("Malformed resource state key: %s", k)
+	}
+	rsk := &ResourceStateKey{
+		Type:  parts[0],
+		Name:  parts[1],
+		Index: -1,
+	}
+	if len(parts) == 3 {
+		index, err := strconv.Atoi(parts[2])
+		if err != nil {
+			return nil, fmt.Errorf("Malformed resource state key index: %s", k)
+		}
+		rsk.Index = index
+	}
+	return rsk, nil
+}
+
 // ResourceState holds the state of a resource that is used so that
 // a provider can find and manage an existing resource as well as for
 // storing attributes that are used to populate variables of child

terraform/state_test.go

@@ -895,3 +895,57 @@ func TestUpgradeV1State(t *testing.T) {
 		t.Fatalf("bad: %#v", bt)
 	}
 }
+
+func TestParseResourceStateKey(t *testing.T) {
+	cases := []struct {
+		Input       string
+		Expected    *ResourceStateKey
+		ExpectedErr bool
+	}{
+		{
+			Input: "aws_instance.foo.3",
+			Expected: &ResourceStateKey{
+				Type:  "aws_instance",
+				Name:  "foo",
+				Index: 3,
+			},
+		},
+		{
+			Input: "aws_instance.foo.0",
+			Expected: &ResourceStateKey{
+				Type:  "aws_instance",
+				Name:  "foo",
+				Index: 0,
+			},
+		},
+		{
+			Input: "aws_instance.foo",
+			Expected: &ResourceStateKey{
+				Type:  "aws_instance",
+				Name:  "foo",
+				Index: -1,
+			},
+		},
+		{
+			Input:       "aws_instance.foo.malformed",
+			ExpectedErr: true,
+		},
+		{
+			Input:       "aws_instance.foo.malformedwithnumber.123",
+			ExpectedErr: true,
+		},
+		{
+			Input:       "malformed",
+			ExpectedErr: true,
+		},
+	}
+	for _, tc := range cases {
+		rsk, err := ParseResourceStateKey(tc.Input)
+		if rsk != nil && tc.Expected != nil && !rsk.Equal(tc.Expected) {
+			t.Fatalf("%s: expected %s, got %s", tc.Input, tc.Expected, rsk)
+		}
+		if (err != nil) != tc.ExpectedErr {
+			t.Fatalf("%s: expected err: %t, got %s", tc.Input, tc.ExpectedErr, err)
+		}
+	}
+}

terraform/test-fixtures/plan-targeted-module-orphan/main.tf

@@ -0,0 +1,6 @@
+# Once opon a time, there was a child module here
+/*
+module "child" {
+  source = "./child"
+}
+*/

terraform/test-fixtures/plan-targeted-over-ten/main.tf

@@ -0,0 +1,3 @@
+resource "aws_instance" "foo" {
+  count    = 13
+}

terraform/transform_orphan.go

@@ -2,7 +2,6 @@ package terraform
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
@@ -26,11 +25,6 @@ type OrphanTransformer struct {
 	// using the graph path.
 	Module *module.Tree
 
-	// Targets are user-specified resources to target. We need to be aware of
-	// these so we don't improperly identify orphans when they've just been
-	// filtered out of the graph via targeting.
-	Targets []ResourceAddress
-
 	// View, if non-nil will set a view on the module state.
 	View string
 }
@@ -68,22 +62,6 @@ func (t *OrphanTransformer) Transform(g *Graph) error {
 		}
 
 		resourceOrphans := state.Orphans(config)
-		if len(t.Targets) > 0 {
-			var targetedOrphans []string
-			for _, o := range resourceOrphans {
-				targeted := false
-				for _, t := range t.Targets {
-					prefix := fmt.Sprintf("%s.%s.%d", t.Type, t.Name, t.Index)
-					if strings.HasPrefix(o, prefix) {
-						targeted = true
-					}
-				}
-				if targeted {
-					targetedOrphans = append(targetedOrphans, o)
-				}
-			}
-			resourceOrphans = targetedOrphans
-		}
 
 		resourceVertexes = make([]dag.Vertex, len(resourceOrphans))
 		for i, k := range resourceOrphans {
@@ -95,11 +73,15 @@ func (t *OrphanTransformer) Transform(g *Graph) error {
 
 			rs := state.Resources[k]
 
+			rsk, err := ParseResourceStateKey(k)
+			if err != nil {
+				return err
+			}
 			resourceVertexes[i] = g.Add(&graphNodeOrphanResource{
-				ResourceName: k,
-				ResourceType: rs.Type,
-				Provider:     rs.Provider,
-				dependentOn:  rs.Dependencies,
+				Path:        g.Path,
+				ResourceKey: rsk,
+				Provider:    rs.Provider,
+				dependentOn: rs.Dependencies,
 			})
 		}
 	}
@@ -175,15 +157,25 @@ func (n *graphNodeOrphanModule) Expand(b GraphBuilder) (GraphNodeSubgraph, error
 
 // graphNodeOrphanResource is the graph vertex representing an orphan resource..
 type graphNodeOrphanResource struct {
-	ResourceName string
-	ResourceType string
-	Provider     string
+	Path        []string
+	ResourceKey *ResourceStateKey
+	Provider    string
 
 	dependentOn []string
 }
 
+func (n *graphNodeOrphanResource) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeResource
+}
+
 func (n *graphNodeOrphanResource) ResourceAddress() *ResourceAddress {
-	return n.ResourceAddress()
+	return &ResourceAddress{
+		Index:        n.ResourceKey.Index,
+		InstanceType: TypePrimary,
+		Name:         n.ResourceKey.Name,
+		Path:         n.Path[1:],
+		Type:         n.ResourceKey.Type,
+	}
 }
 
 func (n *graphNodeOrphanResource) DependableName() []string {
@@ -202,11 +194,11 @@ func (n *graphNodeOrphanResource) Flatten(p []string) (dag.Vertex, error) {
 }
 
 func (n *graphNodeOrphanResource) Name() string {
-	return fmt.Sprintf("%s (orphan)", n.ResourceName)
+	return fmt.Sprintf("%s (orphan)", n.ResourceKey)
 }
 
 func (n *graphNodeOrphanResource) ProvidedBy() []string {
-	return []string{resourceProvider(n.ResourceName, n.Provider)}
+	return []string{resourceProvider(n.ResourceKey.Type, n.Provider)}
 }
 
 // GraphNodeEvalable impl.
@@ -217,7 +209,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 	seq := &EvalSequence{Nodes: make([]EvalNode, 0, 5)}
 
 	// Build instance info
-	info := &InstanceInfo{Id: n.ResourceName, Type: n.ResourceType}
+	info := &InstanceInfo{Id: n.ResourceKey.String(), Type: n.ResourceKey.Type}
 	seq.Nodes = append(seq.Nodes, &EvalInstanceInfo{Info: info})
 
 	// Refresh the resource
@@ -230,7 +222,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 					Output: &provider,
 				},
 				&EvalReadState{
-					Name:   n.ResourceName,
+					Name:   n.ResourceKey.String(),
 					Output: &state,
 				},
 				&EvalRefresh{
@@ -240,8 +232,8 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 					Output:   &state,
 				},
 				&EvalWriteState{
-					Name:         n.ResourceName,
-					ResourceType: n.ResourceType,
+					Name:         n.ResourceKey.String(),
+					ResourceType: n.ResourceKey.Type,
 					Provider:     n.Provider,
 					Dependencies: n.DependentOn(),
 					State:        &state,
@@ -257,7 +249,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 		Node: &EvalSequence{
 			Nodes: []EvalNode{
 				&EvalReadState{
-					Name:   n.ResourceName,
+					Name:   n.ResourceKey.String(),
 					Output: &state,
 				},
 				&EvalDiffDestroy{
@@ -266,7 +258,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 					Output: &diff,
 				},
 				&EvalWriteDiff{
-					Name: n.ResourceName,
+					Name: n.ResourceKey.String(),
 					Diff: &diff,
 				},
 			},
@@ -280,7 +272,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 		Node: &EvalSequence{
 			Nodes: []EvalNode{
 				&EvalReadDiff{
-					Name: n.ResourceName,
+					Name: n.ResourceKey.String(),
 					Diff: &diff,
 				},
 				&EvalGetProvider{
@@ -288,7 +280,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 					Output: &provider,
 				},
 				&EvalReadState{
-					Name:   n.ResourceName,
+					Name:   n.ResourceKey.String(),
 					Output: &state,
 				},
 				&EvalApply{
@@ -300,8 +292,8 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 					Error:    &err,
 				},
 				&EvalWriteState{
-					Name:         n.ResourceName,
-					ResourceType: n.ResourceType,
+					Name:         n.ResourceKey.String(),
+					ResourceType: n.ResourceKey.Type,
 					Provider:     n.Provider,
 					Dependencies: n.DependentOn(),
 					State:        &state,
@@ -320,7 +312,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode {
 }
 
 func (n *graphNodeOrphanResource) dependableName() string {
-	return n.ResourceName
+	return n.ResourceKey.String()
 }
 
 // GraphNodeDestroyable impl.

terraform/transform_orphan_test.go

@@ -333,17 +333,18 @@ func TestGraphNodeOrphanResource_impl(t *testing.T) {
 	var _ dag.Vertex = new(graphNodeOrphanResource)
 	var _ dag.NamedVertex = new(graphNodeOrphanResource)
 	var _ GraphNodeProviderConsumer = new(graphNodeOrphanResource)
+	var _ GraphNodeAddressable = new(graphNodeOrphanResource)
 }
 
 func TestGraphNodeOrphanResource_ProvidedBy(t *testing.T) {
-	n := &graphNodeOrphanResource{ResourceName: "aws_instance.foo"}
+	n := &graphNodeOrphanResource{ResourceKey: &ResourceStateKey{Type: "aws_instance"}}
 	if v := n.ProvidedBy(); v[0] != "aws" {
 		t.Fatalf("bad: %#v", v)
 	}
 }
 
 func TestGraphNodeOrphanResource_ProvidedBy_alias(t *testing.T) {
-	n := &graphNodeOrphanResource{ResourceName: "aws_instance.foo", Provider: "aws.bar"}
+	n := &graphNodeOrphanResource{ResourceKey: &ResourceStateKey{Type: "aws_instance"}, Provider: "aws.bar"}
 	if v := n.ProvidedBy(); v[0] != "aws.bar" {
 		t.Fatalf("bad: %#v", v)
 	}

terraform/transform_targets.go

@@ -13,20 +13,25 @@ type TargetsTransformer struct {
 	// List of targeted resource names specified by the user
 	Targets []string
 
+	// List of parsed targets, provided by callers like ResourceCountTransform
+	// that already have the targets parsed
+	ParsedTargets []ResourceAddress
+
 	// Set to true when we're in a `terraform destroy` or a
 	// `terraform plan -destroy`
 	Destroy bool
 }
 
 func (t *TargetsTransformer) Transform(g *Graph) error {
-	if len(t.Targets) > 0 {
-		// TODO: duplicated in OrphanTransformer; pull up parsing earlier
+	if len(t.Targets) > 0 && len(t.ParsedTargets) == 0 {
 		addrs, err := t.parseTargetAddresses()
 		if err != nil {
 			return err
 		}
-
-		targetedNodes, err := t.selectTargetedNodes(g, addrs)
+		t.ParsedTargets = addrs
+	}
+	if len(t.ParsedTargets) > 0 {
+		targetedNodes, err := t.selectTargetedNodes(g, t.ParsedTargets)
 		if err != nil {
 			return err
 		}