plans: Track both the previous run and prior states in the plan

Until now we've not really cared much about the state snapshot produced
by the previous Terraform operation, except to use it as a jumping-off
point for our refresh step.

However, we'd like to be able to report to an end-user whenever Terraform
detects a change that occurred outside of Terraform, because that's often
helpful context for understanding why a plan contains changes that don't
seem to have corresponding changes in the configuration.

As part of reporting that we'll need to keep track of the state as it
was before we did any refreshing work, so we can then compare that against
the state after refreshing. To retain enough data to achieve that, the
existing Plan field State is now two fields: PrevRunState and PriorState.

This also includes a very shallow change in the core package to make it
populate something somewhat-reasonable into this field so that integration
tests can function reasonably. However, this shallow implementation isn't
really sufficient for real-world use of PrevRunState because we'll
actually need to update PrevRunState as part of planning in order to
incorporate the results of any provider-specific state upgrades to make
the PrevRunState objects compatible with the current provider schema, or
else our diffs won't be valid. This deeper awareness of PrevRunState in
Terraform Core will follow in a subsequent commit, prior to anything else
making use of Plan.PrevRunState.

backend/local/backend_plan.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/terraform/backend"
 	"github.com/hashicorp/terraform/plans"
 	"github.com/hashicorp/terraform/plans/planfile"
+	"github.com/hashicorp/terraform/states/statefile"
 	"github.com/hashicorp/terraform/states/statemgr"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/hashicorp/terraform/tfdiags"
