[WIP] Orphan addressing / targeting

Instead of trying to skip non-targeted orphans as they are added to
the graph in OrphanTransformer, remove knowledge of targeting from
OrphanTransformer and instead make the orphan resource nodes properly
addressable.

That allows us to use existing logic in TargetTransformer to filter out
the nodes appropriately. This does require adding TargetTransformer to the
list of transforms that run during DynamicExpand so that targeting can
be applied to nodes with expanded counts.

Includes test coverage proving that it fixes #4515. Will do further
testing to determine whether this also fixes the various linked issues.

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,90 @@ 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)
+ }
+}
+
+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-over-ten/main.tf

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