Merge pull request #8304 from hashicorp/b-state-mv

Fix `state mv` with nested modules to work properly

command/state_mv.go

@@ -79,13 +79,16 @@ func (c *StateMvCommand) Run(args []string) int {
 		return 1
 	}
 
+	// Get the item to add to the state
+	add := c.addableResult(results)
+
 	// Do the actual move
 	if err := stateFromReal.Remove(args[0]); err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateMv, err))
 		return 1
 	}
 
-	if err := stateToReal.Add(args[0], args[1], results[0].Value); err != nil {
+	if err := stateToReal.Add(args[0], args[1], add); err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateMv, err))
 		return 1
 	}
@@ -119,6 +122,54 @@ func (c *StateMvCommand) Run(args []string) int {
 	return 0
 }
 
+// addableResult takes the result from a filter operation and returns what to
+// call State.Add with. The reason we do this is beacuse in the module case
+// we must add the list of all modules returned versus just the root module.
+func (c *StateMvCommand) addableResult(results []*terraform.StateFilterResult) interface{} {
+	switch v := results[0].Value.(type) {
+	case *terraform.ModuleState:
+		// If a module state then we should add the full list of modules
+		result := []*terraform.ModuleState{v}
+		if len(results) > 1 {
+			for _, r := range results[1:] {
+				if ms, ok := r.Value.(*terraform.ModuleState); ok {
+					result = append(result, ms)
+				}
+			}
+		}
+
+		return result
+
+	case *terraform.ResourceState:
+		// If a resource state with more than one result, it has a multi-count
+		// and we need to add all of them.
+		result := []*terraform.ResourceState{v}
+		if len(results) > 1 {
+			for _, r := range results[1:] {
+				rs, ok := r.Value.(*terraform.ResourceState)
+				if !ok {
+					continue
+				}
+
+				if rs.Type == v.Type {
+					result = append(result, rs)
+				}
+			}
+		}
+
+		// If we only have one item, add it directly
+		if len(result) == 1 {
+			return result[0]
+		}
+
+		return result
+
+	default:
+		// By default just add the first result
+		return v
+	}
+}
+
 func (c *StateMvCommand) Help() string {
 	helpText := `
 Usage: terraform state mv [options] ADDRESS ADDRESS

command/state_mv_test.go

@@ -223,6 +223,164 @@ func TestStateMv_noState(t *testing.T) {
 	}
 }
 
+func TestStateMv_stateOutNew_count(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: "foo",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.1": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+							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, testStateMvCount_stateOut)
+	testStateOutput(t, statePath, testStateMvCount_stateOutSrc)
+
+	// Test we have backups
+	backups := testStateBackups(t, filepath.Dir(statePath))
+	if len(backups) != 1 {
+		t.Fatalf("bad: %#v", backups)
+	}
+	testStateOutput(t, backups[0], testStateMvCount_stateOutOriginal)
+}
+
+func TestStateMv_stateOutNew_nestedModule(t *testing.T) {
+	state := &terraform.State{
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path:      []string{"root"},
+				Resources: map[string]*terraform.ResourceState{},
+			},
+
+			&terraform.ModuleState{
+				Path:      []string{"root", "foo"},
+				Resources: map[string]*terraform.ResourceState{},
+			},
+
+			&terraform.ModuleState{
+				Path: []string{"root", "foo", "child1"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_instance.foo": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+				},
+			},
+
+			&terraform.ModuleState{
+				Path: []string{"root", "foo", "child2"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_instance.foo": &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,
+		"module.foo",
+		"module.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, testStateMvNestedModule_stateOut)
+	testStateOutput(t, statePath, testStateMvNestedModule_stateOutSrc)
+
+	// Test we have backups
+	backups := testStateBackups(t, filepath.Dir(statePath))
+	if len(backups) != 1 {
+		t.Fatalf("bad: %#v", backups)
+	}
+	testStateOutput(t, backups[0], testStateMvNestedModule_stateOutOriginal)
+}
+
 const testStateMvOutputOriginal = `
 test_instance.baz:
   ID = foo
@@ -245,6 +403,75 @@ test_instance.baz:
   foo = value
 `
 
+const testStateMvCount_stateOut = `
+test_instance.bar.0:
+  ID = foo
+  bar = value
+  foo = value
+test_instance.bar.1:
+  ID = bar
+  bar = value
+  foo = value
+`
+
+const testStateMvCount_stateOutSrc = `
+test_instance.bar:
+  ID = bar
+  bar = value
+  foo = value
+`
+
+const testStateMvCount_stateOutOriginal = `
+test_instance.bar:
+  ID = bar
+  bar = value
+  foo = value
+test_instance.foo.0:
+  ID = foo
+  bar = value
+  foo = value
+test_instance.foo.1:
+  ID = bar
+  bar = value
+  foo = value
+`
+
+const testStateMvNestedModule_stateOut = `
+<no state>
+module.bar:
+  <no state>
+module.bar.child1:
+  test_instance.foo:
+    ID = bar
+    bar = value
+    foo = value
+module.bar.child2:
+  test_instance.foo:
+    ID = bar
+    bar = value
+    foo = value
+`
+
+const testStateMvNestedModule_stateOutSrc = `
+<no state>
+`
+
+const testStateMvNestedModule_stateOutOriginal = `
+<no state>
+module.foo:
+  <no state>
+module.foo.child1:
+  test_instance.foo:
+    ID = bar
+    bar = value
+    foo = value
+module.foo.child2:
+  test_instance.foo:
+    ID = bar
+    bar = value
+    foo = value
+`
+
 const testStateMvOutput_stateOut = `
 test_instance.bar:
   ID = bar

terraform/state.go

@@ -808,6 +808,12 @@ func (m *ModuleState) IsRoot() bool {
 	return reflect.DeepEqual(m.Path, rootModulePath)
 }
 
+// IsDescendent returns true if other is a descendent of this module.
+func (m *ModuleState) IsDescendent(other *ModuleState) bool {
+	i := len(m.Path)
+	return len(other.Path) > i && reflect.DeepEqual(other.Path[:i], m.Path)
+}
+
 // Orphans returns a list of keys of resources that are in the State
 // but aren't present in the configuration itself. Hence, these keys
 // represent the state of resources that are orphans.

terraform/state_add.go

@@ -11,6 +11,11 @@ import (
 // module cannot be moved to a resource address, however a resource can be
 // moved to a module address (it retains the same name, under that resource).
 //
+// The item can also be a []*ModuleState, which is the case for nested
+// modules. In this case, Add will expect the zero-index to be the top-most
+// module to add and will only nest children from there. For semantics, this
+// is equivalent to module => module.
+//
 // The full semantics of Add:
 //
 //                         ┌───────────────────────┬───────────────────────┬───────────────────────┐
@@ -65,7 +70,26 @@ func (s *State) Add(fromAddrRaw string, toAddrRaw string, raw interface{}) error
 }
 
 func stateAddFunc_Module_Module(s *State, fromAddr, addr *ResourceAddress, raw interface{}) error {
-	src := raw.(*ModuleState).deepcopy()
+	// raw can be either *ModuleState or []*ModuleState. The former means
+	// we're moving just one module. The latter means we're moving a module
+	// and children.
+	root := raw
+	var rest []*ModuleState
+	if list, ok := raw.([]*ModuleState); ok {
+		// We need at least one item
+		if len(list) == 0 {
+			return fmt.Errorf("module move with no value to: %s", addr)
+		}
+
+		// The first item is always the root
+		root = list[0]
+		if len(list) > 1 {
+			rest = list[1:]
+		}
+	}
+
+	// Get the actual module state
+	src := root.(*ModuleState).deepcopy()
 
 	// If the target module exists, it is an error
 	path := append([]string{"root"}, addr.Path...)
@@ -97,6 +121,22 @@ func stateAddFunc_Module_Module(s *State, fromAddr, addr *ResourceAddress, raw i
 		}
 	}
 
+	// Add all the children if we have them
+	for _, item := range rest {
+		// If item isn't a descendent of our root, then ignore it
+		if !src.IsDescendent(item) {
+			continue
+		}
+
+		// It is! Strip the leading prefix and attach that to our address
+		extra := item.Path[len(src.Path):]
+		addrCopy := addr.Copy()
+		addrCopy.Path = append(addrCopy.Path, extra...)
+
+		// Add it
+		s.Add(fromAddr.String(), addrCopy.String(), item)
+	}
+
 	return nil
 }
 
