From a19e7d72a6a9addf6534254da67d5b136d33d624 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 23 Oct 2020 13:06:17 -0400 Subject: [PATCH 1/4] states: Extract version check logic from read Instead of always checking the Terraform version associated with a state file when reading it, we add a CheckTerraformVersion method and call it from locations where we care about enforcing this check. For this commit, the check has been retained at all call sites for states/statefile.Read, with these exceptions: - Unit tests, which shouldn't care about the state file version; - E2E test runner which should always be using valid state files; - terraform.ShimLegacyState, where the check is pointless as the state file was just created by the current Terraform version. --- backend/remote/backend_state.go | 3 +++ command/show.go | 3 +++ command/state_push.go | 4 ++++ command/workspace_new.go | 4 ++++ plans/planfile/reader.go | 6 +++++- state/remote/state.go | 3 +++ states/statefile/file.go | 14 ++++++++++++++ states/statefile/read.go | 9 --------- states/statemgr/filesystem.go | 8 ++++++++ 9 files changed, 44 insertions(+), 10 deletions(-) diff --git a/backend/remote/backend_state.go b/backend/remote/backend_state.go index 29dc8550e1ce..2c08984ec366 100644 --- a/backend/remote/backend_state.go +++ b/backend/remote/backend_state.go @@ -64,6 +64,9 @@ func (r *remoteClient) Put(state []byte) error { if err != nil { return fmt.Errorf("Error reading state: %s", err) } + if err := stateFile.CheckTerraformVersion(); err != nil { + return fmt.Errorf("Incompatible statefile: %s", err) + } options := tfe.StateVersionCreateOptions{ Lineage: tfe.String(stateFile.Lineage), diff --git a/command/show.go b/command/show.go index d7b5f7ef837d..a4b74e116ed4 100644 --- a/command/show.go +++ b/command/show.go @@ -241,6 +241,9 @@ func getStateFromPath(path string) (*statefile.File, error) { if err != nil { return nil, fmt.Errorf("Error reading %s as a statefile: %s", path, err) } + if err := stateFile.CheckTerraformVersion(); err != nil { + return nil, fmt.Errorf("Incompatible statefile %s: %s", path, err) + } return stateFile, nil } diff --git a/command/state_push.go b/command/state_push.go index 640329a48164..c70c78d2e8d8 100644 --- a/command/state_push.go +++ b/command/state_push.go @@ -67,6 +67,10 @@ func (c *StatePushCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Error reading source state %q: %s", args[0], err)) return 1 } + if err := srcStateFile.CheckTerraformVersion(); err != nil { + c.Ui.Error(fmt.Sprintf("Incompatible statefile %q: %s", args[0], err)) + return 1 + } // Load the backend b, backendDiags := c.Backend(nil) diff --git a/command/workspace_new.go b/command/workspace_new.go index 0c477467c298..b300fcb0af7f 100644 --- a/command/workspace_new.go +++ b/command/workspace_new.go @@ -145,6 +145,10 @@ func (c *WorkspaceNewCommand) Run(args []string) int { c.Ui.Error(err.Error()) return 1 } + if err := stateFile.CheckTerraformVersion(); err != nil { + c.Ui.Error(err.Error()) + return 1 + } // save the existing state in the new Backend. err = stateMgr.WriteState(stateFile.State) diff --git a/plans/planfile/reader.go b/plans/planfile/reader.go index 579e2859996e..a6a34a430f94 100644 --- a/plans/planfile/reader.go +++ b/plans/planfile/reader.go @@ -101,7 +101,11 @@ func (r *Reader) ReadStateFile() (*statefile.File, error) { if err != nil { return nil, fmt.Errorf("failed to extract state from plan file: %s", err) } - return statefile.Read(r) + stateFile, err := statefile.Read(r) + if err == nil { + err = stateFile.CheckTerraformVersion() + } + return stateFile, err } } return nil, statefile.ErrNoState diff --git a/state/remote/state.go b/state/remote/state.go index 3069aeb892f0..652606eab524 100644 --- a/state/remote/state.go +++ b/state/remote/state.go @@ -125,6 +125,9 @@ func (s *State) refreshState() error { if err != nil { return err } + if err := stateFile.CheckTerraformVersion(); err != nil { + return err + } s.lineage = stateFile.Lineage s.serial = stateFile.Serial diff --git a/states/statefile/file.go b/states/statefile/file.go index 6e202401999b..95f52df69aee 100644 --- a/states/statefile/file.go +++ b/states/statefile/file.go @@ -1,6 +1,8 @@ package statefile import ( + "fmt" + version "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/states" @@ -60,3 +62,15 @@ func (f *File) DeepCopy() *File { State: f.State.DeepCopy(), } } + +func (f *File) CheckTerraformVersion() error { + if f.TerraformVersion != nil && f.TerraformVersion.GreaterThan(tfversion.SemVer) { + return fmt.Errorf( + "state snapshot was created by Terraform v%s, which is newer than current v%s; upgrade to Terraform v%s or greater to work with this state", + f.TerraformVersion, + tfversion.SemVer, + f.TerraformVersion, + ) + } + return nil +} diff --git a/states/statefile/read.go b/states/statefile/read.go index d691c0290d4b..8abd3be14da2 100644 --- a/states/statefile/read.go +++ b/states/statefile/read.go @@ -62,15 +62,6 @@ func Read(r io.Reader) (*File, error) { panic("readState returned nil state with no errors") } - if state.TerraformVersion != nil && state.TerraformVersion.GreaterThan(tfversion.SemVer) { - return state, fmt.Errorf( - "state snapshot was created by Terraform v%s, which is newer than current v%s; upgrade to Terraform v%s or greater to work with this state", - state.TerraformVersion, - tfversion.SemVer, - state.TerraformVersion, - ) - } - return state, diags.Err() } diff --git a/states/statemgr/filesystem.go b/states/statemgr/filesystem.go index 541108dde93e..02459f443bec 100644 --- a/states/statemgr/filesystem.go +++ b/states/statemgr/filesystem.go @@ -280,6 +280,10 @@ func (s *Filesystem) refreshState() error { return err } log.Printf("[TRACE] statemgr.Filesystem: snapshot file has nil snapshot, but that's okay") + } else { + if err := f.CheckTerraformVersion(); err != nil { + return err + } } s.file = f @@ -459,6 +463,10 @@ func (s *Filesystem) createStateFiles() error { } log.Printf("[TRACE] statemgr.Filesystem: no previously-stored snapshot exists") } else { + if err := s.backupFile.CheckTerraformVersion(); err != nil { + return err + } + log.Printf("[TRACE] statemgr.Filesystem: existing snapshot has lineage %q serial %d", s.backupFile.Lineage, s.backupFile.Serial) } From 73e1f73b89ba453430687c3133ffce2f81affad6 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 23 Oct 2020 14:50:55 -0400 Subject: [PATCH 2/4] states: RefreshStateWithoutCheckVersion Add RefreshStateWithoutCheckVersion method to the statemgr.Persistent interface, allowing callers to refresh state from the backend without raising errors if the state's Terraform version is thought to be not fully compatible. This enables use cases where we can be extremely confident that any state file we can read is suitable, such as the Terraform provider's remote state data source, which only reads outputs. --- state/lock.go | 4 ++++ state/remote/state.go | 18 +++++++++++++----- states/statemgr/filesystem.go | 15 ++++++++++----- states/statemgr/persistent.go | 10 +++++++++- states/statemgr/statemgr_fake.go | 4 ++++ 5 files changed, 40 insertions(+), 11 deletions(-) diff --git a/state/lock.go b/state/lock.go index 4839df2a71fe..93db4ea559af 100644 --- a/state/lock.go +++ b/state/lock.go @@ -25,6 +25,10 @@ func (s *LockDisabled) RefreshState() error { return s.Inner.RefreshState() } +func (s *LockDisabled) RefreshStateWithoutCheckVersion() error { + return s.Inner.RefreshStateWithoutCheckVersion() +} + func (s *LockDisabled) PersistState() error { return s.Inner.PersistState() } diff --git a/state/remote/state.go b/state/remote/state.go index 652606eab524..3ae182d4bcf6 100644 --- a/state/remote/state.go +++ b/state/remote/state.go @@ -101,13 +101,19 @@ func (s *State) WriteStateForMigration(f *statefile.File, force bool) error { func (s *State) RefreshState() error { s.mu.Lock() defer s.mu.Unlock() - return s.refreshState() + return s.refreshState(true) +} + +func (s *State) RefreshStateWithoutCheckVersion() error { + s.mu.Lock() + defer s.mu.Unlock() + return s.refreshState(false) } // refreshState is the main implementation of RefreshState, but split out so // that we can make internal calls to it from methods that are already holding // the s.mu lock. -func (s *State) refreshState() error { +func (s *State) refreshState(checkVersion bool) error { payload, err := s.Client.Get() if err != nil { return err @@ -125,8 +131,10 @@ func (s *State) refreshState() error { if err != nil { return err } - if err := stateFile.CheckTerraformVersion(); err != nil { - return err + if checkVersion { + if err := stateFile.CheckTerraformVersion(); err != nil { + return err + } } s.lineage = stateFile.Lineage @@ -159,7 +167,7 @@ func (s *State) PersistState() error { // We might be writing a new state altogether, but before we do that // we'll check to make sure there isn't already a snapshot present // that we ought to be updating. - err := s.refreshState() + err := s.refreshState(true) if err != nil { return fmt.Errorf("failed checking for existing remote state: %s", err) } diff --git a/states/statemgr/filesystem.go b/states/statemgr/filesystem.go index 02459f443bec..d6801360ab00 100644 --- a/states/statemgr/filesystem.go +++ b/states/statemgr/filesystem.go @@ -127,7 +127,7 @@ func (s *Filesystem) WriteState(state *states.State) error { defer s.mutex()() if s.readFile == nil { - err := s.refreshState() + err := s.refreshState(true) if err != nil { return err } @@ -230,10 +230,15 @@ func (s *Filesystem) PersistState() error { // RefreshState is an implementation of Refresher. func (s *Filesystem) RefreshState() error { defer s.mutex()() - return s.refreshState() + return s.refreshState(true) } -func (s *Filesystem) refreshState() error { +func (s *Filesystem) RefreshStateWithoutCheckVersion() error { + defer s.mutex()() + return s.refreshState(false) +} + +func (s *Filesystem) refreshState(checkVersion bool) error { var reader io.Reader // The s.readPath file is only OK to read if we have not written any state out @@ -280,7 +285,7 @@ func (s *Filesystem) refreshState() error { return err } log.Printf("[TRACE] statemgr.Filesystem: snapshot file has nil snapshot, but that's okay") - } else { + } else if checkVersion { if err := f.CheckTerraformVersion(); err != nil { return err } @@ -397,7 +402,7 @@ func (s *Filesystem) WriteStateForMigration(f *statefile.File, force bool) error defer s.mutex()() if s.readFile == nil { - err := s.refreshState() + err := s.refreshState(true) if err != nil { return err } diff --git a/states/statemgr/persistent.go b/states/statemgr/persistent.go index c15e84af2dc3..26fc46c5b726 100644 --- a/states/statemgr/persistent.go +++ b/states/statemgr/persistent.go @@ -35,7 +35,9 @@ type Persistent interface { // PersistState that may be happening in other processes. type Refresher interface { // RefreshState retrieves a snapshot of state from persistent storage, - // returning an error if this is not possible. + // returning an error if this is not possible. If a snapshot is retrieved + // but is from an incompatible Terraform version, this will also result + // in an error. // // Types that implement RefreshState generally also implement a State // method that returns the result of the latest successful refresh. @@ -46,6 +48,12 @@ type Refresher interface { // ephemeral portions of the state may be unpopulated after calling // RefreshState. RefreshState() error + + // RefreshStateWithoutCheckVersion is similar to RefreshState, with the + // difference that it does not perform a Terraform version check of the + // state snapshot. Use with caution, as there is no guarantee that the + // state version retrieved is fully compatible. + RefreshStateWithoutCheckVersion() error } // Persister is the interface for managers that can write snapshots to diff --git a/states/statemgr/statemgr_fake.go b/states/statemgr/statemgr_fake.go index 42d2b9bb39e2..dfeb94f26f6b 100644 --- a/states/statemgr/statemgr_fake.go +++ b/states/statemgr/statemgr_fake.go @@ -61,6 +61,10 @@ func (m *fakeFull) RefreshState() error { return m.t.WriteState(m.fakeP.State()) } +func (m *fakeFull) RefreshStateWithoutCheckVersion() error { + return m.t.WriteState(m.fakeP.State()) +} + func (m *fakeFull) PersistState() error { return m.fakeP.WriteState(m.t.State()) } From c11a9d7bd120b9605c0f1b4743a6e3ae98d3f942 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 23 Oct 2020 15:46:42 -0400 Subject: [PATCH 3/4] backend: StateMgrWithoutCheckVersion Allow users of backends to initialize a state manager instance without checking the Terraform version of any state files which are retrieved during this process. Many backends call RefreshState as part of initialization, and this new method instead calls the new RefreshStateWithoutCheckVersion method to prevent version checking. --- backend/atlas/backend.go | 4 ++++ backend/backend.go | 12 ++++++++++ backend/local/backend.go | 4 ++++ backend/nil.go | 4 ++++ backend/remote-state/artifactory/backend.go | 4 ++++ backend/remote-state/azure/backend_state.go | 21 ++++++++++++++--- backend/remote-state/consul/backend_state.go | 21 ++++++++++++++--- backend/remote-state/cos/backend_state.go | 21 ++++++++++++++--- backend/remote-state/etcdv2/backend.go | 4 ++++ backend/remote-state/etcdv3/backend_state.go | 21 ++++++++++++++--- backend/remote-state/gcs/backend_state.go | 18 +++++++++++++-- backend/remote-state/http/backend.go | 4 ++++ backend/remote-state/inmem/backend.go | 4 ++++ backend/remote-state/manta/backend_state.go | 21 ++++++++++++++--- backend/remote-state/oss/backend_state.go | 24 ++++++++++++++++---- backend/remote-state/pg/backend_state.go | 4 ++++ backend/remote-state/s3/backend_state.go | 21 ++++++++++++++--- backend/remote-state/swift/backend_state.go | 21 ++++++++++++++--- backend/remote/backend.go | 4 ++++ 19 files changed, 210 insertions(+), 27 deletions(-) diff --git a/backend/atlas/backend.go b/backend/atlas/backend.go index 9cd15d424c91..929a25196171 100644 --- a/backend/atlas/backend.go +++ b/backend/atlas/backend.go @@ -179,6 +179,10 @@ func (b *Backend) StateMgr(name string) (state.State, error) { return &remote.State{Client: b.stateClient}, nil } +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} + // Colorize returns the Colorize structure that can be used for colorizing // output. This is gauranteed to always return a non-nil value and so is useful // as a helper to wrap any potentially colored strings. diff --git a/backend/backend.go b/backend/backend.go index 268b52f67334..b6ccac1dee2d 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -103,6 +103,18 @@ type Backend interface { // PersistState is called, depending on the state manager implementation. StateMgr(workspace string) (statemgr.Full, error) + // StateMgrWithoutCheckVersion returns the state manager for the given + // workspace name, while ensuring that Terraform version checks are not + // performed if the backend needs to read a state file in order to + // initialize the state manager. + // + // For backends which do not need to read a state file at this point, this + // is identical to StateMgr. + // + // This is used to facilitate reading compatible state files from newer + // versions of Terraform. + StateMgrWithoutCheckVersion(workspace string) (statemgr.Full, error) + // DeleteWorkspace removes the workspace with the given name if it exists. // // DeleteWorkspace cannot prevent deleting a state that is in use. It is diff --git a/backend/local/backend.go b/backend/local/backend.go index e739594996b3..f8b58d71a1fa 100644 --- a/backend/local/backend.go +++ b/backend/local/backend.go @@ -279,6 +279,10 @@ func (b *Local) StateMgr(name string) (statemgr.Full, error) { return s, nil } +func (b *Local) StateMgrWithoutCheckVersion(name string) (statemgr.Full, error) { + return b.StateMgr(name) +} + // Operation implements backend.Enhanced // // This will initialize an in-memory terraform.Context to perform the diff --git a/backend/nil.go b/backend/nil.go index 8c46f49d0076..946d870b22c3 100644 --- a/backend/nil.go +++ b/backend/nil.go @@ -31,6 +31,10 @@ func (Nil) StateMgr(string) (statemgr.Full, error) { return statemgr.NewFullFake(statemgr.NewTransientInMemory(nil), nil), nil } +func (Nil) StateMgrWithoutCheckVersion(string) (statemgr.Full, error) { + return statemgr.NewFullFake(statemgr.NewTransientInMemory(nil), nil), nil +} + func (Nil) DeleteWorkspace(string) error { return nil } diff --git a/backend/remote-state/artifactory/backend.go b/backend/remote-state/artifactory/backend.go index 775584a51b72..3d43a4615136 100644 --- a/backend/remote-state/artifactory/backend.go +++ b/backend/remote-state/artifactory/backend.go @@ -100,3 +100,7 @@ func (b *Backend) StateMgr(name string) (state.State, error) { Client: b.client, }, nil } + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} diff --git a/backend/remote-state/azure/backend_state.go b/backend/remote-state/azure/backend_state.go index cd0054eda17d..a78b9decd7e2 100644 --- a/backend/remote-state/azure/backend_state.go +++ b/backend/remote-state/azure/backend_state.go @@ -77,6 +77,14 @@ func (b *Backend) DeleteWorkspace(name string) error { } func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { ctx := context.TODO() blobClient, err := b.armClient.getBlobClient(ctx) if err != nil { @@ -111,9 +119,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { } // Grab the value - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/consul/backend_state.go b/backend/remote-state/consul/backend_state.go index bdcc9da98bdd..5d5b397bf8fb 100644 --- a/backend/remote-state/consul/backend_state.go +++ b/backend/remote-state/consul/backend_state.go @@ -65,6 +65,14 @@ func (b *Backend) DeleteWorkspace(name string) error { } func (b *Backend) StateMgr(name string) (statemgr.Full, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (statemgr.Full, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (statemgr.Full, error) { // Determine the path of the data path := b.path(name) @@ -110,9 +118,16 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { } // Grab the value - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/cos/backend_state.go b/backend/remote-state/cos/backend_state.go index 2bc3f242807d..2de49b0e07cd 100644 --- a/backend/remote-state/cos/backend_state.go +++ b/backend/remote-state/cos/backend_state.go @@ -75,6 +75,14 @@ func (b *Backend) DeleteWorkspace(name string) error { // StateMgr manage the state, if the named state not exists, a new file will created func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { log.Printf("[DEBUG] state manager, current workspace: %v", name) c, err := b.client(name) @@ -108,9 +116,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { } // Grab the value - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/etcdv2/backend.go b/backend/remote-state/etcdv2/backend.go index aa03056329be..4b990d80d36b 100644 --- a/backend/remote-state/etcdv2/backend.go +++ b/backend/remote-state/etcdv2/backend.go @@ -94,3 +94,7 @@ func (b *Backend) StateMgr(name string) (state.State, error) { }, }, nil } + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} diff --git a/backend/remote-state/etcdv3/backend_state.go b/backend/remote-state/etcdv3/backend_state.go index 44bf0c58816f..3645642837ea 100644 --- a/backend/remote-state/etcdv3/backend_state.go +++ b/backend/remote-state/etcdv3/backend_state.go @@ -42,6 +42,14 @@ func (b *Backend) DeleteWorkspace(name string) error { } func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { var stateMgr state.State = &remote.State{ Client: &RemoteClient{ Client: b.client, @@ -68,9 +76,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { return parent } - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } if v := stateMgr.State(); v == nil { diff --git a/backend/remote-state/gcs/backend_state.go b/backend/remote-state/gcs/backend_state.go index 835ad96a7629..765791c75341 100644 --- a/backend/remote-state/gcs/backend_state.go +++ b/backend/remote-state/gcs/backend_state.go @@ -87,6 +87,14 @@ func (b *Backend) client(name string) (*remoteClient, error) { // StateMgr reads and returns the named state from GCS. If the named state does // not yet exist, a new state file is created. func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { c, err := b.client(name) if err != nil { return nil, err @@ -95,8 +103,14 @@ func (b *Backend) StateMgr(name string) (state.State, error) { st := &remote.State{Client: c} // Grab the value - if err := st.RefreshState(); err != nil { - return nil, err + if checkVersion { + if err := st.RefreshState(); err != nil { + return nil, err + } + } else { + if err := st.RefreshStateWithoutCheckVersion(); err != nil { + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/http/backend.go b/backend/remote-state/http/backend.go index ea5d8772b49a..0fcf35d1e26e 100644 --- a/backend/remote-state/http/backend.go +++ b/backend/remote-state/http/backend.go @@ -183,6 +183,10 @@ func (b *Backend) StateMgr(name string) (state.State, error) { return &remote.State{Client: b.client}, nil } +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} + func (b *Backend) Workspaces() ([]string, error) { return nil, backend.ErrWorkspacesNotSupported } diff --git a/backend/remote-state/inmem/backend.go b/backend/remote-state/inmem/backend.go index c25a80502c62..5e472b08c119 100644 --- a/backend/remote-state/inmem/backend.go +++ b/backend/remote-state/inmem/backend.go @@ -150,6 +150,10 @@ func (b *Backend) StateMgr(name string) (state.State, error) { return s, nil } +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} + type stateMap struct { sync.Mutex m map[string]*remote.State diff --git a/backend/remote-state/manta/backend_state.go b/backend/remote-state/manta/backend_state.go index 1eec6f070959..2a1bf217126d 100644 --- a/backend/remote-state/manta/backend_state.go +++ b/backend/remote-state/manta/backend_state.go @@ -65,6 +65,14 @@ func (b *Backend) DeleteWorkspace(name string) error { } func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { if name == "" { return nil, errors.New("missing state name") } @@ -97,9 +105,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { } // Grab the value - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/oss/backend_state.go b/backend/remote-state/oss/backend_state.go index 28c40ee4afb3..3f34a4e535bf 100644 --- a/backend/remote-state/oss/backend_state.go +++ b/backend/remote-state/oss/backend_state.go @@ -12,9 +12,10 @@ import ( "github.com/hashicorp/terraform/state/remote" "github.com/hashicorp/terraform/states" - "github.com/aliyun/aliyun-tablestore-go-sdk/tablestore" "log" "path" + + "github.com/aliyun/aliyun-tablestore-go-sdk/tablestore" ) const ( @@ -111,6 +112,14 @@ func (b *Backend) DeleteWorkspace(name string) error { } func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { client, err := b.remoteClient(name) if err != nil { return nil, err @@ -151,9 +160,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { } // Grab the value - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/pg/backend_state.go b/backend/remote-state/pg/backend_state.go index a886d7f4c9b3..d075be78d9fb 100644 --- a/backend/remote-state/pg/backend_state.go +++ b/backend/remote-state/pg/backend_state.go @@ -111,3 +111,7 @@ func (b *Backend) StateMgr(name string) (state.State, error) { return stateMgr, nil } + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index 861c284b4c81..6dcf2562a2f9 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -125,6 +125,14 @@ func (b *Backend) remoteClient(name string) (*RemoteClient, error) { } func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { client, err := b.remoteClient(name) if err != nil { return nil, err @@ -173,9 +181,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { // Grab the value // This is to ensure that no one beat us to writing a state between // the `exists` check and taking the lock. - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote-state/swift/backend_state.go b/backend/remote-state/swift/backend_state.go index 13df883858c6..04a89589a4ba 100644 --- a/backend/remote-state/swift/backend_state.go +++ b/backend/remote-state/swift/backend_state.go @@ -92,6 +92,14 @@ func (b *Backend) DeleteWorkspace(name string) error { } func (b *Backend) StateMgr(name string) (state.State, error) { + return b.stateMgr(name, true) +} + +func (b *Backend) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.stateMgr(name, false) +} + +func (b *Backend) stateMgr(name string, checkVersion bool) (state.State, error) { if name == "" { return nil, fmt.Errorf("missing state name") } @@ -161,9 +169,16 @@ func (b *Backend) StateMgr(name string) (state.State, error) { } // Grab the value - if err := stateMgr.RefreshState(); err != nil { - err = lockUnlock(err) - return nil, err + if checkVersion { + if err := stateMgr.RefreshState(); err != nil { + err = lockUnlock(err) + return nil, err + } + } else { + if err := stateMgr.RefreshStateWithoutCheckVersion(); err != nil { + err = lockUnlock(err) + return nil, err + } } // If we have no state, we have to create an empty state diff --git a/backend/remote/backend.go b/backend/remote/backend.go index 35132686f3ac..cc2b35981260 100644 --- a/backend/remote/backend.go +++ b/backend/remote/backend.go @@ -635,6 +635,10 @@ func (b *Remote) StateMgr(name string) (state.State, error) { return &remote.State{Client: client}, nil } +func (b *Remote) StateMgrWithoutCheckVersion(name string) (state.State, error) { + return b.StateMgr(name) +} + // Operation implements backend.Enhanced. func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend.RunningOperation, error) { // Get the remote workspace name. From 08f25fa6cef882e4ef18751bd0d7f4c684565f65 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 23 Oct 2020 15:57:43 -0400 Subject: [PATCH 4/4] builtin: Disable Terraform provider version checks The builtin Terraform provider's remote state data source uses a configured backend to fetch a given state, in order to allow access to its root module outputs. Until this commit, this was only possible with remote states which are from the current Terraform version or older, forcing multi-state users to carefully orchestrate Terraform upgrades. Building on previous commits in this branch, we now disable this version check, and allow any Terraform state file that the current Terraform version can parse. Since we are only ever accessing root module outputs, this is very likely to be safe for the foreseeable future. --- .../providers/terraform/data_source_state.go | 4 ++-- .../terraform/data_source_state_test.go | 23 ++++++++++++++++++- .../terraform/testdata/future.tfstate | 12 ++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 builtin/providers/terraform/testdata/future.tfstate diff --git a/builtin/providers/terraform/data_source_state.go b/builtin/providers/terraform/data_source_state.go index accc356df166..5a71fec94a52 100644 --- a/builtin/providers/terraform/data_source_state.go +++ b/builtin/providers/terraform/data_source_state.go @@ -100,7 +100,7 @@ func dataSourceRemoteStateRead(d cty.Value) (cty.Value, tfdiags.Diagnostics) { newState["workspace"] = cty.StringVal(workspaceName) - state, err := b.StateMgr(workspaceName) + state, err := b.StateMgrWithoutCheckVersion(workspaceName) if err != nil { diags = diags.Append(tfdiags.AttributeValue( tfdiags.Error, @@ -111,7 +111,7 @@ func dataSourceRemoteStateRead(d cty.Value) (cty.Value, tfdiags.Diagnostics) { return cty.NilVal, diags } - if err := state.RefreshState(); err != nil { + if err := state.RefreshStateWithoutCheckVersion(); err != nil { diags = diags.Append(err) return cty.NilVal, diags } diff --git a/builtin/providers/terraform/data_source_state_test.go b/builtin/providers/terraform/data_source_state_test.go index 995bca5dcf30..24f5fb6608b9 100644 --- a/builtin/providers/terraform/data_source_state_test.go +++ b/builtin/providers/terraform/data_source_state_test.go @@ -1,9 +1,10 @@ package terraform import ( - "github.com/hashicorp/terraform/tfdiags" "testing" + "github.com/hashicorp/terraform/tfdiags" + "github.com/apparentlymart/go-dump/dump" "github.com/hashicorp/terraform/backend" "github.com/zclconf/go-cty/cty" @@ -121,6 +122,26 @@ func TestState_basic(t *testing.T) { }), false, }, + "future version": { + cty.ObjectVal(map[string]cty.Value{ + "backend": cty.StringVal("local"), + "config": cty.ObjectVal(map[string]cty.Value{ + "path": cty.StringVal("./testdata/future.tfstate"), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "backend": cty.StringVal("local"), + "config": cty.ObjectVal(map[string]cty.Value{ + "path": cty.StringVal("./testdata/future.tfstate"), + }), + "outputs": cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + }), + "defaults": cty.NullVal(cty.DynamicPseudoType), + "workspace": cty.StringVal(backend.DefaultStateName), + }), + false, + }, "missing": { cty.ObjectVal(map[string]cty.Value{ "backend": cty.StringVal("local"), diff --git a/builtin/providers/terraform/testdata/future.tfstate b/builtin/providers/terraform/testdata/future.tfstate new file mode 100644 index 000000000000..0968eb8fc6d3 --- /dev/null +++ b/builtin/providers/terraform/testdata/future.tfstate @@ -0,0 +1,12 @@ +{ + "version": 4, + "terraform_version": "999.0.0", + "serial": 0, + "lineage": "", + "outputs": { + "foo": { + "value": "bar", + "type": "string" + } + } +}