@@ -115,10 +116,22 @@ func (b *Local) opPlan(
 		// We may have updated the state in the refresh step above, but we
 		// will freeze that updated state in the plan file for now and
 		// only write it if this plan is subsequently applied.
-		plannedStateFile := statemgr.PlannedStateUpdate(opState, plan.State)
+		plannedStateFile := statemgr.PlannedStateUpdate(opState, plan.PriorState)
+
+		// We also include a file containing the state as it existed before
+		// we took any action at all, but this one isn't intended to ever
+		// be saved to the backend (an equivalent snapshot should already be
+		// there) and so we just use a stub state file header in this case.
+		// NOTE: This won't be exactly identical to the latest state snapshot
+		// in the backend because it's still been subject to state upgrading
+		// to make it consumable by the current Terraform version, and
+		// intentionally doesn't preserve the header info.
+		prevStateFile := &statefile.File{
+			State: plan.PrevRunState,
+		}
 
 		log.Printf("[INFO] backend/local: writing plan output to: %s", path)
-		err := planfile.Create(path, configSnap, plannedStateFile, plan)
+		err := planfile.Create(path, configSnap, prevStateFile, plannedStateFile, plan)
 		if err != nil {
 			diags = diags.Append(tfdiags.Sourceless(
 				tfdiags.Error,
@@ -140,7 +153,7 @@ func (b *Local) opPlan(
 	}
 
 	// Render the plan
-	op.View.Plan(plan, plan.State, tfCtx.Schemas())
+	op.View.Plan(plan, plan.PriorState, tfCtx.Schemas())
 
 	// If we've accumulated any warnings along the way then we'll show them
 	// here just before we show the summary and next steps. If we encountered

command/command_test.go

@@ -182,9 +182,14 @@ func testPlanFile(t *testing.T, configSnap *configload.Snapshot, state *states.S
 		State:            state,
 		TerraformVersion: version.SemVer,
 	}
+	prevStateFile := &statefile.File{
+		Lineage:          "",
+		State:            state, // we just assume no changes detected during refresh
+		TerraformVersion: version.SemVer,
+	}
 
 	path := testTempFile(t)
-	err := planfile.Create(path, configSnap, stateFile, plan)
+	err := planfile.Create(path, configSnap, prevStateFile, stateFile, plan)
 	if err != nil {
 		t.Fatalf("failed to create temporary plan file: %s", err)
 	}

plans/plan.go

@@ -35,7 +35,24 @@ type Plan struct {
 	ForceReplaceAddrs []addrs.AbsResourceInstance
 	ProviderSHA256s   map[string][]byte
 	Backend           Backend
-	State             *states.State
+
+	// PrevRunState and PriorState both describe the situation that the plan
+	// was derived from:
+	//
+	// PrevRunState is a representation of the outcome of the previous
+	// Terraform operation, without any updates from the remote system but
+	// potentially including some changes that resulted from state upgrade
+	// actions.
+	//
+	// PriorState is a representation of the current state of remote objects,
+	// which will differ from PrevRunState if the "refresh" step returned
+	// different data, which might reflect drift.
+	//
+	// PriorState is the main snapshot we use for actions during apply.
+	// PrevRunState is only here so that we can diff PriorState against it in
+	// order to report to the user any out-of-band changes we've detected.
+	PrevRunState *states.State
+	PriorState   *states.State
 }
 
 // Backend represents the backend-related configuration and other data as it

plans/planfile/planfile_test.go

@@ -3,10 +3,9 @@ package planfile
 import (
 	"io/ioutil"
 	"path/filepath"
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
+	"github.com/google/go-cmp/cmp"
 
 	"github.com/hashicorp/terraform/configs/configload"
 	"github.com/hashicorp/terraform/plans"
@@ -33,6 +32,12 @@ func TestRoundtrip(t *testing.T) {
 	// We don't need to test the entire thing because the state file
 	// serialization is already tested in its own package.
 	stateFileIn := &statefile.File{
+		TerraformVersion: tfversion.SemVer,
+		Serial:           2,
+		Lineage:          "abc123",
+		State:            states.NewState(),
+	}
+	prevStateFileIn := &statefile.File{
 		TerraformVersion: tfversion.SemVer,
 		Serial:           1,
 		Lineage:          "abc123",
@@ -63,7 +68,7 @@ func TestRoundtrip(t *testing.T) {
 	}
 	planFn := filepath.Join(workDir, "tfplan")
 
-	err = Create(planFn, snapIn, stateFileIn, planIn)
+	err = Create(planFn, snapIn, prevStateFileIn, stateFileIn, planIn)
 	if err != nil {
 		t.Fatalf("failed to create plan file: %s", err)
 	}
@@ -78,8 +83,8 @@ func TestRoundtrip(t *testing.T) {
 		if err != nil {
 			t.Fatalf("failed to read plan: %s", err)
 		}
-		if !reflect.DeepEqual(planIn, planOut) {
-			t.Errorf("plan did not survive round-trip\nresult: %sinput: %s", spew.Sdump(planOut), spew.Sdump(planIn))
+		if diff := cmp.Diff(planIn, planOut); diff != "" {
+			t.Errorf("plan did not survive round-trip\n%s", diff)
 		}
 	})
 
@@ -88,8 +93,18 @@ func TestRoundtrip(t *testing.T) {
 		if err != nil {
 			t.Fatalf("failed to read state: %s", err)
 		}
-		if !reflect.DeepEqual(stateFileIn, stateFileOut) {
-			t.Errorf("state file did not survive round-trip\nresult: %sinput: %s", spew.Sdump(stateFileOut), spew.Sdump(stateFileIn))
+		if diff := cmp.Diff(stateFileIn, stateFileOut); diff != "" {
+			t.Errorf("state file did not survive round-trip\n%s", diff)
+		}
+	})
+
+	t.Run("ReadPrevStateFile", func(t *testing.T) {
+		prevStateFileOut, err := pr.ReadPrevStateFile()
+		if err != nil {
+			t.Fatalf("failed to read state: %s", err)
+		}
+		if diff := cmp.Diff(prevStateFileIn, prevStateFileOut); diff != "" {
+			t.Errorf("state file did not survive round-trip\n%s", diff)
 		}
 	})
 
@@ -98,8 +113,8 @@ func TestRoundtrip(t *testing.T) {
 		if err != nil {
 			t.Fatalf("failed to read config snapshot: %s", err)
 		}
-		if !reflect.DeepEqual(snapIn, snapOut) {
-			t.Errorf("config snapshot did not survive round-trip\nresult: %sinput: %s", spew.Sdump(snapOut), spew.Sdump(snapIn))
+		if diff := cmp.Diff(snapIn, snapOut); diff != "" {
+			t.Errorf("config snapshot did not survive round-trip\n%s", diff)
 		}
 	})
 

plans/planfile/reader.go

@@ -14,6 +14,7 @@ import (
 )
 
 const tfstateFilename = "tfstate"
+const tfstatePreviousFilename = "tfstate-prev"
 
 // Reader is the main type used to read plan files. Create a Reader by calling
 // Open.
@@ -90,7 +91,8 @@ func (r *Reader) ReadPlan() (*plans.Plan, error) {
 	return readTfplan(pr)
 }
 
-// ReadStateFile reads the state file embedded in the plan file.
+// ReadStateFile reads the state file embedded in the plan file, which
+// represents the "PriorState" as defined in plans.Plan.
 //
 // If the plan file contains no embedded state file, the returned error is
 // statefile.ErrNoState.
@@ -107,6 +109,24 @@ func (r *Reader) ReadStateFile() (*statefile.File, error) {
 	return nil, statefile.ErrNoState
 }
 
+// ReadPrevStateFile reads the previous state file embedded in the plan file, which
+// represents the "PrevRunState" as defined in plans.Plan.
+//
+// If the plan file contains no embedded previous state file, the returned error is
+// statefile.ErrNoState.
+func (r *Reader) ReadPrevStateFile() (*statefile.File, error) {
+	for _, file := range r.zip.File {
+		if file.Name == tfstatePreviousFilename {
+			r, err := file.Open()
+			if err != nil {
+				return nil, fmt.Errorf("failed to extract previous state from plan file: %s", err)
+			}
+			return statefile.Read(r)
+		}
+	}
+	return nil, statefile.ErrNoState
+}
+
 // ReadConfigSnapshot reads the configuration snapshot embedded in the plan
 // file.
 //

plans/planfile/writer.go

@@ -18,7 +18,7 @@ import (
 // state file in addition to the plan itself, so that Terraform can detect
 // if the world has changed since the plan was created and thus refuse to
 // apply it.
-func Create(filename string, configSnap *configload.Snapshot, stateFile *statefile.File, plan *plans.Plan) error {
+func Create(filename string, configSnap *configload.Snapshot, prevStateFile, stateFile *statefile.File, plan *plans.Plan) error {
 	f, err := os.Create(filename)
 	if err != nil {
 		return err
@@ -60,6 +60,22 @@ func Create(filename string, configSnap *configload.Snapshot, stateFile *statefi
 		}
 	}
 
+	// tfstate-prev file
+	{
+		w, err := zw.CreateHeader(&zip.FileHeader{
+			Name:     tfstatePreviousFilename,
+			Method:   zip.Deflate,
+			Modified: time.Now(),
+		})
+		if err != nil {
+			return fmt.Errorf("failed to create embedded tfstate-prev file: %s", err)
+		}
+		err = statefile.Write(prevStateFile, w)
+		if err != nil {
+			return fmt.Errorf("failed to write previous state snapshot: %s", err)
+		}
+	}
+
 	// tfconfig directory
 	{
 		err := writeConfigSnapshot(configSnap, zw)

terraform/context.go

@@ -631,6 +631,8 @@ The -target option is not for routine use, and is provided only for exceptional
 func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
+	prevRunState := c.state.DeepCopy()
+
 	graph, graphDiags := c.Graph(GraphTypePlan, nil)
 	diags = diags.Append(graphDiags)
 	if graphDiags.HasErrors() {
@@ -648,12 +650,13 @@ func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) {
 		UIMode:            plans.NormalMode,
 		Changes:           c.changes,
 		ForceReplaceAddrs: c.forceReplace,
+		PrevRunState:      prevRunState,
 	}
 
 	c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects()
 
 	refreshedState := c.refreshState.DeepCopy()
-	plan.State = refreshedState
+	plan.PriorState = refreshedState
 
 	// replace the working state with the updated state, so that immediate calls
 	// to Apply work as expected.
@@ -665,7 +668,8 @@ func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) {
 func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 	destroyPlan := &plans.Plan{
-		State: c.state.DeepCopy(),
+		PrevRunState: c.state.DeepCopy(),
+		PriorState:   c.state.DeepCopy(),
 	}
 	c.changes = plans.NewChanges()
 
@@ -682,7 +686,7 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) {
 
 		// insert the refreshed state into the destroy plan result, and discard
 		// the changes recorded from the refresh.
-		destroyPlan.State = refreshPlan.State
+		destroyPlan.PriorState = refreshPlan.PriorState.DeepCopy()
 		c.changes = plans.NewChanges()
 	}
 
@@ -708,6 +712,8 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) {
 func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
+	prevRunState := c.state.DeepCopy()
+
 	graph, graphDiags := c.Graph(GraphTypePlanRefreshOnly, nil)
 	diags = diags.Append(graphDiags)
 	if graphDiags.HasErrors() {
@@ -722,8 +728,9 @@ func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) {
 		return nil, diags
 	}
 	plan := &plans.Plan{
-		UIMode:  plans.RefreshOnlyMode,
-		Changes: c.changes,
+		UIMode:       plans.RefreshOnlyMode,
+		Changes:      c.changes,
+		PrevRunState: prevRunState,
 	}
 
 	// If the graph builder and graph nodes correctly obeyed our directive
@@ -740,11 +747,12 @@ func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) {
 
 	c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects()
 
-	refreshedState := c.refreshState.DeepCopy()
-	plan.State = refreshedState
+	refreshedState := c.refreshState
+	plan.PriorState = refreshedState.DeepCopy()
 
 	// replace the working state with the updated state, so that immediate calls
-	// to Apply work as expected.
+	// to Apply work as expected. DeepCopy because such an apply should not
+	// mutate
 	c.state = refreshedState
 
 	return plan, diags
@@ -761,7 +769,7 @@ func (c *Context) Refresh() (*states.State, tfdiags.Diagnostics) {
 		return nil, diags
 	}
 
-	return p.State, diags
+	return p.PriorState, diags
 }
 
 // Stop stops the running task.

terraform/context_apply2_test.go

@@ -333,22 +333,22 @@ resource "aws_instance" "bin" {
 		t.Fatal(diags.Err())
 	}
 
-	bar := plan.State.ResourceInstance(barAddr)
+	bar := plan.PriorState.ResourceInstance(barAddr)
 	if len(bar.Current.Dependencies) == 0 || !bar.Current.Dependencies[0].Equal(fooAddr.ContainingResource().Config()) {
 		t.Fatalf("bar should depend on foo after refresh, but got %s", bar.Current.Dependencies)
 	}
 
-	foo := plan.State.ResourceInstance(fooAddr)
+	foo := plan.PriorState.ResourceInstance(fooAddr)
 	if len(foo.Current.Dependencies) == 0 || !foo.Current.Dependencies[0].Equal(bazAddr.ContainingResource().Config()) {
 		t.Fatalf("foo should depend on baz after refresh because of the update, but got %s", foo.Current.Dependencies)
 	}
 
-	bin := plan.State.ResourceInstance(binAddr)
+	bin := plan.PriorState.ResourceInstance(binAddr)
 	if len(bin.Current.Dependencies) != 0 {
 		t.Fatalf("bin should depend on nothing after refresh because there is no change, but got %s", bin.Current.Dependencies)
 	}
 
-	baz := plan.State.ResourceInstance(bazAddr)
+	baz := plan.PriorState.ResourceInstance(bazAddr)
 	if len(baz.Current.Dependencies) == 0 || !baz.Current.Dependencies[0].Equal(bamAddr.ContainingResource().Config()) {
 		t.Fatalf("baz should depend on bam after refresh, but got %s", baz.Current.Dependencies)
 	}

terraform/context_plan2_test.go

@@ -370,7 +370,7 @@ resource "test_object" "a" {
 		t.Fatal(diags.Err())
 	}
 
-	if plan.State == nil {
+	if plan.PriorState == nil {
 		t.Fatal("missing plan state")
 	}
 
@@ -545,7 +545,7 @@ func TestContext2Plan_refreshOnlyMode(t *testing.T) {
 		t.Fatalf("plan contains resource changes; want none\n%s", spew.Sdump(plan.Changes.Resources))
 	}
 
-	state = plan.State
+	state = plan.PriorState
 	instState := state.ResourceInstance(addr)
 	if instState == nil {
 		t.Fatalf("%s has no state at all after plan", addr)

terraform/context_plan_test.go

@@ -6332,9 +6332,9 @@ data "test_data_source" "d" {
 		t.Fatal(diags.ErrWithWarnings())
 	}
 
-	d := plan.State.ResourceInstance(mustResourceInstanceAddr("data.test_data_source.d"))
+	d := plan.PriorState.ResourceInstance(mustResourceInstanceAddr("data.test_data_source.d"))
 	if d == nil || d.Current == nil {
-		t.Fatal("data.test_data_source.d not found in state:", plan.State)
+		t.Fatal("data.test_data_source.d not found in state:", plan.PriorState)
 	}
 
 	if d.Current.Status != states.ObjectReady {