@@ -111,6 +151,36 @@ func stateAddFunc_Resource_Module(
 }
 
 func stateAddFunc_Resource_Resource(s *State, fromAddr, addr *ResourceAddress, raw interface{}) error {
+	// raw can be either *ResourceState or []*ResourceState. The former means
+	// we're moving just one resource. The latter means we're moving a count
+	// of resources.
+	if list, ok := raw.([]*ResourceState); ok {
+		// We need at least one item
+		if len(list) == 0 {
+			return fmt.Errorf("resource move with no value to: %s", addr)
+		}
+
+		// If there is an index, this is an error since we can't assign
+		// a set of resources to a single index
+		if addr.Index >= 0 && len(list) > 1 {
+			return fmt.Errorf(
+				"multiple resources can't be moved to a single index: "+
+					"%s => %s", fromAddr, addr)
+		}
+
+		// Add each with a specific index
+		for i, rs := range list {
+			addrCopy := addr.Copy()
+			addrCopy.Index = i
+
+			if err := s.Add(fromAddr.String(), addrCopy.String(), rs); err != nil {
+				return err
+			}
+		}
+
+		return nil
+	}
+
 	src := raw.(*ResourceState).deepcopy()
 
 	// Initialize the resource
@@ -227,8 +297,12 @@ func detectValueAddLoc(raw interface{}) stateAddLoc {
 	switch raw.(type) {
 	case *ModuleState:
 		return stateAddModule
+	case []*ModuleState:
+		return stateAddModule
 	case *ResourceState:
 		return stateAddResource
+	case []*ResourceState:
+		return stateAddResource
 	case *InstanceState:
 		return stateAddInstance
 	default:

terraform/state_add_test.go

@@ -194,6 +194,90 @@ func TestStateAdd(t *testing.T) {
 			nil,
 		},
 
+		"ModuleState with children => Module Addr (new)": {
+			false,
+			"module.foo",
+			"module.bar",
+
+			[]*ModuleState{
+				&ModuleState{
+					Path:      []string{"root", "foo"},
+					Resources: map[string]*ResourceState{},
+				},
+
+				&ModuleState{
+					Path: []string{"root", "foo", "child1"},
+					Resources: map[string]*ResourceState{
+						"test_instance.foo": &ResourceState{
+							Type: "test_instance",
+							Primary: &InstanceState{
+								ID: "foo",
+							},
+						},
+					},
+				},
+
+				&ModuleState{
+					Path: []string{"root", "foo", "child2"},
+					Resources: map[string]*ResourceState{
+						"test_instance.foo": &ResourceState{
+							Type: "test_instance",
+							Primary: &InstanceState{
+								ID: "foo",
+							},
+						},
+					},
+				},
+
+				// Should be ignored
+				&ModuleState{
+					Path: []string{"root", "baz", "child2"},
+					Resources: map[string]*ResourceState{
+						"test_instance.foo": &ResourceState{
+							Type: "test_instance",
+							Primary: &InstanceState{
+								ID: "foo",
+							},
+						},
+					},
+				},
+			},
+
+			&State{},
+			&State{
+				Modules: []*ModuleState{
+					&ModuleState{
+						Path:      []string{"root", "bar"},
+						Resources: map[string]*ResourceState{},
+					},
+
+					&ModuleState{
+						Path: []string{"root", "bar", "child1"},
+						Resources: map[string]*ResourceState{
+							"test_instance.foo": &ResourceState{
+								Type: "test_instance",
+								Primary: &InstanceState{
+									ID: "foo",
+								},
+							},
+						},
+					},
+
+					&ModuleState{
+						Path: []string{"root", "bar", "child2"},
+						Resources: map[string]*ResourceState{
+							"test_instance.foo": &ResourceState{
+								Type: "test_instance",
+								Primary: &InstanceState{
+									ID: "foo",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+
 		"ResourceState => Resource Addr (new)": {
 			false,
 			"aws_instance.bar",
@@ -287,6 +371,135 @@ func TestStateAdd(t *testing.T) {
 			},
 		},
 
+		"ResourceState with count unspecified => Resource Addr (new)": {
+			false,
+			"aws_instance.bar",
+			"aws_instance.foo",
+			[]*ResourceState{
+				&ResourceState{
+					Type: "test_instance",
+					Primary: &InstanceState{
+						ID: "foo",
+					},
+				},
+
+				&ResourceState{
+					Type: "test_instance",
+					Primary: &InstanceState{
+						ID: "bar",
+					},
+				},
+			},
+
+			&State{},
+			&State{
+				Modules: []*ModuleState{
+					&ModuleState{
+						Path: []string{"root"},
+						Resources: map[string]*ResourceState{
+							"aws_instance.foo.0": &ResourceState{
+								Type: "test_instance",
+								Primary: &InstanceState{
+									ID: "foo",
+								},
+							},
+
+							"aws_instance.foo.1": &ResourceState{
+								Type: "test_instance",
+								Primary: &InstanceState{
+									ID: "bar",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+
+		"ResourceState with count unspecified => Resource Addr (new with count)": {
+			true,
+			"aws_instance.bar",
+			"aws_instance.foo[0]",
+			[]*ResourceState{
+				&ResourceState{
+					Type: "test_instance",
+					Primary: &InstanceState{
+						ID: "foo",
+					},
+				},
+
+				&ResourceState{
+					Type: "test_instance",
+					Primary: &InstanceState{
+						ID: "bar",
+					},
+				},
+			},
+
+			&State{},
+			nil,
+		},
+
+		"ResourceState with single count unspecified => Resource Addr (new with count)": {
+			false,
+			"aws_instance.bar",
+			"aws_instance.foo[0]",
+			[]*ResourceState{
+				&ResourceState{
+					Type: "test_instance",
+					Primary: &InstanceState{
+						ID: "foo",
+					},
+				},
+			},
+
+			&State{},
+			&State{
+				Modules: []*ModuleState{
+					&ModuleState{
+						Path: []string{"root"},
+						Resources: map[string]*ResourceState{
+							"aws_instance.foo.0": &ResourceState{
+								Type: "test_instance",
+								Primary: &InstanceState{
+									ID: "foo",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+
+		"ResourceState => Resource Addr (new with count)": {
+			false,
+			"aws_instance.bar",
+			"aws_instance.foo[0]",
+			&ResourceState{
+				Type: "test_instance",
+				Primary: &InstanceState{
+					ID: "foo",
+				},
+			},
+
+			&State{},
+			&State{
+				Modules: []*ModuleState{
+					&ModuleState{
+						Path: []string{"root"},
+						Resources: map[string]*ResourceState{
+							"aws_instance.foo.0": &ResourceState{
+								Type: "test_instance",
+								Primary: &InstanceState{
+									ID: "foo",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+
 		"ResourceState => Resource Addr (existing)": {
 			true,
 			"aws_instance.bar",
@@ -420,8 +633,8 @@ func TestStateAdd(t *testing.T) {
 
 		// Verify equality
 		if !tc.One.Equal(tc.Two) {
-			t.Fatalf("Bad: %s\n\n%#v\n\n%#v", k, tc.One, tc.Two)
-			//t.Fatalf("Bad: %s\n\n%s\n\n%s", k, tc.One.String(), tc.Two.String())
+			//t.Fatalf("Bad: %s\n\n%#v\n\n%#v", k, tc.One, tc.Two)
+			t.Fatalf("Bad: %s\n\n%s\n\n%s", k, tc.One.String(), tc.Two.String())
 		}
 	}
 }

terraform/state_filter_test.go

@@ -92,6 +92,33 @@ func TestStateFilterFilter(t *testing.T) {
 				"*terraform.InstanceState: module.consul.aws_instance.consul-green[0]",
 			},
 		},
+
+		"no count index": {
+			"complete.tfstate",
+			[]string{"module.consul.aws_instance.consul-green"},
+			[]string{
+				"*terraform.ResourceState: module.consul.aws_instance.consul-green[0]",
+				"*terraform.InstanceState: module.consul.aws_instance.consul-green[0]",
+				"*terraform.ResourceState: module.consul.aws_instance.consul-green[1]",
+				"*terraform.InstanceState: module.consul.aws_instance.consul-green[1]",
+				"*terraform.ResourceState: module.consul.aws_instance.consul-green[2]",
+				"*terraform.InstanceState: module.consul.aws_instance.consul-green[2]",
+			},
+		},
+
+		"nested modules": {
+			"nested-modules.tfstate",
+			[]string{"module.outer"},
+			[]string{
+				"*terraform.ModuleState: module.outer",
+				"*terraform.ModuleState: module.outer.module.child1",
+				"*terraform.ResourceState: module.outer.module.child1.aws_instance.foo",
+				"*terraform.InstanceState: module.outer.module.child1.aws_instance.foo",
+				"*terraform.ModuleState: module.outer.module.child2",
+				"*terraform.ResourceState: module.outer.module.child2.aws_instance.foo",
+				"*terraform.InstanceState: module.outer.module.child2.aws_instance.foo",
+			},
+		},
 	}
 
 	for n, tc := range cases {

terraform/test-fixtures/state-filter/nested-modules.tfstate

@@ -0,0 +1,47 @@
+{
+    "version": 1,
+    "serial": 12,
+    "modules": [
+        {
+            "path": [
+                "root",
+                "outer"
+            ],
+            "resources": {}
+        },
+        {
+            "path": [
+                "root",
+                "outer",
+                "child1"
+            ],
+            "resources": {
+                "aws_instance.foo": {
+                    "type": "aws_instance",
+                    "depends_on": [],
+                    "primary": {
+                        "id": "1",
+                        "attributes": {}
+                    }
+                }
+            }
+        },
+        {
+            "path": [
+                "root",
+                "outer",
+                "child2"
+            ],
+            "resources": {
+                "aws_instance.foo": {
+                    "type": "aws_instance",
+                    "depends_on": [],
+                    "primary": {
+                        "id": "1",
+                        "attributes": {}
+                    }
+                }
+            }
+        }
+    ]
+}