From 5ffa0839f9fe18087b9c51b100acb2fcd15c848d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 13 Oct 2021 17:07:26 -0400 Subject: [PATCH 1/2] only check serial when applying the first plan Ensure that we still check for a stale plan even when it was created with no previous state. Create separate errors for incorrect lineage vs incorrect serial. To prevent confusion when applying a first plan multiple times, only report it as a stale plan rather than different lineage. --- internal/backend/local/backend_local.go | 29 ++++++++++++++++++------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 209757e433..6082bfdf6c 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -290,14 +290,27 @@ func (b *Local) localRunForPlanFile(op *backend.Operation, pf *planfile.Reader, // has changed since the plan was created. (All of the "real-world" // state manager implementations support this, but simpler test backends // may not.) - if currentStateMeta.Lineage != "" && priorStateFile.Lineage != "" { - if priorStateFile.Serial != currentStateMeta.Serial || priorStateFile.Lineage != currentStateMeta.Lineage { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Saved plan is stale", - "The given plan file can no longer be applied because the state was changed by another operation after the plan was created.", - )) - } + + // Because the plan always contains a state, even if it is empty, the + // first plan to be applied will have empty snapshot metadata. In this + // case we compare only the serial in order to provide a more correct + // error. + firstPlan := priorStateFile.Lineage == "" && priorStateFile.Serial == 0 + + switch { + case !firstPlan && priorStateFile.Lineage != currentStateMeta.Lineage: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Saved plan does not match the given state", + "The given plan file can not be applied because it was created from a different state lineage.", + )) + + case priorStateFile.Serial != currentStateMeta.Serial: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Saved plan is stale", + "The given plan file can no longer be applied because the state was changed by another operation after the plan was created.", + )) } } // When we're applying a saved plan, the input state is the "prior state" From 9c80574417f48d34313d7608ec56d48ee51f8bdd Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 13 Oct 2021 17:28:14 -0400 Subject: [PATCH 2/2] test planfile may need to have a specific lineage In order to test applying a plan from an existing state, we need to be able to inject the state meta into the planfile. --- internal/command/apply_test.go | 19 ++++++++++++++++--- internal/command/command_test.go | 10 ++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/internal/command/apply_test.go b/internal/command/apply_test.go index 750c6b6bf3..b2306623cb 100644 --- a/internal/command/apply_test.go +++ b/internal/command/apply_test.go @@ -710,7 +710,6 @@ func TestApply_plan(t *testing.T) { } func TestApply_plan_backup(t *testing.T) { - planPath := applyFixturePlanFile(t) statePath := testTempFile(t) backupPath := testTempFile(t) @@ -724,11 +723,17 @@ func TestApply_plan_backup(t *testing.T) { } // create a state file that needs to be backed up - err := statemgr.NewFilesystem(statePath).WriteState(states.NewState()) + fs := statemgr.NewFilesystem(statePath) + fs.StateSnapshotMeta() + err := fs.WriteState(states.NewState()) if err != nil { t.Fatal(err) } + // the plan file must contain the metadata from the prior state to be + // backed up + planPath := applyFixturePlanFileMatchState(t, fs.StateSnapshotMeta()) + args := []string{ "-state", statePath, "-backup", backupPath, @@ -2280,6 +2285,13 @@ func applyFixtureProvider() *terraform.MockProvider { // a single change to create the test_instance.foo that is included in the // "apply" test fixture, returning the location of that plan file. func applyFixturePlanFile(t *testing.T) string { + return applyFixturePlanFileMatchState(t, statemgr.SnapshotMeta{}) +} + +// applyFixturePlanFileMatchState creates a planfile like applyFixturePlanFile, +// but inserts the state meta information if that plan must match a preexisting +// state. +func applyFixturePlanFileMatchState(t *testing.T, stateMeta statemgr.SnapshotMeta) string { _, snap := testModuleWithSnapshot(t, "apply") plannedVal := cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), @@ -2310,11 +2322,12 @@ func applyFixturePlanFile(t *testing.T) string { After: plannedValRaw, }, }) - return testPlanFile( + return testPlanFileMatchState( t, snap, states.NewState(), plan, + stateMeta, ) } diff --git a/internal/command/command_test.go b/internal/command/command_test.go index a9ba2a044e..4b6fcf311c 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -229,15 +229,21 @@ func testPlan(t *testing.T) *plans.Plan { } func testPlanFile(t *testing.T, configSnap *configload.Snapshot, state *states.State, plan *plans.Plan) string { + return testPlanFileMatchState(t, configSnap, state, plan, statemgr.SnapshotMeta{}) +} + +func testPlanFileMatchState(t *testing.T, configSnap *configload.Snapshot, state *states.State, plan *plans.Plan, stateMeta statemgr.SnapshotMeta) string { t.Helper() stateFile := &statefile.File{ - Lineage: "", + Lineage: stateMeta.Lineage, + Serial: stateMeta.Serial, State: state, TerraformVersion: version.SemVer, } prevStateFile := &statefile.File{ - Lineage: "", + Lineage: stateMeta.Lineage, + Serial: stateMeta.Serial, State: state, // we just assume no changes detected during refresh TerraformVersion: version.SemVer, }