plans/planfile: Read state snapshots as part of reading a plan

Our model for plans/planfile has unfortunately grown inconsistent with
changes to our modeling of plans.Plan.

Originally we considered the plan "header" and the planned changes as an
entirely separate artifact from the prior state, but we later realized
that carrying the prior state around with the plan is important to
ensuring we always have enough context to faithfully render a plan to the
user, and so we added the prior state as a field of plans.Plan.
More recently we've also added the "previous run state" to plans.Plan for
similar reasons.

Unfortunately as a result of that modeling drift our ReadPlan method was
silently producing an incomplete plans.Plan object, causing use-cases like
"terraform show" to produce slightly different results due to the
plan object not round-tripping completely.

As a short-term tactical fix, here we add state snapshot reading into the
ReadPlan function. This is not an ideal solution because it means that
in the case of applying a plan, where we really do need access to the
state _file_, we'll end up reading the prior state file twice. However,
the goal here is only to heal the modelling quirk with as little change
as possible, because we're not currently at a point where we'd be willing
to risk regressions from a larger refactoring.
This commit is contained in:
Martin Atkins 2021-05-07 10:18:59 -07:00
parent 87c9e78666
commit 0f936b9d80
2 changed files with 38 additions and 1 deletions

View File

@ -60,6 +60,15 @@ func TestRoundtrip(t *testing.T) {
Config: plans.DynamicValue([]byte("config placeholder")),
Workspace: "default",
},
// Due to some historical oddities in how we've changed modelling over
// time, we also include the states (without the corresponding file
// headers) in the plans.Plan object. This is currently ignored by
// Create but will be returned by ReadPlan and so we need to include
// it here so that we'll get a match when we compare input and output
// below.
PrevRunState: prevStateFileIn.State,
PriorState: stateFileIn.State,
}
workDir, err := ioutil.TempDir("", "tf-planfile")

View File

@ -88,7 +88,35 @@ func (r *Reader) ReadPlan() (*plans.Plan, error) {
}
defer pr.Close()
return readTfplan(pr)
// There's a slight mismatch in how plans.Plan is modeled vs. how
// the underlying plan file format works, because the "tfplan" embedded
// file contains only some top-level metadata and the planned changes,
// and not the previous run or prior states. Therefore we need to
// build this up in multiple steps.
// This is some technical debt because historically we considered the
// planned changes and prior state as totally separate, but later realized
// that it made sense for a plans.Plan to include the prior state directly
// so we can see what state the plan applies to. Hopefully later we'll
// clean this up some more so that we don't have two different ways to
// access the prior state (this and the ReadStateFile method).
ret, err := readTfplan(pr)
if err != nil {
return nil, err
}
prevRunStateFile, err := r.ReadPrevStateFile()
if err != nil {
return nil, fmt.Errorf("failed to read previous run state from plan file: %s", err)
}
priorStateFile, err := r.ReadStateFile()
if err != nil {
return nil, fmt.Errorf("failed to read prior state from plan file: %s", err)
}
ret.PrevRunState = prevRunStateFile.State
ret.PriorState = priorStateFile.State
return ret, nil
}
// ReadStateFile reads the state file embedded in the plan file, which