[WIP] Safety features for Terraform State

command/apply_test.go

@@ -417,8 +417,35 @@ func TestApply_plan(t *testing.T) {
 
 	planPath := testPlanFile(t, &terraform.Plan{
 		Module: testModule(t, "apply"),
+		State: &terraform.State{
+			Version: 2,
+			Lineage: "TestApply_plan",
+			Serial:  1,
+			Modules: []*terraform.ModuleState{
+				&terraform.ModuleState{
+					Path:      []string{"root"},
+					Resources: map[string]*terraform.ResourceState{},
+				},
+			},
+		},
+	})
+
+	// Current state has the same lineage and serial as the plan state,
+	// so the plan is current and should apply successfully.
+	statePath := testStateFile(t, &terraform.State{
+		Version: 2,
+		Lineage: "TestApply_plan",
+		Serial:  1,
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path:      []string{"root"},
+				Resources: map[string]*terraform.ResourceState{},
+			},
+		},
 	})
-	statePath := testTempFile(t)
+
+	stateContents, _ := ioutil.ReadFile(statePath)
+	os.Stderr.Write(stateContents)
 
 	p := testProvider()
 	ui := new(cli.MockUi)
@@ -460,6 +487,94 @@ func TestApply_plan(t *testing.T) {
 	}
 }
 
