Merge pull request #12219 from hashicorp/jbardin/state-mv-sort

fix sorting of module resources during state mv

command/state_mv_test.go

@@ -370,6 +370,184 @@ func TestStateMv_stateOutNew_count(t *testing.T) {
 	testStateOutput(t, backups[0], testStateMvCount_stateOutOriginal)
 }
 
+// Modules with more than 10 resources were sorted lexically, causing the
+// indexes in the new location to change.
+func TestStateMv_stateOutNew_largeCount(t *testing.T) {
+	state := &terraform.State{
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_instance.foo.0": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo0",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.1": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo1",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.2": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo2",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.3": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo3",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.4": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo4",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.5": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo5",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.6": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo6",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.7": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo7",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.8": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo8",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.9": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo9",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.10": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo10",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.bar": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	statePath := testStateFile(t, state)
+	stateOutPath := statePath + ".out"
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &StateMvCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		"-state-out", stateOutPath,
+		"test_instance.foo",
+		"test_instance.bar",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// Test it is correct
+	testStateOutput(t, stateOutPath, testStateMvLargeCount_stateOut)
+	testStateOutput(t, statePath, testStateMvLargeCount_stateOutSrc)
+
+	// Test we have backups
+	backups := testStateBackups(t, filepath.Dir(statePath))
+	if len(backups) != 1 {
+		t.Fatalf("bad: %#v", backups)
+	}
+	testStateOutput(t, backups[0], testStateMvLargeCount_stateOutOriginal)
+}
+
 func TestStateMv_stateOutNew_nestedModule(t *testing.T) {
 	state := &terraform.State{
 		Modules: []*terraform.ModuleState{
@@ -506,6 +684,111 @@ test_instance.foo.1:
   foo = value
 `
 
+const testStateMvLargeCount_stateOut = `
+test_instance.bar.0:
+  ID = foo0
+  bar = value
+  foo = value
+test_instance.bar.1:
+  ID = foo1
+  bar = value
+  foo = value
+test_instance.bar.2:
+  ID = foo2
+  bar = value
+  foo = value
+test_instance.bar.3:
+  ID = foo3
+  bar = value
+  foo = value
+test_instance.bar.4:
+  ID = foo4
+  bar = value
+  foo = value
+test_instance.bar.5:
+  ID = foo5
+  bar = value
+  foo = value
+test_instance.bar.6:
+  ID = foo6
+  bar = value
+  foo = value
+test_instance.bar.7:
+  ID = foo7
+  bar = value
+  foo = value
+test_instance.bar.8:
+  ID = foo8
+  bar = value
+  foo = value
+test_instance.bar.9:
+  ID = foo9
+  bar = value
+  foo = value
+test_instance.bar.10:
+  ID = foo10
+  bar = value
+  foo = value
+`
+
+const testStateMvLargeCount_stateOutSrc = `
+test_instance.bar:
+  ID = bar
+  bar = value
+  foo = value
+`
+
+const testStateMvLargeCount_stateOutOriginal = `
+test_instance.bar:
+  ID = bar
+  bar = value
+  foo = value
+test_instance.foo.0:
+  ID = foo0
+  bar = value
+  foo = value
+test_instance.foo.1:
+  ID = foo1
+  bar = value
+  foo = value
+test_instance.foo.2:
+  ID = foo2
+  bar = value
+  foo = value
+test_instance.foo.3:
+  ID = foo3
+  bar = value
+  foo = value
+test_instance.foo.4:
+  ID = foo4
+  bar = value
+  foo = value
+test_instance.foo.5:
+  ID = foo5
+  bar = value
+  foo = value
+test_instance.foo.6:
+  ID = foo6
+  bar = value
+  foo = value
+test_instance.foo.7:
+  ID = foo7
+  bar = value
+  foo = value
+test_instance.foo.8:
+  ID = foo8
+  bar = value
+  foo = value
+test_instance.foo.9:
+  ID = foo9
+  bar = value
+  foo = value
+test_instance.foo.10:
+  ID = foo10
+  bar = value
+  foo = value
+`
+
 const testStateMvNestedModule_stateOut = `
 <no state>
 module.bar:

terraform/context_plan_test.go

@@ -2756,12 +2756,6 @@ 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:
@@ -2778,6 +2772,12 @@ aws_instance.foo.8:
   ID = i-abc8
 aws_instance.foo.9:
   ID = i-abc9
+aws_instance.foo.10:
+  ID = i-abc10
+aws_instance.foo.11:
+  ID = i-abc11
+aws_instance.foo.12:
+  ID = i-abc12
 	`)
 	if actual != expected {
 		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)

terraform/state.go

@@ -1164,7 +1164,8 @@ func (m *ModuleState) String() string {
 	for name, _ := range m.Resources {
 		names = append(names, name)
 	}
-	sort.Strings(names)
+
+	sort.Sort(resourceNameSort(names))
 
 	for _, k := range names {
 		rs := m.Resources[k]
@@ -1204,6 +1205,7 @@ func (m *ModuleState) String() string {
 
 			attrKeys = append(attrKeys, ak)
 		}
+
 		sort.Strings(attrKeys)
 
 		for _, ak := range attrKeys {
@@ -1234,6 +1236,7 @@ func (m *ModuleState) String() string {
 		for k, _ := range m.Outputs {
 			ks = append(ks, k)
 		}
+
 		sort.Strings(ks)
 
 		for _, k := range ks {
@@ -2032,6 +2035,48 @@ func WriteState(d *State, dst io.Writer) error {
 	return nil
 }
 
+// resourceNameSort implements the sort.Interface to sort name parts lexically for
+// strings and numerically for integer indexes.
+type resourceNameSort []string
+
+func (r resourceNameSort) Len() int      { return len(r) }
+func (r resourceNameSort) Swap(i, j int) { r[i], r[j] = r[j], r[i] }
+
+func (r resourceNameSort) Less(i, j int) bool {
+	iParts := strings.Split(r[i], ".")
+	jParts := strings.Split(r[j], ".")
+
+	end := len(iParts)
+	if len(jParts) < end {
+		end = len(jParts)
+	}
+
+	for idx := 0; idx < end; idx++ {
+		if iParts[idx] == jParts[idx] {
+			continue
+		}
+
+		// sort on the first non-matching part
+		iInt, iIntErr := strconv.Atoi(iParts[idx])
+		jInt, jIntErr := strconv.Atoi(jParts[idx])
+
+		switch {
+		case iIntErr == nil && jIntErr == nil:
+			// sort numerically if both parts are integers
+			return iInt < jInt
+		case iIntErr == nil:
+			// numbers sort before strings
+			return true
+		case jIntErr == nil:
+			return false
+		default:
+			return iParts[idx] < jParts[idx]
+		}
+	}
+
+	return false
+}
+
 // moduleStateSort implements sort.Interface to sort module states
 type moduleStateSort []*ModuleState
 

terraform/state_filter.go

@@ -34,7 +34,7 @@ func (f *StateFilter) Filter(fs ...string) ([]*StateFilterResult, error) {
 		as[i] = a
 	}
 
-	// If we werent given any filters, then we list all
+	// If we weren't given any filters, then we list all
 	if len(fs) == 0 {
 		as = append(as, &ResourceAddress{Index: -1})
 	}
@@ -250,6 +250,13 @@ func (s StateFilterResultSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
 func (s StateFilterResultSlice) Less(i, j int) bool {
 	a, b := s[i], s[j]
 
+	// if these address contain an index, we want to sort by index rather than name
+	addrA, errA := ParseResourceAddress(a.Address)
+	addrB, errB := ParseResourceAddress(b.Address)
+	if errA == nil && errB == nil && addrA.Name == addrB.Name && addrA.Index != addrB.Index {
+		return addrA.Index < addrB.Index
+	}
+
 	// If the addresses are different it is just lexographic sorting
 	if a.Address != b.Address {
 		return a.Address < b.Address