From a1727ec4c2e6c40848b9b90e013b0342cce0d5db Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 17 Jul 2017 10:35:48 -0400 Subject: [PATCH] Add warning to mismatched plan state Forward-port the plan state check from the 0.9 series. 0.10 has improved the serial handling for the state, so this adds relevant comments and some more test coverage for the case of an incrementing serial during apply. --- backend/local/backend_local.go | 4 +++ terraform/plan.go | 23 +++++++++++++--- terraform/plan_test.go | 49 ++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/backend/local/backend_local.go b/backend/local/backend_local.go index 0b829b611..2c121d2e6 100644 --- a/backend/local/backend_local.go +++ b/backend/local/backend_local.go @@ -49,6 +49,10 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, state.State, } // Load our state + // By the time we get here, the backend creation code in "command" took + // care of making s.State() return a state compatible with our plan, + // if any, so we can safely pass this value in both the plan context + // and new context cases below. opts.State = s.State() // Build the context diff --git a/terraform/plan.go b/terraform/plan.go index 31b26bb68..51d66529b 100644 --- a/terraform/plan.go +++ b/terraform/plan.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "log" "sync" "github.com/hashicorp/terraform/config/module" @@ -43,7 +44,11 @@ type Plan struct { // Context returns a Context with the data encapsulated in this plan. // // The following fields in opts are overridden by the plan: Config, -// Diff, State, Variables. +// Diff, Variables. +// +// If State is not provided, it is set from the plan. If it _is_ provided, +// it must be Equal to the state stored in plan, but may have a newer +// serial. func (p *Plan) Context(opts *ContextOpts) (*Context, error) { var err error opts, err = p.contextOpts(opts) @@ -60,11 +65,23 @@ func (p *Plan) contextOpts(base *ContextOpts) (*ContextOpts, error) { opts.Diff = p.Diff opts.Module = p.Module - opts.State = p.State opts.Targets = p.Targets - opts.ProviderSHA256s = p.ProviderSHA256s + if opts.State == nil { + opts.State = p.State + } else if !opts.State.Equal(p.State) { + // Even if we're overriding the state, it should be logically equal + // to what's in plan. The only valid change to have made by the time + // we get here is to have incremented the serial. + // + // Due to the fact that serialization may change the representation of + // the state, there is little chance that these aren't actually equal. + // Log the error condition for reference, but continue with the state + // we have. + log.Println("[WARNING] Plan state and ContextOpts state are not equal") + } + thisVersion := VersionString() if p.TerraformVersion != "" && p.TerraformVersion != thisVersion { return nil, fmt.Errorf( diff --git a/terraform/plan_test.go b/terraform/plan_test.go index 4f654f6a0..5de506165 100644 --- a/terraform/plan_test.go +++ b/terraform/plan_test.go @@ -118,3 +118,52 @@ func TestReadWritePlan(t *testing.T) { t.Fatalf("bad:\n\n%s\n\nexpected:\n\n%s", actualStr, expectedStr) } } + +func TestPlanContextOptsOverrideStateGood(t *testing.T) { + plan := &Plan{ + Diff: &Diff{ + Modules: []*ModuleDiff{ + { + Path: []string{"test"}, + }, + }, + }, + Module: module.NewTree("test", nil), + State: &State{ + TFVersion: "sigil", + Serial: 1, + }, + Vars: map[string]interface{}{"foo": "bar"}, + Targets: []string{"baz"}, + + TerraformVersion: VersionString(), + ProviderSHA256s: map[string][]byte{ + "test": []byte("placeholder"), + }, + } + + base := &ContextOpts{ + State: &State{ + TFVersion: "sigil", + Serial: 2, + }, + } + + got, err := plan.contextOpts(base) + if err != nil { + t.Fatalf("error creating context: %s", err) + } + + want := &ContextOpts{ + Diff: plan.Diff, + Module: plan.Module, + State: base.State, + Variables: plan.Vars, + Targets: plan.Targets, + ProviderSHA256s: plan.ProviderSHA256s, + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("wrong result\ngot: %#v\nwant %#v", got, want) + } +}