+func TestApply_stalePlan(t *testing.T) {
+	// Disable test mode so input would be asked
+	test = false
+	defer func() { test = true }()
+
+	// Set some default reader/writers for the inputs
+	defaultInputReader = new(bytes.Buffer)
+	defaultInputWriter = new(bytes.Buffer)
+
+	planPath := testPlanFile(t, &terraform.Plan{
+		Module: testModule(t, "apply"),
+		State: &terraform.State{
+			Version: 2,
+			Lineage: "TestApply_stalePlan",
+			Serial:  1,
+		},
+	})
+
+	// Current state has a higher serial than the state in the
+	// plan, so the plan is "stale" and should fail to apply.
+	statePath := testStateFile(t, &terraform.State{
+		Version: 2,
+		Lineage: "TestApply_stalePlan",
+		Serial:  2,
+	})
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		planPath,
+	}
+	if code := c.Run(args); code == 0 {
+		t.Fatalf("bad: succeeded, but wanted failure")
+	}
+}
+
+func TestApply_unrelatedPlan(t *testing.T) {
+	// Disable test mode so input would be asked
+	test = false
+	defer func() { test = true }()
+
+	// Set some default reader/writers for the inputs
+	defaultInputReader = new(bytes.Buffer)
+	defaultInputWriter = new(bytes.Buffer)
+
+	planPath := testPlanFile(t, &terraform.Plan{
+		Module: testModule(t, "apply"),
+		State: &terraform.State{
+			Version: 2,
+			Lineage: "TestApply_unrelatedPlan",
+			Serial:  1,
+		},
+	})
+
+	// Current state has a different lineage than plan, so the plan
+	// should fail to apply.
+	statePath := testStateFile(t, &terraform.State{
+		Version: 2,
+		Lineage: "TestApply_unrelatedPlan_other",
+		Serial:  1,
+	})
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		planPath,
+	}
+	if code := c.Run(args); code == 0 {
+		t.Fatalf("bad: succeeded, but wanted failure")
+	}
+}
+
 func TestApply_plan_remoteState(t *testing.T) {
 	// Disable test mode so input would be asked
 	test = false

command/meta.go

@@ -114,6 +114,26 @@ func (m *Meta) Context(copts contextOpts) (*terraform.Context, bool, error) {
 				return nil, false, fmt.Errorf("Error loading plan: %s", err)
 			}
 
+			// The plan's state must mach our current state, to avoid
+			// accidentally re-applying a state or applying a plan
+			// that was generated against a different lineage of states.
+			if state != nil {
+				if planState := state.State(); planState != nil {
+					stateMatch, err := m.stateMatchesCurrent(planState)
+
+					if err != nil {
+						return nil, false, fmt.Errorf("Error verifying plan state: %s", err)
+					}
+					if !stateMatch {
+						return nil, false, fmt.Errorf(
+							"Plan does not apply to current state.\n\n" +
+								"Perhaps you have tried to apply a plan that was already applied,\n" +
+								"or somebody else has applied another plan first.\n",
+						)
+					}
+				}
+			}
+
 			// Set our state
 			m.state = state
 			m.stateOutPath = statePath
@@ -168,6 +188,33 @@ func (m *Meta) Context(copts contextOpts) (*terraform.Context, bool, error) {
 	return ctx, false, err
 }
 
+func (m *Meta) stateMatchesCurrent(state *terraform.State) (bool, error) {
+	currentStateResult, err := m.State()
+	if err != nil {
+		return false, err
+	}
+	currentState := currentStateResult.State()
+
+	// If we have no current state then no state can match.
+	if currentState == nil {
+		return false, nil
+	}
+
+	if !currentState.SameLineage(state) {
+		return false, nil
+	}
+
+	if currentState.Serial != state.Serial {
+		return false, nil
+	}
+
+	if !currentState.Equal(state) {
+		return false, nil
+	}
+
+	return true, nil
+}
+
 // DataDir returns the directory where local data will be stored.
 func (m *Meta) DataDir() string {
 	dataDir := DefaultDataDirectory

command/remote_config_test.go

@@ -406,6 +406,128 @@ func TestRemoteConfig_enableRemote(t *testing.T) {
 	testRemoteLocalBackup(t, true)
 }
 
+func TestRemoteConfig_conflictRemote(t *testing.T) {
+	tmp, cwd := testCwd(t)
+	defer testFixCwd(t, tmp, cwd)
+
+	s := terraform.NewState()
+	s.Serial = 5
+	// Conflict is detected only when state has resources
+	s.Modules = []*terraform.ModuleState{
+		{
+			Path: []string{"root"},
+			Resources: map[string]*terraform.ResourceState{
+				"test_instance.foo": {},
+			},
+		},
+	}
+	conf, srv := testRemoteState(t, s, 200)
+	defer srv.Close()
+
+	s = s.DeepCopy()
+	// Different lineage locally means this is a conflict, because remote
+	// already exists but is describing an unrelated state lineage.
+	s.Lineage = "different-lineage"
+	s.Remote = conf
+
+	// Store the local state
+	statePath := filepath.Join(tmp, DefaultDataDir, DefaultStateFilename)
+	if err := os.MkdirAll(filepath.Dir(statePath), 0755); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	f, err := os.Create(statePath)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	err = terraform.WriteState(s, f)
+	f.Close()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	ui := new(cli.MockUi)
+	c := &RemoteConfigCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(testProvider()),
+			Ui:          ui,
+		},
+	}
+	args := make([]string, 0, 5)
+	args = append(args, "-backend="+conf.Type)
+	for bck, bcv := range conf.Config {
+		args = append(args, "-backend-config", bck+"="+bcv)
+	}
+	if code := c.Run(args); code == 0 {
+		t.Fatalf("bad: got success, but want error")
+	}
+}
+
+func TestRemoteConfig_localEmptyRemoteExists(t *testing.T) {
+	tmp, cwd := testCwd(t)
+	defer testFixCwd(t, tmp, cwd)
+
+	s := terraform.NewState()
+	s.Lineage = "remote-lineage"
+	s.Serial = 5
+	// Conflict is detected only when state has resources
+	s.Modules = []*terraform.ModuleState{
+		{
+			Path: []string{"root"},
+			Resources: map[string]*terraform.ResourceState{
+				"test_instance.foo": {},
+			},
+		},
+	}
+	conf, srv := testRemoteState(t, s, 200)
+	defer srv.Close()
+
+	s = terraform.NewState()
+	// Different lineage locally would normally mean conflict, but
+	// we tolerate it in this case because the local state contains
+	// no resources. The user is probably just enabling remote state
+	// against an existing config for the first time.
+	s.Lineage = "local-lineage"
+	s.Modules = []*terraform.ModuleState{
+		{
+			Path:      []string{"root"},
+			Resources: map[string]*terraform.ResourceState{},
+		},
+	}
+
+	// Store the local state
+	// Note that this is is just 'terraform.tfstate', not
+	// '.terraform/terraform.tfstate'; remote state is not enabled yet.
+	statePath := filepath.Join(tmp, DefaultStateFilename)
+	if err := os.MkdirAll(filepath.Dir(statePath), 0755); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	f, err := os.Create(statePath)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	err = terraform.WriteState(s, f)
+	f.Close()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	ui := new(cli.MockUi)
+	c := &RemoteConfigCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(testProvider()),
+			Ui:          ui,
+		},
+	}
+	args := make([]string, 0, 5)
+	args = append(args, "-backend="+conf.Type)
+	for bck, bcv := range conf.Config {
+		args = append(args, "-backend-config", bck+"="+bcv)
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: \n%s", ui.ErrorWriter.String())
+	}
+}
+
 func testRemoteLocal(t *testing.T, exists bool) {
 	_, err := os.Stat(DefaultStateFilename)
 	if os.IsNotExist(err) && !exists {

command/remote_pull_test.go

@@ -41,7 +41,7 @@ func TestRemotePull_local(t *testing.T) {
 	s.Serial = 10
 	conf, srv := testRemoteState(t, s, 200)
 
-	s = terraform.NewState()
+	s = s.DeepCopy()
 	s.Serial = 5
 	s.Remote = conf
 	defer srv.Close()

command/remote_push_test.go

@@ -36,7 +36,7 @@ func TestRemotePush_local(t *testing.T) {
 	conf, srv := testRemoteState(t, s, 200)
 	defer srv.Close()
 
-	s = terraform.NewState()
+	s = s.DeepCopy()
 	s.Serial = 10
 	s.Remote = conf
 

command/state.go

@@ -255,6 +255,14 @@ func remoteState(
 				return nil, errwrap.Wrapf(
 					"Error preparing remote state: {{err}}", err)
 			}
+
+		case state.CacheRefreshConflict:
+			return nil, fmt.Errorf(
+				"%s.\n%s",
+				cache.RefreshResult(),
+				cacheRefreshConflictMessage,
+			)
+
 		default:
 			return nil, fmt.Errorf(
 				"Unknown refresh result: %s", cache.RefreshResult())
@@ -274,3 +282,30 @@ func remoteStateFromPath(path string, refresh bool) (*state.CacheState, error) {
 
 	return remoteState(localState, path, refresh)
 }
+
+// This is a scary but not-particularly-helpful error message. This situation
+// should not arise in legitimate usage, but if it *does* arise there isn't
+// really a "magic bullet" to fix it, so this message is primarily to explain
+// the gravity of the situation to the user.
+//
+// In future it would be nice to do more to prevent this situation from
+// arising. In medium-term, we could provide more elaborate advice on the
+// website and link to it here. For now, we're just focused on minimizing the
+// risk of accidental loss of state, so we do our best.
+const cacheRefreshConflictMessage = `
+This may mean that you have activated remote state while you already had
+resources locally, but the remote state also has resources. In this case,
+you must decide to keep either the local or remote resources.
+
+This may alternatively mean that two "refresh" or "apply" operations
+ran concurrently and have created divergent states. In this case, some
+resources may be duplicated across the two states.
+
+In either case, caution is advised. Your local state cache is preserved
+at .terraform/terraform.tfstate and it will not overwrite the remote until
+the confict is resolved.
+
+At this point it is safe to disable remote state, which will retain the
+local state on this computer and leave the remote state untouched:
+    terraform remote config -disable -pull=false
+`