diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index b6fea26fc2f7..d88be4c7d940 100644 --- a/terraform/context_apply_test.go +++ b/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") diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index e91fc77472e8..0008f913cda4 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1647,6 +1647,12 @@ func TestContext2Plan_targetedOrphan(t *testing.T) { ID: "i-789xyz", }, }, + "aws_instance.nottargeted": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "i-abc123", + }, + }, }, }, }, @@ -1667,8 +1673,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) } diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 9fc696c2a3aa..1d36274282e5 100644 --- a/terraform/graph_config_node_resource.go +++ b/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{}) diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index 0b10a24fde50..03e762a74205 100644 --- a/terraform/resource_address_test.go +++ b/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", diff --git a/terraform/state.go b/terraform/state.go index 8734cfc17858..a20fd7ba49af 100644 --- a/terraform/state.go +++ b/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 diff --git a/terraform/state_test.go b/terraform/state_test.go index 8d24a8e75c02..c3bfb18df397 100644 --- a/terraform/state_test.go +++ b/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) + } + } +} diff --git a/terraform/test-fixtures/plan-targeted-module-orphan/main.tf b/terraform/test-fixtures/plan-targeted-module-orphan/main.tf new file mode 100644 index 000000000000..2b33fedaed10 --- /dev/null +++ b/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" +} +*/ diff --git a/terraform/test-fixtures/plan-targeted-over-ten/main.tf b/terraform/test-fixtures/plan-targeted-over-ten/main.tf new file mode 100644 index 000000000000..1c7bc8769e07 --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-over-ten/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "foo" { + count = 13 +} diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index 13e8fbf941c2..d221e57db5be 100644 --- a/terraform/transform_orphan.go +++ b/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. diff --git a/terraform/transform_orphan_test.go b/terraform/transform_orphan_test.go index 85bb4637bb58..76fbaa6a1b73 100644 --- a/terraform/transform_orphan_test.go +++ b/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) } diff --git a/terraform/transform_targets.go b/terraform/transform_targets.go index cab8c8b1ec55..db577b361f14 100644 --- a/terraform/transform_targets.go +++ b/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 }