From 4fab46749ab8fa2234ad8a48471b34bebdd4eece Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Thu, 25 Aug 2022 14:57:40 -0500 Subject: [PATCH 01/18] update persist state --- internal/backend/local/backend.go | 2 +- internal/backend/local/backend_apply.go | 18 +++++----- internal/backend/local/backend_local_test.go | 3 +- internal/backend/local/backend_refresh.go | 12 +++++-- .../remote-state/azure/backend_state.go | 2 +- .../remote-state/consul/backend_state.go | 2 +- .../backend/remote-state/cos/backend_state.go | 2 +- .../backend/remote-state/gcs/backend_state.go | 2 +- .../backend/remote-state/inmem/backend.go | 2 +- .../remote-state/inmem/backend_test.go | 4 +-- .../remote-state/kubernetes/backend_state.go | 2 +- .../remote-state/manta/backend_state.go | 2 +- .../backend/remote-state/oss/backend_state.go | 2 +- .../backend/remote-state/pg/backend_state.go | 2 +- .../backend/remote-state/pg/backend_test.go | 4 +-- .../backend/remote-state/s3/backend_state.go | 2 +- .../backend/remote-state/s3/backend_test.go | 10 +++--- .../remote-state/swift/backend_state.go | 2 +- internal/backend/testing.go | 4 +-- internal/cloud/state.go | 7 ++-- internal/command/helper.go | 34 +++++++++++++++++++ internal/command/import.go | 11 +++++- internal/command/meta_backend.go | 2 +- internal/command/meta_backend_migrate.go | 20 +++++++++-- internal/command/meta_backend_test.go | 26 +++++++------- internal/command/show.go | 16 ++------- internal/command/state_mv.go | 24 +++++++++++-- internal/command/state_push.go | 19 ++++++++++- internal/command/state_push_test.go | 2 +- internal/command/state_replace_provider.go | 21 +++++++++++- internal/command/state_rm.go | 22 +++++++++++- internal/command/taint.go | 22 +++++++++++- internal/command/untaint.go | 22 +++++++++++- internal/command/workspace_new.go | 2 +- internal/states/remote/state.go | 3 +- internal/states/remote/state_test.go | 8 ++--- internal/states/statemgr/filesystem.go | 3 +- internal/states/statemgr/helper.go | 5 +-- internal/states/statemgr/lock.go | 9 +++-- internal/states/statemgr/persistent.go | 7 +++- internal/states/statemgr/statemgr_fake.go | 5 +-- internal/states/statemgr/testing.go | 6 ++-- internal/states/statemgr/transient.go | 2 +- 43 files changed, 280 insertions(+), 97 deletions(-) create mode 100644 internal/command/helper.go diff --git a/internal/backend/local/backend.go b/internal/backend/local/backend.go index dd6c9cc56f10..de1e7e5a422e 100644 --- a/internal/backend/local/backend.go +++ b/internal/backend/local/backend.go @@ -344,7 +344,7 @@ func (b *Local) opWait( // try to force a PersistState just in case the process is terminated // before we can complete. - if err := opStateMgr.PersistState(); err != nil { + if err := opStateMgr.PersistState(nil); err != nil { // We can't error out from here, but warn the user if there was an error. // If this isn't transient, we will catch it again below, and // attempt to save the state another way. diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index a6c6ea9f0b4b..5bec4b442a44 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -53,7 +53,7 @@ func (b *Local) opApply( op.ReportResult(runningOp, diags) return } - // the state was locked during succesfull context creation; unlock the state + // the state was locked during successful context creation; unlock the state // when the operation completes defer func() { diags := op.StateLocker.Unlock() @@ -68,6 +68,13 @@ func (b *Local) opApply( // operation. runningOp.State = lr.InputState + schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState) + diags = diags.Append(moreDiags) + if moreDiags.HasErrors() { + op.ReportResult(runningOp, diags) + return + } + var plan *plans.Plan // If we weren't given a plan, then we refresh/plan if op.PlanFile == nil { @@ -80,13 +87,6 @@ func (b *Local) opApply( return } - schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState) - diags = diags.Append(moreDiags) - if moreDiags.HasErrors() { - op.ReportResult(runningOp, diags) - return - } - trivialPlan := !plan.CanApply() hasUI := op.UIOut != nil && op.UIIn != nil mustConfirm := hasUI && !op.AutoApprove && !trivialPlan @@ -198,7 +198,7 @@ func (b *Local) opApply( // Store the final state runningOp.State = applyState - err := statemgr.WriteAndPersist(opState, applyState) + err := statemgr.WriteAndPersist(opState, applyState, schemas) if err != nil { // Export the state file from the state manager and assign the new // state. This is needed to preserve the existing serial and lineage. diff --git a/internal/backend/local/backend_local_test.go b/internal/backend/local/backend_local_test.go index 05573f0f9d97..136dbaccf275 100644 --- a/internal/backend/local/backend_local_test.go +++ b/internal/backend/local/backend_local_test.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terminal" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -233,6 +234,6 @@ func (s *stateStorageThatFailsRefresh) RefreshState() error { return fmt.Errorf("intentionally failing for testing purposes") } -func (s *stateStorageThatFailsRefresh) PersistState() error { +func (s *stateStorageThatFailsRefresh) PersistState(schemas *terraform.Schemas) error { return fmt.Errorf("unimplemented") } diff --git a/internal/backend/local/backend_refresh.go b/internal/backend/local/backend_refresh.go index 8ce3b6aff1a8..244e8e89bb6c 100644 --- a/internal/backend/local/backend_refresh.go +++ b/internal/backend/local/backend_refresh.go @@ -52,7 +52,7 @@ func (b *Local) opRefresh( return } - // the state was locked during succesfull context creation; unlock the state + // the state was locked during successful context creation; unlock the state // when the operation completes defer func() { diags := op.StateLocker.Unlock() @@ -73,6 +73,14 @@ func (b *Local) opRefresh( )) } + // get schemas before writing state + schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState) + diags = diags.Append(moreDiags) + if moreDiags.HasErrors() { + op.ReportResult(runningOp, diags) + return + } + // Perform the refresh in a goroutine so we can be interrupted var newState *states.State var refreshDiags tfdiags.Diagnostics @@ -96,7 +104,7 @@ func (b *Local) opRefresh( return } - err := statemgr.WriteAndPersist(opState, newState) + err := statemgr.WriteAndPersist(opState, newState, schemas) if err != nil { diags = diags.Append(fmt.Errorf("failed to write state: %w", err)) op.ReportResult(runningOp, diags) diff --git a/internal/backend/remote-state/azure/backend_state.go b/internal/backend/remote-state/azure/backend_state.go index 6a1a9c02f01d..82d2505c65a4 100644 --- a/internal/backend/remote-state/azure/backend_state.go +++ b/internal/backend/remote-state/azure/backend_state.go @@ -131,7 +131,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/consul/backend_state.go b/internal/backend/remote-state/consul/backend_state.go index be1841eb8df3..5bd74b250878 100644 --- a/internal/backend/remote-state/consul/backend_state.go +++ b/internal/backend/remote-state/consul/backend_state.go @@ -120,7 +120,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/cos/backend_state.go b/internal/backend/remote-state/cos/backend_state.go index ab92cfb7c0e8..4e47ae0e6811 100644 --- a/internal/backend/remote-state/cos/backend_state.go +++ b/internal/backend/remote-state/cos/backend_state.go @@ -126,7 +126,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/gcs/backend_state.go b/internal/backend/remote-state/gcs/backend_state.go index ee764efb4c49..21b71834735c 100644 --- a/internal/backend/remote-state/gcs/backend_state.go +++ b/internal/backend/remote-state/gcs/backend_state.go @@ -131,7 +131,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { if err := st.WriteState(states.NewState()); err != nil { return nil, unlock(err) } - if err := st.PersistState(); err != nil { + if err := st.PersistState(nil); err != nil { return nil, unlock(err) } diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index 7f8f56ef2034..4e0113cbc721 100644 --- a/internal/backend/remote-state/inmem/backend.go +++ b/internal/backend/remote-state/inmem/backend.go @@ -141,7 +141,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { if err := s.WriteState(statespkg.NewState()); err != nil { return nil, err } - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { return nil, err } } diff --git a/internal/backend/remote-state/inmem/backend_test.go b/internal/backend/remote-state/inmem/backend_test.go index 395199890a78..204858824d0a 100644 --- a/internal/backend/remote-state/inmem/backend_test.go +++ b/internal/backend/remote-state/inmem/backend_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/terraform/internal/backend" statespkg "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" @@ -82,7 +80,7 @@ func TestRemoteState(t *testing.T) { t.Fatal(err) } - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatal(err) } diff --git a/internal/backend/remote-state/kubernetes/backend_state.go b/internal/backend/remote-state/kubernetes/backend_state.go index 56aa089ff81c..6e8ce449d1c4 100644 --- a/internal/backend/remote-state/kubernetes/backend_state.go +++ b/internal/backend/remote-state/kubernetes/backend_state.go @@ -123,7 +123,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { if err := stateMgr.WriteState(states.NewState()); err != nil { return nil, unlock(err) } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { return nil, unlock(err) } diff --git a/internal/backend/remote-state/manta/backend_state.go b/internal/backend/remote-state/manta/backend_state.go index 925d82083d52..b30b250dc73a 100644 --- a/internal/backend/remote-state/manta/backend_state.go +++ b/internal/backend/remote-state/manta/backend_state.go @@ -108,7 +108,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/oss/backend_state.go b/internal/backend/remote-state/oss/backend_state.go index 77a2775f8ac4..672a2e1aa25a 100644 --- a/internal/backend/remote-state/oss/backend_state.go +++ b/internal/backend/remote-state/oss/backend_state.go @@ -160,7 +160,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/pg/backend_state.go b/internal/backend/remote-state/pg/backend_state.go index 2700c5196927..f3eb650092eb 100644 --- a/internal/backend/remote-state/pg/backend_state.go +++ b/internal/backend/remote-state/pg/backend_state.go @@ -99,7 +99,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/pg/backend_test.go b/internal/backend/remote-state/pg/backend_test.go index da058483d845..064c001f5b3b 100644 --- a/internal/backend/remote-state/pg/backend_test.go +++ b/internal/backend/remote-state/pg/backend_test.go @@ -330,7 +330,7 @@ func TestBackendConcurrentLock(t *testing.T) { t.Fatalf("failed to lock first state: %v", err) } - if err = s1.PersistState(); err != nil { + if err = s1.PersistState(nil); err != nil { t.Fatalf("failed to persist state: %v", err) } @@ -343,7 +343,7 @@ func TestBackendConcurrentLock(t *testing.T) { t.Fatalf("failed to lock second state: %v", err) } - if err = s2.PersistState(); err != nil { + if err = s2.PersistState(nil); err != nil { t.Fatalf("failed to persist state: %v", err) } diff --git a/internal/backend/remote-state/s3/backend_state.go b/internal/backend/remote-state/s3/backend_state.go index 0134c861d015..d13cc32d4f83 100644 --- a/internal/backend/remote-state/s3/backend_state.go +++ b/internal/backend/remote-state/s3/backend_state.go @@ -184,7 +184,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go index a44f154c08c4..1fd49c461ab5 100644 --- a/internal/backend/remote-state/s3/backend_test.go +++ b/internal/backend/remote-state/s3/backend_test.go @@ -478,7 +478,7 @@ func TestBackendExtraPaths(t *testing.T) { // Write the first state stateMgr := &remote.State{Client: client} stateMgr.WriteState(s1) - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { t.Fatal(err) } @@ -488,7 +488,7 @@ func TestBackendExtraPaths(t *testing.T) { client.path = b.path("s2") stateMgr2 := &remote.State{Client: client} stateMgr2.WriteState(s2) - if err := stateMgr2.PersistState(); err != nil { + if err := stateMgr2.PersistState(nil); err != nil { t.Fatal(err) } @@ -501,7 +501,7 @@ func TestBackendExtraPaths(t *testing.T) { // put a state in an env directory name client.path = b.workspaceKeyPrefix + "/error" stateMgr.WriteState(states.NewState()) - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { t.Fatal(err) } if err := checkStateList(b, []string{"default", "s1", "s2"}); err != nil { @@ -511,7 +511,7 @@ func TestBackendExtraPaths(t *testing.T) { // add state with the wrong key for an existing env client.path = b.workspaceKeyPrefix + "/s2/notTestState" stateMgr.WriteState(states.NewState()) - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { t.Fatal(err) } if err := checkStateList(b, []string{"default", "s1", "s2"}); err != nil { @@ -550,7 +550,7 @@ func TestBackendExtraPaths(t *testing.T) { // add a state with a key that matches an existing environment dir name client.path = b.workspaceKeyPrefix + "/s2/" stateMgr.WriteState(states.NewState()) - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { t.Fatal(err) } diff --git a/internal/backend/remote-state/swift/backend_state.go b/internal/backend/remote-state/swift/backend_state.go index b853b64c9638..719585d855f7 100644 --- a/internal/backend/remote-state/swift/backend_state.go +++ b/internal/backend/remote-state/swift/backend_state.go @@ -172,7 +172,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { err = lockUnlock(err) return nil, err } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(nil); err != nil { err = lockUnlock(err) return nil, err } diff --git a/internal/backend/testing.go b/internal/backend/testing.go index 844f95668dd5..a8c04e0ac7ec 100644 --- a/internal/backend/testing.go +++ b/internal/backend/testing.go @@ -132,7 +132,7 @@ func TestBackendStates(t *testing.T, b Backend) { if err := foo.WriteState(fooState); err != nil { t.Fatal("error writing foo state:", err) } - if err := foo.PersistState(); err != nil { + if err := foo.PersistState(nil); err != nil { t.Fatal("error persisting foo state:", err) } @@ -160,7 +160,7 @@ func TestBackendStates(t *testing.T, b Backend) { if err := bar.WriteState(barState); err != nil { t.Fatalf("bad: %s", err) } - if err := bar.PersistState(); err != nil { + if err := bar.PersistState(nil); err != nil { t.Fatalf("bad: %s", err) } diff --git a/internal/cloud/state.go b/internal/cloud/state.go index 73bea8ba5d9c..bc1cc1414e11 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/states/statemgr" + "github.com/hashicorp/terraform/internal/terraform" ) // State is similar to remote State and delegates to it, except in the case of output values, @@ -63,9 +64,9 @@ func (s *State) RefreshState() error { return s.delegate.RefreshState() } -// RefreshState delegates calls to refresh State to the remote State -func (s *State) PersistState() error { - return s.delegate.PersistState() +// PersistState delegates calls to persist State to the remote State +func (s *State) PersistState(schemas *terraform.Schemas) error { + return s.delegate.PersistState(schemas) } // WriteState delegates calls to write State to the remote State diff --git a/internal/command/helper.go b/internal/command/helper.go new file mode 100644 index 000000000000..d8e7515df5c7 --- /dev/null +++ b/internal/command/helper.go @@ -0,0 +1,34 @@ +package command + +import ( + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +func getSchemas(c *Meta, state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + if config != nil || state != nil { + opts, err := c.contextOpts() + if err != nil { + diags = diags.Append(err) + return nil, diags + } + tfCtx, ctxDiags := terraform.NewContext(opts) + diags = diags.Append(ctxDiags) + if ctxDiags.HasErrors() { + return nil, diags + } + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags := tfCtx.Schemas(config, state) + diags = diags.Append(schemaDiags) + if schemaDiags.HasErrors() { + return nil, diags + } + return schemas, diags + + } + return nil, diags +} diff --git a/internal/command/import.go b/internal/command/import.go index 51d3895e73e9..e25f8e3af1e4 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -248,13 +248,22 @@ func (c *ImportCommand) Run(args []string) int { return 1 } + // Get schemas, if possible, before writing state + schemas, diags := getSchemas(&c.Meta, newState, config) + if diags.HasErrors() { + // MBANG TODO - add warning that the schema could not be initialized + // and therefore the JSON state can not be created and may affect + // external applications relying on that data format + return 1 + } + // Persist the final state log.Printf("[INFO] Writing state output to: %s", c.Meta.StateOutPath()) if err := state.WriteState(newState); err != nil { c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err)) return 1 } - if err := state.PersistState(); err != nil { + if err := state.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err)) return 1 } diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index 43e49152cf71..d7fa62e00121 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -998,7 +998,7 @@ func (m *Meta) backend_C_r_s(c *configs.Backend, cHash int, sMgr *clistate.Local diags = diags.Append(fmt.Errorf(errBackendMigrateLocalDelete, err)) return nil, diags } - if err := localState.PersistState(); err != nil { + if err := localState.PersistState(nil); err != nil { diags = diags.Append(fmt.Errorf(errBackendMigrateLocalDelete, err)) return nil, diags } diff --git a/internal/command/meta_backend_migrate.go b/internal/command/meta_backend_migrate.go index 8cd011eff077..e0a81ba39008 100644 --- a/internal/command/meta_backend_migrate.go +++ b/internal/command/meta_backend_migrate.go @@ -434,11 +434,27 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { // includes preserving any lineage/serial information where possible, if // both managers support such metadata. log.Print("[TRACE] backendMigrateState: migration confirmed, so migrating") + + path, err := os.Getwd() + if err != nil { + return fmt.Errorf("could not get working directory") + } + + config, diags := m.loadConfig(path) + if diags.HasErrors() { + return diags.Err() + } + + schemas, diags := getSchemas(m, destination, config) + if diags.HasErrors() { + return diags.Err() + } + if err := statemgr.Migrate(destinationState, sourceState); err != nil { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) } - if err := destinationState.PersistState(); err != nil { + if err := destinationState.PersistState(schemas); err != nil { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) } @@ -960,7 +976,7 @@ This will attempt to copy (with permission) all workspaces again. ` const errBackendStateCopy = ` -Error copying state from the previous %q backend to the newly configured +Error copying state from the previous %q backend to the newly configured %q backend: %s diff --git a/internal/command/meta_backend_test.go b/internal/command/meta_backend_test.go index 111e136cee5b..2fa61b618f7f 100644 --- a/internal/command/meta_backend_test.go +++ b/internal/command/meta_backend_test.go @@ -45,7 +45,7 @@ func TestMetaBackend_emptyDir(t *testing.T) { t.Fatalf("unexpected error: %s", err) } s.WriteState(testState()) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -134,7 +134,7 @@ func TestMetaBackend_emptyWithDefaultState(t *testing.T) { next := testState() next.RootModule().SetOutputValue("foo", cty.StringVal("bar"), false) s.WriteState(next) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -205,7 +205,7 @@ func TestMetaBackend_emptyWithExplicitState(t *testing.T) { next := testState() markStateForMatching(next, "bar") // just any change so it shows as different than before s.WriteState(next) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -265,7 +265,7 @@ func TestMetaBackend_configureNew(t *testing.T) { mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -339,7 +339,7 @@ func TestMetaBackend_configureNewWithState(t *testing.T) { state = states.NewState() mark := markStateForMatching(state, "changing") - if err := statemgr.WriteAndPersist(s, state); err != nil { + if err := statemgr.WriteAndPersist(s, state, nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -505,7 +505,7 @@ func TestMetaBackend_configureNewWithStateExisting(t *testing.T) { mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -576,7 +576,7 @@ func TestMetaBackend_configureNewWithStateExistingNoMigrate(t *testing.T) { state = states.NewState() mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -695,7 +695,7 @@ func TestMetaBackend_configuredChange(t *testing.T) { mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -1448,7 +1448,7 @@ func TestMetaBackend_configuredUnset(t *testing.T) { // Write some state s.WriteState(testState()) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -1506,7 +1506,7 @@ func TestMetaBackend_configuredUnsetCopy(t *testing.T) { // Write some state s.WriteState(testState()) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -1585,7 +1585,7 @@ func TestMetaBackend_planLocal(t *testing.T) { mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -1686,7 +1686,7 @@ func TestMetaBackend_planLocalStatePath(t *testing.T) { mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } @@ -1773,7 +1773,7 @@ func TestMetaBackend_planLocalMatch(t *testing.T) { mark := markStateForMatching(state, "changing") s.WriteState(state) - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("unexpected error: %s", err) } diff --git a/internal/command/show.go b/internal/command/show.go index 123b86536eab..c3486cbac70c 100644 --- a/internal/command/show.go +++ b/internal/command/show.go @@ -110,20 +110,8 @@ func (c *ShowCommand) show(path string) (*plans.Plan, *statefile.File, *configs. // Get schemas, if possible if config != nil || stateFile != nil { - opts, err := c.contextOpts() - if err != nil { - diags = diags.Append(err) - return plan, stateFile, config, schemas, diags - } - tfCtx, ctxDiags := terraform.NewContext(opts) - diags = diags.Append(ctxDiags) - if ctxDiags.HasErrors() { - return plan, stateFile, config, schemas, diags - } - var schemaDiags tfdiags.Diagnostics - schemas, schemaDiags = tfCtx.Schemas(config, stateFile.State) - diags = diags.Append(schemaDiags) - if schemaDiags.HasErrors() { + schemas, diags = getSchemas(&c.Meta, stateFile.State, config) + if diags.HasErrors() { return plan, stateFile, config, schemas, diags } } diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 949f6c4b459d..a0ceb4368a8a 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -385,12 +386,31 @@ func (c *StateMvCommand) Run(args []string) int { return 0 // This is as far as we go in dry-run mode } + path, err := os.Getwd() + if err != nil { + // MBANG TODO - add warning here too? + return 1 + } + + config, diags := c.loadConfig(path) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) + return 1 + } + + schemas, diags := getSchemas(&c.Meta, stateTo, config) + if diags.HasErrors() { + // MBANG TODO - is this the warning? + c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) + return 1 + } + // Write the new state if err := stateToMgr.WriteState(stateTo); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } - if err := stateToMgr.PersistState(); err != nil { + if err := stateToMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } @@ -401,7 +421,7 @@ func (c *StateMvCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } - if err := stateFromMgr.PersistState(); err != nil { + if err := stateFromMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 0b863740c59f..3654e24c7613 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -126,11 +126,28 @@ func (c *StatePushCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Failed to write state: %s", err)) return 1 } + + // Get schemas, if possible, before writing state + path, err := os.Getwd() + if err != nil { + return 1 + } + config, diags := c.loadConfig(path) + if diags.HasErrors() { + // MBANG TODO - add warnings here? + return 1 + } + schemas, diags := getSchemas(&c.Meta, srcStateFile.State, config) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf("Failed to load schemas: %s", err)) + return 1 + } + if err := stateMgr.WriteState(srcStateFile.State); err != nil { c.Ui.Error(fmt.Sprintf("Failed to write state: %s", err)) return 1 } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf("Failed to persist state: %s", err)) return 1 } diff --git a/internal/command/state_push_test.go b/internal/command/state_push_test.go index e30010bb9eac..f79efa9b3b5e 100644 --- a/internal/command/state_push_test.go +++ b/internal/command/state_push_test.go @@ -267,7 +267,7 @@ func TestStatePush_forceRemoteState(t *testing.T) { if err := sMgr.WriteState(states.NewState()); err != nil { t.Fatal(err) } - if err := sMgr.PersistState(); err != nil { + if err := sMgr.PersistState(nil); err != nil { t.Fatal(err) } diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index ec5347a7697a..015a214335e1 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -160,12 +161,30 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { resource.ProviderConfig.Provider = to } + // Get schemas, if possible, before writing state + path, err := os.Getwd() + if err != nil { + return 1 + } + + config, diags := c.loadConfig(path) + if diags.HasErrors() { + // MBANG TODO - add warnings here? + return 1 + } + + schemas, diags := getSchemas(&c.Meta, state, config) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf("Failed to load schemas: %s", err)) + return 1 + } + // Write the updated state if err := stateMgr.WriteState(state); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index f126c5f5a561..a8a93d98c8fb 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -110,11 +111,30 @@ func (c *StateRmCommand) Run(args []string) int { return 0 // This is as far as we go in dry-run mode } + // Get schemas, if possible, before writing state + path, err := os.Getwd() + if err != nil { + // MBANG TODO - add warnings here? + return 1 + } + + config, diags := c.loadConfig(path) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) + return 1 + } + + schemas, diags := getSchemas(&c.Meta, state, config) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) + return 1 + } + if err := stateMgr.WriteState(state); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 } diff --git a/internal/command/taint.go b/internal/command/taint.go index 0c5a499f2e32..b3ced4219c20 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -125,6 +126,25 @@ func (c *TaintCommand) Run(args []string) int { return 1 } + // Get schemas, if possible, before writing state + path, err := os.Getwd() + if err != nil { + // MBANG TODO - add warnings here? + return 1 + } + + config, diags := c.loadConfig(path) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) + return 1 + } + + schemas, diags := getSchemas(&c.Meta, state, config) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) + return 1 + } + ss := state.SyncWrapper() // Get the resource and instance we're going to taint @@ -171,7 +191,7 @@ func (c *TaintCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err)) return 1 } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err)) return 1 } diff --git a/internal/command/untaint.go b/internal/command/untaint.go index ba290a8a47a9..c7c5174b02f5 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -163,6 +164,25 @@ func (c *UntaintCommand) Run(args []string) int { c.showDiagnostics(diags) return 1 } + + // Get schemas, if possible, before writing state + path, err := os.Getwd() + if err != nil { + // MBANG TODO - add warnings here? + return 1 + } + + config, diags := c.loadConfig(path) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) + return 1 + } + + schemas, diags := getSchemas(&c.Meta, state, config) + if diags.HasErrors() { + c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) + return 1 + } obj.Status = states.ObjectReady ss.SetResourceInstanceCurrent(addr, obj, rs.ProviderConfig) @@ -170,7 +190,7 @@ func (c *UntaintCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err)) return 1 } - if err := stateMgr.PersistState(); err != nil { + if err := stateMgr.PersistState(schemas); err != nil { c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err)) return 1 } diff --git a/internal/command/workspace_new.go b/internal/command/workspace_new.go index cd28e6986744..38985622079f 100644 --- a/internal/command/workspace_new.go +++ b/internal/command/workspace_new.go @@ -156,7 +156,7 @@ func (c *WorkspaceNewCommand) Run(args []string) int { c.Ui.Error(err.Error()) return 1 } - err = stateMgr.PersistState() + err = stateMgr.PersistState(nil) if err != nil { c.Ui.Error(err.Error()) return 1 diff --git a/internal/states/remote/state.go b/internal/states/remote/state.go index ae123f8e3647..412adc3eb4f4 100644 --- a/internal/states/remote/state.go +++ b/internal/states/remote/state.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" + "github.com/hashicorp/terraform/internal/terraform" ) // State implements the State interfaces in the state package to handle @@ -153,7 +154,7 @@ func (s *State) refreshState() error { } // statemgr.Persister impl. -func (s *State) PersistState() error { +func (s *State) PersistState(schemas *terraform.Schemas) error { s.mu.Lock() defer s.mu.Unlock() diff --git a/internal/states/remote/state_test.go b/internal/states/remote/state_test.go index 1089ba1aaeed..721a7a0af38c 100644 --- a/internal/states/remote/state_test.go +++ b/internal/states/remote/state_test.go @@ -37,7 +37,7 @@ func TestStateRace(t *testing.T) { go func() { defer wg.Done() s.WriteState(current) - s.PersistState() + s.PersistState(nil) s.RefreshState() }() } @@ -252,7 +252,7 @@ func TestStatePersist(t *testing.T) { if err := mgr.WriteState(s); err != nil { t.Fatalf("failed to WriteState for %q: %s", tc.name, err) } - if err := mgr.PersistState(); err != nil { + if err := mgr.PersistState(nil); err != nil { t.Fatalf("failed to PersistState for %q: %s", tc.name, err) } @@ -447,7 +447,7 @@ func TestWriteStateForMigration(t *testing.T) { // At this point we should just do a normal write and persist // as would happen from the CLI mgr.WriteState(mgr.State()) - mgr.PersistState() + mgr.PersistState(nil) if logIdx >= len(mockClient.log) { t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log)) @@ -611,7 +611,7 @@ func TestWriteStateForMigrationWithForcePushClient(t *testing.T) { // At this point we should just do a normal write and persist // as would happen from the CLI mgr.WriteState(mgr.State()) - mgr.PersistState() + mgr.PersistState(nil) if logIdx >= len(mockClient.log) { t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log)) diff --git a/internal/states/statemgr/filesystem.go b/internal/states/statemgr/filesystem.go index 1406f046563b..bdfc6832b5dd 100644 --- a/internal/states/statemgr/filesystem.go +++ b/internal/states/statemgr/filesystem.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statefile" + "github.com/hashicorp/terraform/internal/terraform" ) // Filesystem is a full state manager that uses a file in the local filesystem @@ -223,7 +224,7 @@ func (s *Filesystem) writeState(state *states.State, meta *SnapshotMeta) error { // PersistState is an implementation of Persister that does nothing because // this type's Writer implementation does its own persistence. -func (s *Filesystem) PersistState() error { +func (s *Filesystem) PersistState(schemas *terraform.Schemas) error { return nil } diff --git a/internal/states/statemgr/helper.go b/internal/states/statemgr/helper.go index a019b2c431a6..6cae85702ea9 100644 --- a/internal/states/statemgr/helper.go +++ b/internal/states/statemgr/helper.go @@ -6,6 +6,7 @@ package statemgr import ( "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statefile" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/version" ) @@ -44,10 +45,10 @@ func RefreshAndRead(mgr Storage) (*states.State, error) { // out quickly with a user-facing error. In situations where more control // is required, call WriteState and PersistState on the state manager directly // and handle their errors. -func WriteAndPersist(mgr Storage, state *states.State) error { +func WriteAndPersist(mgr Storage, state *states.State, schemas *terraform.Schemas) error { err := mgr.WriteState(state) if err != nil { return err } - return mgr.PersistState() + return mgr.PersistState(schemas) } diff --git a/internal/states/statemgr/lock.go b/internal/states/statemgr/lock.go index 79c149fe736e..863dc2f0dd18 100644 --- a/internal/states/statemgr/lock.go +++ b/internal/states/statemgr/lock.go @@ -1,6 +1,9 @@ package statemgr -import "github.com/hashicorp/terraform/internal/states" +import ( + "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" +) // LockDisabled implements State and Locker but disables state locking. // If State doesn't support locking, this is a no-op. This is useful for @@ -27,8 +30,8 @@ func (s *LockDisabled) RefreshState() error { return s.Inner.RefreshState() } -func (s *LockDisabled) PersistState() error { - return s.Inner.PersistState() +func (s *LockDisabled) PersistState(schemas *terraform.Schemas) error { + return s.Inner.PersistState(schemas) } func (s *LockDisabled) Lock(info *LockInfo) (string, error) { diff --git a/internal/states/statemgr/persistent.go b/internal/states/statemgr/persistent.go index fde4a7f0f8ac..70d709f85f4e 100644 --- a/internal/states/statemgr/persistent.go +++ b/internal/states/statemgr/persistent.go @@ -4,6 +4,7 @@ import ( version "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" ) // Persistent is a union of the Refresher and Persistent interfaces, for types @@ -72,8 +73,12 @@ type Refresher interface { // is most commonly achieved by making use of atomic write capabilities on // the remote storage backend in conjunction with book-keeping with the // Serial and Lineage fields in the standard state file formats. +// +// Some implementations may optionally utilize config schema to persist +// state. For example, when representing state in an external JSON +// representation. type Persister interface { - PersistState() error + PersistState(*terraform.Schemas) error } // PersistentMeta is an optional extension to Persistent that allows inspecting diff --git a/internal/states/statemgr/statemgr_fake.go b/internal/states/statemgr/statemgr_fake.go index 8d88e4d24e7e..985e6c677517 100644 --- a/internal/states/statemgr/statemgr_fake.go +++ b/internal/states/statemgr/statemgr_fake.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" ) // NewFullFake returns a full state manager that really only supports transient @@ -61,7 +62,7 @@ func (m *fakeFull) RefreshState() error { return m.t.WriteState(m.fakeP.State()) } -func (m *fakeFull) PersistState() error { +func (m *fakeFull) PersistState(schemas *terraform.Schemas) error { return m.fakeP.WriteState(m.t.State()) } @@ -127,7 +128,7 @@ func (m *fakeErrorFull) RefreshState() error { return errors.New("fake state manager error") } -func (m *fakeErrorFull) PersistState() error { +func (m *fakeErrorFull) PersistState(schemas *terraform.Schemas) error { return errors.New("fake state manager error") } diff --git a/internal/states/statemgr/testing.go b/internal/states/statemgr/testing.go index 171b21ad2ea0..eabf46dc0e40 100644 --- a/internal/states/statemgr/testing.go +++ b/internal/states/statemgr/testing.go @@ -56,7 +56,7 @@ func TestFull(t *testing.T, s Full) { } // Test persistence - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("err: %s", err) } @@ -81,7 +81,7 @@ func TestFull(t *testing.T, s Full) { if err := s.WriteState(current); err != nil { t.Fatalf("err: %s", err) } - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("err: %s", err) } @@ -104,7 +104,7 @@ func TestFull(t *testing.T, s Full) { if err := s.WriteState(current); err != nil { t.Fatalf("err: %s", err) } - if err := s.PersistState(); err != nil { + if err := s.PersistState(nil); err != nil { t.Fatalf("err: %s", err) } diff --git a/internal/states/statemgr/transient.go b/internal/states/statemgr/transient.go index 0ac9b1dedaaa..e47683e98bb3 100644 --- a/internal/states/statemgr/transient.go +++ b/internal/states/statemgr/transient.go @@ -57,7 +57,7 @@ type Reader interface { // since the caller may continue to modify the given state object after // WriteState returns. type Writer interface { - // Write state saves a transient snapshot of the given state. + // WriteState saves a transient snapshot of the given state. // // The caller must ensure that the given state object is not concurrently // modified while a WriteState call is in progress. WriteState itself From b8f2f81cd612f5469531064b51205a9e2d335f5a Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 26 Aug 2022 14:17:37 -0500 Subject: [PATCH 02/18] update to warn if schemas aren't available --- go.mod | 4 +- go.sum | 8 ++-- internal/command/command_test.go | 3 ++ internal/command/helper.go | 10 +++++ internal/command/import.go | 5 +-- internal/command/state_mv.go | 9 ++-- internal/command/state_push.go | 10 ++--- internal/command/state_replace_provider.go | 10 ++--- .../command/state_replace_provider_test.go | 42 +++++++++++++++++++ internal/command/state_rm.go | 9 ++-- internal/command/taint.go | 9 ++-- internal/command/taint_test.go | 11 +++-- internal/command/untaint.go | 9 ++-- 13 files changed, 91 insertions(+), 48 deletions(-) diff --git a/go.mod b/go.mod index 3407e47df1d7..d47eb0612cb3 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ require ( github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-plugin v1.4.3 github.com/hashicorp/go-retryablehttp v0.7.1 - github.com/hashicorp/go-tfe v1.7.0 + github.com/hashicorp/go-tfe v1.8.1-0.20220826155809-b28335218b42 github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f @@ -144,7 +144,7 @@ require ( github.com/hashicorp/go-msgpack v0.5.4 // indirect github.com/hashicorp/go-rootcerts v1.0.2 // indirect github.com/hashicorp/go-safetemp v1.0.0 // indirect - github.com/hashicorp/go-slug v0.9.1 // indirect + github.com/hashicorp/go-slug v0.10.0 // indirect github.com/hashicorp/golang-lru v0.5.1 // indirect github.com/hashicorp/jsonapi v0.0.0-20210826224640-ee7dae0fb22d // indirect github.com/hashicorp/serf v0.9.5 // indirect diff --git a/go.sum b/go.sum index 204fd15ee384..073db9ca669b 100644 --- a/go.sum +++ b/go.sum @@ -370,13 +370,13 @@ github.com/hashicorp/go-rootcerts v1.0.2 h1:jzhAVGtqPKbwpyCPELlgNWhE1znq+qwJtW5O github.com/hashicorp/go-rootcerts v1.0.2/go.mod h1:pqUvnprVnM5bf7AOirdbb01K4ccR319Vf4pU3K5EGc8= github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo= github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I= -github.com/hashicorp/go-slug v0.9.1 h1:gYNVJ3t0jAWx8AT2eYZci3Xd7NBHyjayW9AR1DU4ki0= -github.com/hashicorp/go-slug v0.9.1/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu41RKLH301v4= +github.com/hashicorp/go-slug v0.10.0 h1:mh4DDkBJTh9BuEjY/cv8PTo7k9OjT4PcW8PgZnJ4jTY= +github.com/hashicorp/go-slug v0.10.0/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu41RKLH301v4= github.com/hashicorp/go-sockaddr v1.0.0 h1:GeH6tui99pF4NJgfnhp+L6+FfobzVW3Ah46sLo0ICXs= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4= -github.com/hashicorp/go-tfe v1.7.0 h1:GELRhS5dizF6giwjZBqUC/xPaSuNYB+hWRtUnf6i8K8= -github.com/hashicorp/go-tfe v1.7.0/go.mod h1:E8a90lC4kjU5Lc2c0D+SnWhUuyuoCIVm4Ewzv3jCD3A= +github.com/hashicorp/go-tfe v1.8.1-0.20220826155809-b28335218b42 h1:3Ejb02U7WAl9NBRYArj8Hyw0dvHEB5NO1AsX06wbDq4= +github.com/hashicorp/go-tfe v1.8.1-0.20220826155809-b28335218b42/go.mod h1:uSWi2sPw7tLrqNIiASid9j3SprbbkPSJ/2s3X0mMemg= github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 09b2bb793016..43c4640822eb 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -137,6 +137,9 @@ func metaOverridesForProvider(p providers.Interface) *testingOverrides { Providers: map[addrs.Provider]providers.Factory{ addrs.NewDefaultProvider("test"): providers.FactoryFixed(p), addrs.NewProvider(addrs.DefaultProviderRegistryHost, "hashicorp2", "test"): providers.FactoryFixed(p), + addrs.NewLegacyProvider("null"): providers.FactoryFixed(p), + addrs.NewLegacyProvider("azurerm"): providers.FactoryFixed(p), + addrs.NewProvider(addrs.DefaultProviderRegistryHost, "acmecorp", "aws"): providers.FactoryFixed(p), }, } } diff --git a/internal/command/helper.go b/internal/command/helper.go index d8e7515df5c7..ef13934cc0fd 100644 --- a/internal/command/helper.go +++ b/internal/command/helper.go @@ -7,6 +7,16 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) +const failedToLoadSchemasMessage = ` +Terraform failed to load schemas, which will in turn affect its ability to generate the +external JSON state file. This will not have any adverse effects on Terraforms ability +to maintain state information, but may have adverse effects on any external integrations +relying on this format. The file should be created on the next successful "terraform apply" +however, historic state information may be missing if the affected integration relies on that + +%s +` + func getSchemas(c *Meta, state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics diff --git a/internal/command/import.go b/internal/command/import.go index e25f8e3af1e4..8d98fb74c877 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -251,10 +251,7 @@ func (c *ImportCommand) Run(args []string) int { // Get schemas, if possible, before writing state schemas, diags := getSchemas(&c.Meta, newState, config) if diags.HasErrors() { - // MBANG TODO - add warning that the schema could not be initialized - // and therefore the JSON state can not be created and may affect - // external applications relying on that data format - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } // Persist the final state diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index a0ceb4368a8a..231f9649962a 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -388,20 +388,17 @@ func (c *StateMvCommand) Run(args []string) int { path, err := os.Getwd() if err != nil { - // MBANG TODO - add warning here too? - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, stateTo, config) if diags.HasErrors() { - // MBANG TODO - is this the warning? - c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) return 1 } diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 3654e24c7613..fd3f55a248cf 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -130,17 +130,17 @@ func (c *StatePushCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() if err != nil { - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } + config, diags := c.loadConfig(path) if diags.HasErrors() { - // MBANG TODO - add warnings here? - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } + schemas, diags := getSchemas(&c.Meta, srcStateFile.State, config) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf("Failed to load schemas: %s", err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } if err := stateMgr.WriteState(srcStateFile.State); err != nil { diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 015a214335e1..4b08439e23e2 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -164,19 +164,17 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() if err != nil { - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) if diags.HasErrors() { - // MBANG TODO - add warnings here? - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } - + schemas, diags := getSchemas(&c.Meta, state, config) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf("Failed to load schemas: %s", err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } // Write the updated state diff --git a/internal/command/state_replace_provider_test.go b/internal/command/state_replace_provider_test.go index 9c86cf7797d1..e86e5d669f33 100644 --- a/internal/command/state_replace_provider_test.go +++ b/internal/command/state_replace_provider_test.go @@ -2,7 +2,9 @@ package command import ( "bytes" + "fmt" "path/filepath" + "regexp" "strings" "testing" @@ -61,6 +63,46 @@ func TestStateReplaceProvider(t *testing.T) { ) }) + t.Run("Schemas not initialized and JSON output not generated", func(t *testing.T) { + statePath := testStateFile(t, state) + + ui := new(cli.MockUi) + view, _ := testView(t) + c := &StateReplaceProviderCommand{ + StateMeta{ + Meta: Meta{ + Ui: ui, + View: view, + }, + }, + } + + inputBuf := &bytes.Buffer{} + ui.InputReader = inputBuf + inputBuf.WriteString("yes\n") + + args := []string{ + "-state", statePath, + "hashicorp/aws", + "acmecorp/aws", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + // Check for the warning + actual := strings.TrimSpace(ui.ErrorWriter.String()) + expected := strings.TrimSpace(fmt.Sprintf(failedToLoadSchemasMessage, "")) + re, err := regexp.Compile(expected) + if err != nil { + t.Fatalf("Error compiling regexp: %s", err) + } + + if !re.MatchString(actual) { + t.Fatalf("wrong output\n expected: %s \n actual: %s", expected, actual) + } + }) + t.Run("happy path", func(t *testing.T) { statePath := testStateFile(t, state) diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index a8a93d98c8fb..1273d2cca879 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -114,20 +114,17 @@ func (c *StateRmCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() if err != nil { - // MBANG TODO - add warnings here? - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } if err := stateMgr.WriteState(state); err != nil { diff --git a/internal/command/taint.go b/internal/command/taint.go index b3ced4219c20..754636078212 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -129,20 +129,17 @@ func (c *TaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() if err != nil { - // MBANG TODO - add warnings here? - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } ss := state.SyncWrapper() diff --git a/internal/command/taint_test.go b/internal/command/taint_test.go index 001d477082bc..b62be299ffa7 100644 --- a/internal/command/taint_test.go +++ b/internal/command/taint_test.go @@ -2,10 +2,10 @@ package command import ( "os" + "regexp" "strings" "testing" - "github.com/google/go-cmp/cmp" "github.com/mitchellh/cli" "github.com/hashicorp/terraform/internal/addrs" @@ -381,8 +381,13 @@ Resource instance test_instance.bar was not found, but this is not an error because -allow-missing was set. `) - if diff := cmp.Diff(expected, actual); diff != "" { - t.Fatalf("wrong output\n%s", diff) + re, err := regexp.Compile(expected) + if err != nil { + t.Fatalf("Error compiling regexp: %s", err) + } + + if !re.MatchString(actual) { + t.Fatalf("wrong output\n expected: %s \n actual: %s", expected, actual) } } diff --git a/internal/command/untaint.go b/internal/command/untaint.go index c7c5174b02f5..a25d5de470d7 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -168,20 +168,17 @@ func (c *UntaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() if err != nil { - // MBANG TODO - add warnings here? - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) if diags.HasErrors() { - c.Ui.Error(fmt.Sprintf("Failed to load config: %s", err)) - return 1 + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } obj.Status = states.ObjectReady ss.SetResourceInstanceCurrent(addr, obj, rs.ProviderConfig) From 021f1f69e949188d53f1b62469688e0009d37974 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 26 Aug 2022 14:18:34 -0500 Subject: [PATCH 03/18] updates to cloud state --- internal/cloud/backend.go | 24 +- internal/cloud/backend_state_test.go | 117 ++++---- internal/cloud/state.go | 415 ++++++++++++++++++++++++--- internal/cloud/state_test.go | 11 +- internal/cloud/testing.go | 101 ++++++- 5 files changed, 538 insertions(+), 130 deletions(-) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 669cc372270d..ded38f75f29f 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -526,15 +526,10 @@ func (b *Cloud) DeleteWorkspace(name string) error { } // Configure the remote workspace name. - client := &remoteClient{ - client: b.client, - organization: b.organization, - workspace: &tfe.Workspace{ - Name: name, - }, - } - - return client.Delete() + State := &State{tfeClient: b.client, organization: b.organization, workspace: &tfe.Workspace{ + Name: name, + }} + return State.Delete() } // StateMgr implements backend.Enhanced. @@ -619,16 +614,7 @@ func (b *Cloud) StateMgr(name string) (statemgr.Full, error) { } } - client := &remoteClient{ - client: b.client, - organization: b.organization, - workspace: workspace, - - // This is optionally set during Terraform Enterprise runs. - runID: os.Getenv("TFE_RUN_ID"), - } - - return NewState(client), nil + return &State{tfeClient: b.client, organization: b.organization, workspace: workspace}, nil } // Operation implements backend.Enhanced. diff --git a/internal/cloud/backend_state_test.go b/internal/cloud/backend_state_test.go index 3b9833c38e5e..223ddb9153fa 100644 --- a/internal/cloud/backend_state_test.go +++ b/internal/cloud/backend_state_test.go @@ -2,69 +2,68 @@ package cloud import ( "bytes" + "io/ioutil" "os" "testing" - tfe "github.com/hashicorp/go-tfe" - "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/states/statefile" ) -func TestRemoteClient_impl(t *testing.T) { +func TestCloudState_impl(t *testing.T) { var _ remote.Client = new(remoteClient) } -func TestRemoteClient(t *testing.T) { - client := testRemoteClient(t) - remote.TestClient(t, client) +func TestCloudState(t *testing.T) { + state := testCloudState(t) + TestState(t, state) } -func TestRemoteClient_stateVersionCreated(t *testing.T) { - b, bCleanup := testBackendWithName(t) - defer bCleanup() - - raw, err := b.StateMgr(testBackendSingleWorkspaceName) - if err != nil { - t.Fatalf("error: %v", err) - } - - client := raw.(*State).Client - - err = client.Put(([]byte)(` -{ - "version": 4, - "terraform_version": "1.3.0", - "serial": 1, - "lineage": "backend-change", - "outputs": { - "foo": { - "type": "string", - "value": "bar" - } - } -}`)) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - stateVersionsAPI := b.client.StateVersions.(*MockStateVersions) - if got, want := len(stateVersionsAPI.stateVersions), 1; got != want { - t.Fatalf("wrong number of state versions in the mock client %d; want %d", got, want) - } - - var stateVersion *tfe.StateVersion - for _, sv := range stateVersionsAPI.stateVersions { - stateVersion = sv - } - - if stateVersionsAPI.outputStates[stateVersion.ID] == nil || len(stateVersionsAPI.outputStates[stateVersion.ID]) == 0 { - t.Fatal("no state version outputs in the mock client") - } -} - -func TestRemoteClient_TestRemoteLocks(t *testing.T) { +//func TestRemoteClient_stateVersionCreated(t *testing.T) { +// b, bCleanup := testBackendWithName(t) +// defer bCleanup() +// +// raw, err := b.StateMgr(testBackendSingleWorkspaceName) +// if err != nil { +// t.Fatalf("error: %v", err) +// } +// +// state := raw.(*State) +// +// err = state.WriteState(([]byte)(` +//{ +// "version": 4, +// "terraform_version": "1.3.0", +// "serial": 1, +// "lineage": "backend-change", +// "outputs": { +// "foo": { +// "type": "string", +// "value": "bar" +// } +// } +//}`)) +// if err != nil { +// t.Fatalf("expected no error, got %v", err) +// } +// +// stateVersionsAPI := b.client.StateVersions.(*MockStateVersions) +// if got, want := len(stateVersionsAPI.stateVersions), 1; got != want { +// t.Fatalf("wrong number of state versions in the mock client %d; want %d", got, want) +// } +// +// var stateVersion *tfe.StateVersion +// for _, sv := range stateVersionsAPI.stateVersions { +// stateVersion = sv +// } +// +// if stateVersionsAPI.outputStates[stateVersion.ID] == nil || len(stateVersionsAPI.outputStates[stateVersion.ID]) == 0 { +// t.Fatal("no state version outputs in the mock client") +// } +//} + +func TestCLoudState_TestRemoteLocks(t *testing.T) { b, bCleanup := testBackendWithName(t) defer bCleanup() @@ -78,26 +77,30 @@ func TestRemoteClient_TestRemoteLocks(t *testing.T) { t.Fatalf("expected no error, got %v", err) } - remote.TestRemoteLocks(t, s1.(*State).Client, s2.(*State).Client) + TestCloudLocks(t, s1, s2) } -func TestRemoteClient_withRunID(t *testing.T) { +func TestCloudState_withRunID(t *testing.T) { // Set the TFE_RUN_ID environment variable before creating the client! if err := os.Setenv("TFE_RUN_ID", GenerateID("run-")); err != nil { t.Fatalf("error setting env var TFE_RUN_ID: %v", err) } // Create a new test client. - client := testRemoteClient(t) + state := testCloudState(t) // Create a new empty state. sf := statefile.New(states.NewState(), "", 0) var buf bytes.Buffer statefile.Write(sf, &buf) - // Store the new state to verify (this will be done - // by the mock that is used) that the run ID is set. - if err := client.Put(buf.Bytes()); err != nil { - t.Fatalf("expected no error, got %v", err) + jsonState, err := ioutil.ReadFile("../command/testdata/show-json-state/sensitive-variables/output.json") + + if err != nil { + t.Fatal(err) + } + + if err := state.uploadState(state.lineage, state.serial, state.forcePush, buf.Bytes(), jsonState); err != nil { + t.Fatalf("put: %s", err) } } diff --git a/internal/cloud/state.go b/internal/cloud/state.go index bc1cc1414e11..02622642c406 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -1,30 +1,59 @@ package cloud import ( + "bytes" "context" + "crypto/md5" + "encoding/base64" "encoding/json" "errors" "fmt" "log" + "os" "strings" + "sync" - "github.com/hashicorp/go-tfe" + tfe "github.com/hashicorp/go-tfe" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" + uuid "github.com/hashicorp/go-uuid" + "github.com/hashicorp/terraform/internal/command/jsonstate" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" + "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terraform" ) -// State is similar to remote State and delegates to it, except in the case of output values, -// which use a separate methodology that ensures the caller is authorized to read cloud -// workspace outputs. +// State implements the State interfaces in the state package to handle +// reading and writing the remote state to TFC. This State on its own does no +// local caching so every persist will go to the remote storage and local +// writes will go to memory. type State struct { - Client *remoteClient + mu sync.Mutex - delegate remote.State + // Client Client + + // We track two pieces of meta data in addition to the state itself: + // + // lineage - the state's unique ID + // serial - the monotonic counter of "versions" of the state + // + // Both of these (along with state) have a sister field + // that represents the values read in from an existing source. + // All three of these values are used to determine if the new + // state has changed from an existing state we read in. + lineage, readLineage string + serial, readSerial uint64 + state, readState *states.State + disableLocks bool + tfeClient *tfe.Client + organization string + workspace *tfe.Workspace + stateUploadErr bool + forcePush bool + lockInfo *statemgr.LockInfo } var ErrStateVersionUnauthorizedUpgradeState = errors.New(strings.TrimSpace(` @@ -34,69 +63,355 @@ of authorization and therefore this error can usually be fixed by upgrading the remote state version. `)) -// Proof that cloud State is a statemgr.Persistent interface -var _ statemgr.Persistent = (*State)(nil) +var _ statemgr.Full = (*State)(nil) +var _ statemgr.Migrator = (*State)(nil) + +// statemgr.Reader impl. +func (s *State) State() *states.State { + s.mu.Lock() + defer s.mu.Unlock() + + return s.state.DeepCopy() +} + +// StateForMigration is part of our implementation of statemgr.Migrator. +func (s *State) StateForMigration() *statefile.File { + s.mu.Lock() + defer s.mu.Unlock() + + return statefile.New(s.state.DeepCopy(), s.lineage, s.serial) +} -func NewState(client *remoteClient) *State { - return &State{ - Client: client, - delegate: remote.State{Client: client}, +// WriteStateForMigration is part of our implementation of statemgr.Migrator. +func (s *State) WriteStateForMigration(f *statefile.File, force bool) error { + s.mu.Lock() + defer s.mu.Unlock() + + if !force { + checkFile := statefile.New(s.state, s.lineage, s.serial) + if err := statemgr.CheckValidImport(f, checkFile); err != nil { + return err + } + } + + // The remote backend needs to pass the `force` flag through to its client. + // For backends that support such operations, inform the client + // that a force push has been requested + if force { + s.EnableForcePush() } + + // We create a deep copy of the state here, because the caller also has + // a reference to the given object and can potentially go on to mutate + // it after we return, but we want the snapshot at this point in time. + s.state = f.State.DeepCopy() + s.lineage = f.Lineage + s.serial = f.Serial + + return nil } -// State delegates calls to read State to the remote State -func (s *State) State() *states.State { - return s.delegate.State() +// DisableLocks turns the Lock and Unlock methods into no-ops. This is intended +// to be called during initialization of a state manager and should not be +// called after any of the statemgr.Full interface methods have been called. +func (s *State) DisableLocks() { + s.disableLocks = true } -// Lock delegates calls to lock state to the remote State -func (s *State) Lock(info *statemgr.LockInfo) (string, error) { - return s.delegate.Lock(info) +// StateSnapshotMeta returns the metadata from the most recently persisted +// or refreshed persistent state snapshot. +// +// This is an implementation of statemgr.PersistentMeta. +func (s *State) StateSnapshotMeta() statemgr.SnapshotMeta { + return statemgr.SnapshotMeta{ + Lineage: s.lineage, + Serial: s.serial, + } } -// Unlock delegates calls to unlock state to the remote State -func (s *State) Unlock(id string) error { - return s.delegate.Unlock(id) +// statemgr.Writer impl. +func (s *State) WriteState(state *states.State) error { + s.mu.Lock() + defer s.mu.Unlock() + + // We create a deep copy of the state here, because the caller also has + // a reference to the given object and can potentially go on to mutate + // it after we return, but we want the snapshot at this point in time. + s.state = state.DeepCopy() + + return nil } -// RefreshState delegates calls to refresh State to the remote State +// statemgr.Persister impl. +func (s *State) PersistState(schemas *terraform.Schemas) error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.readState != nil { + lineageUnchanged := s.readLineage != "" && s.lineage == s.readLineage + serialUnchanged := s.readSerial != 0 && s.serial == s.readSerial + stateUnchanged := statefile.StatesMarshalEqual(s.state, s.readState) + if stateUnchanged && lineageUnchanged && serialUnchanged { + // If the state, lineage or serial haven't changed at all then we have nothing to do. + return nil + } + s.serial++ + } else { + // 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() + if err != nil { + return fmt.Errorf("failed checking for existing remote state: %s", err) + } + if s.lineage == "" { // indicates that no state snapshot is present yet + lineage, err := uuid.GenerateUUID() + if err != nil { + return fmt.Errorf("failed to generate initial lineage: %v", err) + } + s.lineage = lineage + s.serial = 0 + } + } + + f := statefile.New(s.state, s.lineage, s.serial) + + var buf bytes.Buffer + err := statefile.Write(f, &buf) + if err != nil { + return err + } + + var jsonState []byte + if schemas != nil { + jsonState, err = jsonstate.Marshal(f, schemas) + if err != nil { + return err + } + } + + err = s.uploadState(s.lineage, s.serial, s.forcePush, buf.Bytes(), jsonState) + if err != nil { + s.stateUploadErr = true + return fmt.Errorf("error uploading state: %w", err) + } + // After we've successfully persisted, what we just wrote is our new + // reference state until someone calls RefreshState again. + // We've potentially overwritten (via force) the state, lineage + // and / or serial (and serial was incremented) so we copy over all + // three fields so everything matches the new state and a subsequent + // operation would correctly detect no changes to the lineage, serial or state. + s.readState = s.state.DeepCopy() + s.readLineage = s.lineage + s.readSerial = s.serial + return nil +} + +func (s *State) uploadState(lineage string, serial uint64, isForcePush bool, state, jsonState []byte) error { + ctx := context.Background() + + options := tfe.StateVersionCreateOptions{ + Lineage: tfe.String(lineage), + Serial: tfe.Int64(int64(serial)), + MD5: tfe.String(fmt.Sprintf("%x", md5.Sum(state))), + State: tfe.String(base64.StdEncoding.EncodeToString(state)), + Force: tfe.Bool(isForcePush), + JSONState: tfe.String(base64.StdEncoding.EncodeToString(jsonState)), + } + + // If we have a run ID, make sure to add it to the options + // so the state will be properly associated with the run. + runID := os.Getenv("TFE_RUN_ID") + if runID != "" { + options.Run = &tfe.Run{ID: runID} + } + // Create the new state. + _, err := s.tfeClient.StateVersions.Create(ctx, s.workspace.ID, options) + return err +} + +// Lock calls the Client's Lock method if it's implemented. +func (s *State) Lock(info *statemgr.LockInfo) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if s.disableLocks { + return "", nil + } + ctx := context.Background() + + lockErr := &statemgr.LockError{Info: s.lockInfo} + + // Lock the workspace. + _, err := s.tfeClient.Workspaces.Lock(ctx, s.workspace.ID, tfe.WorkspaceLockOptions{ + Reason: tfe.String("Locked by Terraform"), + }) + if err != nil { + if err == tfe.ErrWorkspaceLocked { + lockErr.Info = info + err = fmt.Errorf("%s (lock ID: \"%s/%s\")", err, s.organization, s.workspace.Name) + } + lockErr.Err = err + return "", lockErr + } + + s.lockInfo = info + + return s.lockInfo.ID, nil +} + +// statemgr.Refresher impl. func (s *State) RefreshState() error { - return s.delegate.RefreshState() + s.mu.Lock() + defer s.mu.Unlock() + return s.refreshState() } -// PersistState delegates calls to persist State to the remote State -func (s *State) PersistState(schemas *terraform.Schemas) error { - return s.delegate.PersistState(schemas) +// 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 { + payload, err := s.getStatePayload() + if err != nil { + return err + } + + // no remote state is OK + if payload == nil { + s.readState = nil + s.lineage = "" + s.serial = 0 + return nil + } + + stateFile, err := statefile.Read(bytes.NewReader(payload.Data)) + if err != nil { + return err + } + + s.lineage = stateFile.Lineage + s.serial = stateFile.Serial + s.state = stateFile.State + + // Properties from the remote must be separate so we can + // track changes as lineage, serial and/or state are mutated + s.readLineage = stateFile.Lineage + s.readSerial = stateFile.Serial + s.readState = s.state.DeepCopy() + return nil } -// WriteState delegates calls to write State to the remote State -func (s *State) WriteState(state *states.State) error { - return s.delegate.WriteState(state) +func (s *State) getStatePayload() (*remote.Payload, error) { + ctx := context.Background() + + sv, err := s.tfeClient.StateVersions.ReadCurrent(ctx, s.workspace.ID) + if err != nil { + if err == tfe.ErrResourceNotFound { + // If no state exists, then return nil. + return nil, nil + } + return nil, fmt.Errorf("error retrieving state: %v", err) + } + + state, err := s.tfeClient.StateVersions.Download(ctx, sv.DownloadURL) + if err != nil { + return nil, fmt.Errorf("error downloading state: %v", err) + } + + // If the state is empty, then return nil. + if len(state) == 0 { + return nil, nil + } + + // Get the MD5 checksum of the state. + sum := md5.Sum(state) + + return &remote.Payload{ + Data: state, + MD5: sum[:], + }, nil } -func (s *State) fallbackReadOutputsFromFullState() (map[string]*states.OutputValue, error) { - log.Printf("[DEBUG] falling back to reading full state") +// Unlock calls the Client's Unlock method if it's implemented. +func (s *State) Unlock(id string) error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.disableLocks { + return nil + } + + ctx := context.Background() + + // We first check if there was an error while uploading the latest + // state. If so, we will not unlock the workspace to prevent any + // changes from being applied until the correct state is uploaded. + if s.stateUploadErr { + return nil + } + + lockErr := &statemgr.LockError{Info: s.lockInfo} + + // With lock info this should be treated as a normal unlock. + if s.lockInfo != nil { + // Verify the expected lock ID. + if s.lockInfo.ID != id { + lockErr.Err = fmt.Errorf("lock ID does not match existing lock") + return lockErr + } + + // Unlock the workspace. + _, err := s.tfeClient.Workspaces.Unlock(ctx, s.workspace.ID) + if err != nil { + lockErr.Err = err + return lockErr + } + + return nil + } - if err := s.RefreshState(); err != nil { - return nil, fmt.Errorf("failed to load state: %w", err) + // Verify the optional force-unlock lock ID. + if s.organization+"/"+s.workspace.Name != id { + lockErr.Err = fmt.Errorf( + "lock ID %q does not match existing lock ID \"%s/%s\"", + id, + s.organization, + s.workspace.Name, + ) + return lockErr } - state := s.State() - if state == nil { - // We know that there is supposed to be state (and this is not simply a new workspace - // without state) because the fallback is only invoked when outputs are present but - // detailed types are not available. - return nil, ErrStateVersionUnauthorizedUpgradeState + // Force unlock the workspace. + _, err := s.tfeClient.Workspaces.ForceUnlock(ctx, s.workspace.ID) + if err != nil { + lockErr.Err = err + return lockErr + } + + return nil +} + +// Delete the remote state. +func (s *State) Delete() error { + err := s.tfeClient.Workspaces.Delete(context.Background(), s.organization, s.workspace.Name) + if err != nil && err != tfe.ErrResourceNotFound { + return fmt.Errorf("error deleting workspace %s: %v", s.workspace.Name, err) } - return state.RootModule().OutputValues, nil + return nil +} + +// EnableForcePush to allow the remote client to overwrite state +// by implementing remote.ClientForcePusher +func (s *State) EnableForcePush() { + s.forcePush = true } // GetRootOutputValues fetches output values from Terraform Cloud func (s *State) GetRootOutputValues() (map[string]*states.OutputValue, error) { ctx := context.Background() - so, err := s.Client.client.StateVersionOutputs.ReadCurrent(ctx, s.Client.workspace.ID) + so, err := s.tfeClient.StateVersionOutputs.ReadCurrent(ctx, s.workspace.ID) if err != nil { return nil, fmt.Errorf("could not read state version outputs: %w", err) @@ -110,13 +425,27 @@ func (s *State) GetRootOutputValues() (map[string]*states.OutputValue, error) { // with a version of terraform < 1.3.0. In this case, we'll eject completely from this // function and fall back to the old behavior of reading the entire state file, which // requires a higher level of authorization. - return s.fallbackReadOutputsFromFullState() + log.Printf("[DEBUG] falling back to reading full state") + + if err := s.RefreshState(); err != nil { + return nil, fmt.Errorf("failed to load state: %w", err) + } + + state := s.State() + if state == nil { + // We know that there is supposed to be state (and this is not simply a new workspace + // without state) because the fallback is only invoked when outputs are present but + // detailed types are not available. + return nil, ErrStateVersionUnauthorizedUpgradeState + } + + return state.RootModule().OutputValues, nil } if output.Sensitive { // Since this is a sensitive value, the output must be requested explicitly in order to // read its value, which is assumed to be present by callers - sensitiveOutput, err := s.Client.client.StateVersionOutputs.Read(ctx, output.ID) + sensitiveOutput, err := s.tfeClient.StateVersionOutputs.Read(ctx, output.ID) if err != nil { return nil, fmt.Errorf("could not read state version output %s: %w", output.ID, err) } diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index 738ae721a44d..72e8cf5ceebb 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -27,14 +27,9 @@ func TestState_GetRootOutputValues(t *testing.T) { b, bCleanup := testBackendWithOutputs(t) defer bCleanup() - client := &remoteClient{ - client: b.client, - workspace: &tfe.Workspace{ - ID: "ws-abcd", - }, - } - - state := NewState(client) + state := &State{tfeClient: b.client, organization: b.organization, workspace: &tfe.Workspace{ + ID: "ws-abcd", + }} outputs, err := state.GetRootOutputValues() if err != nil { diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index cfb49cf9b352..95ac257c6d3c 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -1,10 +1,14 @@ package cloud import ( + "bytes" "context" "encoding/json" "fmt" + "github.com/hashicorp/terraform/internal/states/statefile" + "github.com/hashicorp/terraform/internal/states/statemgr" "io" + "io/ioutil" "net/http" "net/http/httptest" "path" @@ -23,7 +27,6 @@ import ( "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/httpclient" "github.com/hashicorp/terraform/internal/providers" - "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/hashicorp/terraform/version" @@ -110,7 +113,7 @@ func testBackendNoOperations(t *testing.T) (*Cloud, func()) { return testBackend(t, obj) } -func testRemoteClient(t *testing.T) remote.Client { +func testCloudState(t *testing.T) *State { b, bCleanup := testBackendWithName(t) defer bCleanup() @@ -119,7 +122,7 @@ func testRemoteClient(t *testing.T) remote.Client { t.Fatalf("error: %v", err) } - return raw.(*State).Client + return raw.(*State) } func testBackendWithOutputs(t *testing.T) (*Cloud, func()) { @@ -443,3 +446,95 @@ func testVariables(s terraform.ValueSourceType, vs ...string) map[string]backend } return vars } + +func TestState(t *testing.T, state *State) { + var buf bytes.Buffer + s := statemgr.TestFullInitialState() + sf := statefile.New(s, "stub-lineage", 2) + err := statefile.Write(sf, &buf) + if err != nil { + t.Fatalf("err: %s", err) + } + data := buf.Bytes() + + jsonState, err := ioutil.ReadFile("../command/testdata/show-json-state/sensitive-variables/output.json") + + if err != nil { + t.Fatal(err) + } + + if err := state.uploadState(state.lineage, state.serial, state.forcePush, data, jsonState); err != nil { + t.Fatalf("put: %s", err) + } + + payload, err := state.getStatePayload() + if err != nil { + t.Fatalf("get: %s", err) + } + if !bytes.Equal(payload.Data, data) { + t.Fatalf("expected full state %q\n\ngot: %q", string(payload.Data), string(data)) + } + + if err := state.Delete(); err != nil { + t.Fatalf("delete: %s", err) + } + + p, err := state.getStatePayload() + if err != nil { + t.Fatalf("get: %s", err) + } + if p != nil { + t.Fatalf("expected empty state, got: %q", string(p.Data)) + } +} + +func TestCloudLocks(t *testing.T, a, b statemgr.Full) { + lockerA, ok := a.(statemgr.Locker) + if !ok { + t.Fatal("client A not a statemgr.Locker") + } + + lockerB, ok := b.(statemgr.Locker) + if !ok { + t.Fatal("client B not a statemgr.Locker") + } + + infoA := statemgr.NewLockInfo() + infoA.Operation = "test" + infoA.Who = "clientA" + + infoB := statemgr.NewLockInfo() + infoB.Operation = "test" + infoB.Who = "clientB" + + lockIDA, err := lockerA.Lock(infoA) + if err != nil { + t.Fatal("unable to get initial lock:", err) + } + + _, err = lockerB.Lock(infoB) + if err == nil { + lockerA.Unlock(lockIDA) + t.Fatal("client B obtained lock while held by client A") + } + if _, ok := err.(*statemgr.LockError); !ok { + t.Errorf("expected a LockError, but was %t: %s", err, err) + } + + if err := lockerA.Unlock(lockIDA); err != nil { + t.Fatal("error unlocking client A", err) + } + + lockIDB, err := lockerB.Lock(infoB) + if err != nil { + t.Fatal("unable to obtain lock from client B") + } + + if lockIDB == lockIDA { + t.Fatalf("duplicate lock IDs: %q", lockIDB) + } + + if err = lockerB.Unlock(lockIDB); err != nil { + t.Fatal("error unlocking client B:", err) + } +} From 12e3560d21e1aeefc429b5bff74ded7cc551faf4 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 26 Aug 2022 15:32:18 -0500 Subject: [PATCH 04/18] check for cloud integration mode --- internal/command/command_test.go | 56 ++++++++++++++++++++++ internal/command/helper.go | 8 ++++ internal/command/import.go | 2 +- internal/command/state_mv.go | 13 +++-- internal/command/state_push.go | 6 +-- internal/command/state_replace_provider.go | 13 +++-- internal/command/state_rm.go | 13 +++-- internal/command/taint.go | 6 +-- internal/command/untaint.go | 6 +-- 9 files changed, 104 insertions(+), 19 deletions(-) diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 43c4640822eb..df02a9166c23 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -766,6 +766,62 @@ func testBackendState(t *testing.T, s *states.State, c int) (*legacy.State, *htt return state, srv } +// testCloudBackendState is used to make a cloud configured +// backend. +// +// When using this function, the configuration fixture for the test must +// include an empty configuration block for the HTTP backend, like this: +// +// terraform { +// cloud { +// } +// } +// +// If such a block isn't present, then an error will +// be returned about the backend configuration having changed and that +// "terraform init" must be run, since the test backend config cache created +// by this function contains the hash for an empty configuration. +func testCloudBackendState(t *testing.T, s *states.State, c int) (*legacy.State, *httptest.Server) { + t.Helper() + + var b64md5 string + buf := bytes.NewBuffer(nil) + + cb := func(resp http.ResponseWriter, req *http.Request) { + if req.Method == "PUT" { + resp.WriteHeader(c) + return + } + if s == nil { + resp.WriteHeader(404) + return + } + + resp.Header().Set("Content-MD5", b64md5) + resp.Write(buf.Bytes()) + } + + // If a state was given, make sure we calculate the proper b64md5 + if s != nil { + err := statefile.Write(&statefile.File{State: s}, buf) + if err != nil { + t.Fatalf("err: %v", err) + } + md5 := md5.Sum(buf.Bytes()) + b64md5 = base64.StdEncoding.EncodeToString(md5[:16]) + } + + srv := httptest.NewServer(http.HandlerFunc(cb)) + + state := legacy.NewState() + state.Backend = &legacy.BackendState{ + Type: "cloud", + ConfigRaw: json.RawMessage(fmt.Sprintf(`{"address":%q}`, srv.URL)), + } + + return state, srv +} + // testRemoteState is used to make a test HTTP server to return a given // state file that can be used for testing legacy remote state. // diff --git a/internal/command/helper.go b/internal/command/helper.go index ef13934cc0fd..46f3863354bc 100644 --- a/internal/command/helper.go +++ b/internal/command/helper.go @@ -1,6 +1,8 @@ package command import ( + "github.com/hashicorp/terraform/internal/backend" + "github.com/hashicorp/terraform/internal/cloud" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/terraform" @@ -17,6 +19,12 @@ however, historic state information may be missing if the affected integration r %s ` +func isCloudMode(b backend.Enhanced) bool { + _, ok := b.(*cloud.Cloud) + + return ok +} + func getSchemas(c *Meta, state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics diff --git a/internal/command/import.go b/internal/command/import.go index 8d98fb74c877..a7e829882d91 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -250,7 +250,7 @@ func (c *ImportCommand) Run(args []string) int { // Get schemas, if possible, before writing state schemas, diags := getSchemas(&c.Meta, newState, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 231f9649962a..d057834b5934 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -386,18 +386,25 @@ func (c *StateMvCommand) Run(args []string) int { return 0 // This is as far as we go in dry-run mode } + b, backendDiags := c.Backend(&BackendOpts{}) + diags = diags.Append(backendDiags) + if backendDiags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } + path, err := os.Getwd() - if err != nil { + if err != nil && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, stateTo, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) return 1 } diff --git a/internal/command/state_push.go b/internal/command/state_push.go index fd3f55a248cf..1d9dfcc52b79 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -129,17 +129,17 @@ func (c *StatePushCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() - if err != nil { + if err != nil && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, srcStateFile.State, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 4b08439e23e2..19a1f8df7752 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -161,19 +161,26 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { resource.ProviderConfig.Provider = to } + b, backendDiags := c.Backend(&BackendOpts{}) + diags = diags.Append(backendDiags) + if backendDiags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } + // Get schemas, if possible, before writing state path, err := os.Getwd() - if err != nil { + if err != nil && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index 1273d2cca879..8dfc9f7eb5b5 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -111,19 +111,26 @@ func (c *StateRmCommand) Run(args []string) int { return 0 // This is as far as we go in dry-run mode } + b, backendDiags := c.Backend(&BackendOpts{}) + diags = diags.Append(backendDiags) + if backendDiags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } + // Get schemas, if possible, before writing state path, err := os.Getwd() - if err != nil { + if err != nil && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/taint.go b/internal/command/taint.go index 754636078212..03ddbbddd778 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -128,17 +128,17 @@ func (c *TaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() - if err != nil { + if err != nil && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/untaint.go b/internal/command/untaint.go index a25d5de470d7..68d29d99befa 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -167,17 +167,17 @@ func (c *UntaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state path, err := os.Getwd() - if err != nil { + if err != nil && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } config, diags := c.loadConfig(path) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() { + if diags.HasErrors() && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } obj.Status = states.ObjectReady From d5decc2407a918f0a3dec61e921c07a1a640340c Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 26 Aug 2022 16:00:20 -0500 Subject: [PATCH 05/18] update flow of schema checking --- internal/command/state_mv.go | 22 +++++++++++++--------- internal/command/state_push.go | 20 ++++++++++++-------- internal/command/state_replace_provider.go | 20 ++++++++++++-------- internal/command/state_rm.go | 20 ++++++++++++-------- internal/command/taint.go | 20 ++++++++++++-------- internal/command/untaint.go | 21 +++++++++++++-------- 6 files changed, 74 insertions(+), 49 deletions(-) diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index d057834b5934..93b8117ca378 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/terraform" "os" "strings" @@ -393,20 +394,23 @@ func (c *StateMvCommand) Run(args []string) int { return 1 } + // Get schemas, if possible, before writing state + schemas := &terraform.Schemas{} path, err := os.Getwd() - if err != nil && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } + schemaErr := err != nil - config, diags := c.loadConfig(path) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + if !schemaErr { + config, diags := c.loadConfig(path) + schemaErr = diags.HasErrors() + + if !schemaErr { + schemas, diags = getSchemas(&c.Meta, stateTo, config) + schemaErr = diags.HasErrors() + } } - schemas, diags := getSchemas(&c.Meta, stateTo, config) - if diags.HasErrors() && isCloudMode(b) { + if schemaErr && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - return 1 } // Write the new state diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 1d9dfcc52b79..95dc0fb8106d 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/terraform" "io" "os" "strings" @@ -128,18 +129,21 @@ func (c *StatePushCommand) Run(args []string) int { } // Get schemas, if possible, before writing state + schemas := &terraform.Schemas{} path, err := os.Getwd() - if err != nil && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } + schemaErr := err != nil - config, diags := c.loadConfig(path) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + if !schemaErr { + config, diags := c.loadConfig(path) + schemaErr = diags.HasErrors() + + if !schemaErr { + schemas, diags = getSchemas(&c.Meta, srcStateFile.State, config) + schemaErr = diags.HasErrors() + } } - schemas, diags := getSchemas(&c.Meta, srcStateFile.State, config) - if diags.HasErrors() && isCloudMode(b) { + if schemaErr && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 19a1f8df7752..684e8d340959 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/terraform" "os" "strings" @@ -169,18 +170,21 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { } // Get schemas, if possible, before writing state + schemas := &terraform.Schemas{} path, err := os.Getwd() - if err != nil && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } + schemaErr := err != nil - config, diags := c.loadConfig(path) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + if !schemaErr { + config, diags := c.loadConfig(path) + schemaErr = diags.HasErrors() + + if !schemaErr { + schemas, diags = getSchemas(&c.Meta, state, config) + schemaErr = diags.HasErrors() + } } - schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() && isCloudMode(b) { + if schemaErr && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index 8dfc9f7eb5b5..0344d51ce525 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/terraform" "os" "strings" @@ -119,18 +120,21 @@ func (c *StateRmCommand) Run(args []string) int { } // Get schemas, if possible, before writing state + schemas := &terraform.Schemas{} path, err := os.Getwd() - if err != nil && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } + schemaErr := err != nil - config, diags := c.loadConfig(path) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + if !schemaErr { + config, diags := c.loadConfig(path) + schemaErr = diags.HasErrors() + + if !schemaErr { + schemas, diags = getSchemas(&c.Meta, state, config) + schemaErr = diags.HasErrors() + } } - schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() && isCloudMode(b) { + if schemaErr && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/taint.go b/internal/command/taint.go index 03ddbbddd778..d2710d6b7fbb 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/terraform" "os" "strings" @@ -127,18 +128,21 @@ func (c *TaintCommand) Run(args []string) int { } // Get schemas, if possible, before writing state + schemas := &terraform.Schemas{} path, err := os.Getwd() - if err != nil && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } + schemaErr := err != nil - config, diags := c.loadConfig(path) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + if !schemaErr { + config, diags := c.loadConfig(path) + schemaErr = diags.HasErrors() + + if !schemaErr { + schemas, diags = getSchemas(&c.Meta, state, config) + schemaErr = diags.HasErrors() + } } - schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() && isCloudMode(b) { + if schemaErr && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/untaint.go b/internal/command/untaint.go index 68d29d99befa..d66b6d1bce7e 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/terraform" "os" "strings" @@ -166,20 +167,24 @@ func (c *UntaintCommand) Run(args []string) int { } // Get schemas, if possible, before writing state + schemas := &terraform.Schemas{} path, err := os.Getwd() - if err != nil && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } + schemaErr := err != nil - config, diags := c.loadConfig(path) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + if !schemaErr { + config, diags := c.loadConfig(path) + schemaErr = diags.HasErrors() + + if !schemaErr { + schemas, diags = getSchemas(&c.Meta, state, config) + schemaErr = diags.HasErrors() + } } - schemas, diags := getSchemas(&c.Meta, state, config) - if diags.HasErrors() && isCloudMode(b) { + if schemaErr && isCloudMode(b) { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } + obj.Status = states.ObjectReady ss.SetResourceInstanceCurrent(addr, obj, rs.ProviderConfig) From 344379f5c794d0fb52a9e485426ed438b4fba084 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Fri, 26 Aug 2022 16:26:09 -0500 Subject: [PATCH 06/18] fix cloud state breaking? --- internal/cloud/state.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/internal/cloud/state.go b/internal/cloud/state.go index 02622642c406..44f5faa1e1bf 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -210,13 +210,29 @@ func (s *State) PersistState(schemas *terraform.Schemas) error { func (s *State) uploadState(lineage string, serial uint64, isForcePush bool, state, jsonState []byte) error { ctx := context.Background() + // Read the raw state into a Terraform state. + stateFile, err := statefile.Read(bytes.NewReader(state)) + if err != nil { + return fmt.Errorf("failed to read state: %w", err) + } + + ov, err := jsonstate.MarshalOutputs(stateFile.State.RootModule().OutputValues) + if err != nil { + return fmt.Errorf("failed to translate outputs: %w", err) + } + o, err := json.Marshal(ov) + if err != nil { + return fmt.Errorf("failed to marshal outputs to json: %w", err) + } + options := tfe.StateVersionCreateOptions{ - Lineage: tfe.String(lineage), - Serial: tfe.Int64(int64(serial)), - MD5: tfe.String(fmt.Sprintf("%x", md5.Sum(state))), - State: tfe.String(base64.StdEncoding.EncodeToString(state)), - Force: tfe.Bool(isForcePush), - JSONState: tfe.String(base64.StdEncoding.EncodeToString(jsonState)), + Lineage: tfe.String(lineage), + Serial: tfe.Int64(int64(serial)), + MD5: tfe.String(fmt.Sprintf("%x", md5.Sum(state))), + State: tfe.String(base64.StdEncoding.EncodeToString(state)), + Force: tfe.Bool(isForcePush), + JSONState: tfe.String(base64.StdEncoding.EncodeToString(jsonState)), + JSONStateOutputs: tfe.String(base64.StdEncoding.EncodeToString(o)), } // If we have a run ID, make sure to add it to the options @@ -226,7 +242,7 @@ func (s *State) uploadState(lineage string, serial uint64, isForcePush bool, sta options.Run = &tfe.Run{ID: runID} } // Create the new state. - _, err := s.tfeClient.StateVersions.Create(ctx, s.workspace.ID, options) + _, err = s.tfeClient.StateVersions.Create(ctx, s.workspace.ID, options) return err } From 00cc1ea26dee6f2b5d9ac44de1b4aca178489773 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 11:10:03 -0500 Subject: [PATCH 07/18] refactor getSchemas --- internal/command/command_test.go | 56 ---------------------- internal/command/import.go | 9 ++-- internal/command/meta.go | 42 ++++++++++++++++ internal/command/meta_backend_migrate.go | 20 ++------ internal/command/state_mv.go | 23 +++------ internal/command/state_push.go | 22 +++------ internal/command/state_replace_provider.go | 23 +++------ internal/command/state_rm.go | 23 +++------ internal/command/taint.go | 21 ++------ internal/command/untaint.go | 21 ++------ 10 files changed, 87 insertions(+), 173 deletions(-) diff --git a/internal/command/command_test.go b/internal/command/command_test.go index df02a9166c23..43c4640822eb 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -766,62 +766,6 @@ func testBackendState(t *testing.T, s *states.State, c int) (*legacy.State, *htt return state, srv } -// testCloudBackendState is used to make a cloud configured -// backend. -// -// When using this function, the configuration fixture for the test must -// include an empty configuration block for the HTTP backend, like this: -// -// terraform { -// cloud { -// } -// } -// -// If such a block isn't present, then an error will -// be returned about the backend configuration having changed and that -// "terraform init" must be run, since the test backend config cache created -// by this function contains the hash for an empty configuration. -func testCloudBackendState(t *testing.T, s *states.State, c int) (*legacy.State, *httptest.Server) { - t.Helper() - - var b64md5 string - buf := bytes.NewBuffer(nil) - - cb := func(resp http.ResponseWriter, req *http.Request) { - if req.Method == "PUT" { - resp.WriteHeader(c) - return - } - if s == nil { - resp.WriteHeader(404) - return - } - - resp.Header().Set("Content-MD5", b64md5) - resp.Write(buf.Bytes()) - } - - // If a state was given, make sure we calculate the proper b64md5 - if s != nil { - err := statefile.Write(&statefile.File{State: s}, buf) - if err != nil { - t.Fatalf("err: %v", err) - } - md5 := md5.Sum(buf.Bytes()) - b64md5 = base64.StdEncoding.EncodeToString(md5[:16]) - } - - srv := httptest.NewServer(http.HandlerFunc(cb)) - - state := legacy.NewState() - state.Backend = &legacy.BackendState{ - Type: "cloud", - ConfigRaw: json.RawMessage(fmt.Sprintf(`{"address":%q}`, srv.URL)), - } - - return state, srv -} - // testRemoteState is used to make a test HTTP server to return a given // state file that can be used for testing legacy remote state. // diff --git a/internal/command/import.go b/internal/command/import.go index a7e829882d91..08aa7f6ff005 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -249,9 +249,12 @@ func (c *ImportCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas, diags := getSchemas(&c.Meta, newState, config) - if diags.HasErrors() && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + var schemas *terraform.Schemas + if isCloudMode(b) { + schemas, diags = c.GetSchemas(newState) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + } } // Persist the final state diff --git a/internal/command/meta.go b/internal/command/meta.go index 594292f1b95c..8139a63262ba 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -6,6 +6,8 @@ import ( "errors" "flag" "fmt" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/states" "io/ioutil" "log" "os" @@ -779,3 +781,43 @@ func (m *Meta) checkRequiredVersion() tfdiags.Diagnostics { return nil } + +// GetSchemas loads and returns the schemas +func (c *Meta) GetSchemas(state *states.State) (*terraform.Schemas, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + var config *configs.Config + + path, err := os.Getwd() + if err != nil { + diags.Append(err) + return nil, diags + } + + config, diags = c.loadConfig(path) + if diags.HasErrors() { + diags.Append(diags) + return nil, diags + } + + if config != nil || state != nil { + opts, err := c.contextOpts() + if err != nil { + diags = diags.Append(err) + return nil, diags + } + tfCtx, ctxDiags := terraform.NewContext(opts) + diags = diags.Append(ctxDiags) + if ctxDiags.HasErrors() { + return nil, diags + } + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags := tfCtx.Schemas(config, state) + diags = diags.Append(schemaDiags) + if schemaDiags.HasErrors() { + return nil, diags + } + return schemas, diags + + } + return nil, diags +} diff --git a/internal/command/meta_backend_migrate.go b/internal/command/meta_backend_migrate.go index e0a81ba39008..ba0fbb55257d 100644 --- a/internal/command/meta_backend_migrate.go +++ b/internal/command/meta_backend_migrate.go @@ -435,26 +435,14 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { // both managers support such metadata. log.Print("[TRACE] backendMigrateState: migration confirmed, so migrating") - path, err := os.Getwd() - if err != nil { - return fmt.Errorf("could not get working directory") - } - - config, diags := m.loadConfig(path) - if diags.HasErrors() { - return diags.Err() - } - - schemas, diags := getSchemas(m, destination, config) - if diags.HasErrors() { - return diags.Err() - } - if err := statemgr.Migrate(destinationState, sourceState); err != nil { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) } - if err := destinationState.PersistState(schemas); err != nil { + // Fetching schemas during init might be more of a hassle than we want to attempt + // in the case that we're migrating to TFC backend, the initial JSON state won't + // be generated and stored. + if err := destinationState.PersistState(nil); err != nil { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) } diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 93b8117ca378..b00eb2d8ba48 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -3,7 +3,6 @@ package command import ( "fmt" "github.com/hashicorp/terraform/internal/terraform" - "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -387,7 +386,7 @@ func (c *StateMvCommand) Run(args []string) int { return 0 // This is as far as we go in dry-run mode } - b, backendDiags := c.Backend(&BackendOpts{}) + b, backendDiags := c.Backend(nil) diags = diags.Append(backendDiags) if backendDiags.HasErrors() { c.showDiagnostics(diags) @@ -395,24 +394,14 @@ func (c *StateMvCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas := &terraform.Schemas{} - path, err := os.Getwd() - schemaErr := err != nil - - if !schemaErr { - config, diags := c.loadConfig(path) - schemaErr = diags.HasErrors() - - if !schemaErr { - schemas, diags = getSchemas(&c.Meta, stateTo, config) - schemaErr = diags.HasErrors() + var schemas *terraform.Schemas + if isCloudMode(b) { + schemas, diags = c.GetSchemas(stateTo) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } } - if schemaErr && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } - // Write the new state if err := stateToMgr.WriteState(stateTo); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 95dc0fb8106d..8adf612f4076 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -3,6 +3,7 @@ package command import ( "fmt" "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/tfdiags" "io" "os" "strings" @@ -129,24 +130,15 @@ func (c *StatePushCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas := &terraform.Schemas{} - path, err := os.Getwd() - schemaErr := err != nil - - if !schemaErr { - config, diags := c.loadConfig(path) - schemaErr = diags.HasErrors() - - if !schemaErr { - schemas, diags = getSchemas(&c.Meta, srcStateFile.State, config) - schemaErr = diags.HasErrors() + var schemas *terraform.Schemas + if isCloudMode(b) { + var diags tfdiags.Diagnostics + schemas, diags = c.GetSchemas(srcStateFile.State) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } } - if schemaErr && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } - if err := stateMgr.WriteState(srcStateFile.State); err != nil { c.Ui.Error(fmt.Sprintf("Failed to write state: %s", err)) return 1 diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 684e8d340959..ec7062af8ca3 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -3,7 +3,6 @@ package command import ( "fmt" "github.com/hashicorp/terraform/internal/terraform" - "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -162,7 +161,7 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { resource.ProviderConfig.Provider = to } - b, backendDiags := c.Backend(&BackendOpts{}) + b, backendDiags := c.Backend(nil) diags = diags.Append(backendDiags) if backendDiags.HasErrors() { c.showDiagnostics(diags) @@ -170,24 +169,14 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas := &terraform.Schemas{} - path, err := os.Getwd() - schemaErr := err != nil - - if !schemaErr { - config, diags := c.loadConfig(path) - schemaErr = diags.HasErrors() - - if !schemaErr { - schemas, diags = getSchemas(&c.Meta, state, config) - schemaErr = diags.HasErrors() + var schemas *terraform.Schemas + if isCloudMode(b) { + schemas, diags = c.GetSchemas(state) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } } - if schemaErr && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } - // Write the updated state if err := stateMgr.WriteState(state); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index 0344d51ce525..f7c9a937db1c 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -3,7 +3,6 @@ package command import ( "fmt" "github.com/hashicorp/terraform/internal/terraform" - "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -112,7 +111,7 @@ func (c *StateRmCommand) Run(args []string) int { return 0 // This is as far as we go in dry-run mode } - b, backendDiags := c.Backend(&BackendOpts{}) + b, backendDiags := c.Backend(nil) diags = diags.Append(backendDiags) if backendDiags.HasErrors() { c.showDiagnostics(diags) @@ -120,24 +119,14 @@ func (c *StateRmCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas := &terraform.Schemas{} - path, err := os.Getwd() - schemaErr := err != nil - - if !schemaErr { - config, diags := c.loadConfig(path) - schemaErr = diags.HasErrors() - - if !schemaErr { - schemas, diags = getSchemas(&c.Meta, state, config) - schemaErr = diags.HasErrors() + var schemas *terraform.Schemas + if isCloudMode(b) { + schemas, diags = c.GetSchemas(state) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } } - if schemaErr && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } - if err := stateMgr.WriteState(state); err != nil { c.Ui.Error(fmt.Sprintf(errStateRmPersist, err)) return 1 diff --git a/internal/command/taint.go b/internal/command/taint.go index d2710d6b7fbb..547a0038e1b6 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -3,7 +3,6 @@ package command import ( "fmt" "github.com/hashicorp/terraform/internal/terraform" - "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -128,24 +127,14 @@ func (c *TaintCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas := &terraform.Schemas{} - path, err := os.Getwd() - schemaErr := err != nil - - if !schemaErr { - config, diags := c.loadConfig(path) - schemaErr = diags.HasErrors() - - if !schemaErr { - schemas, diags = getSchemas(&c.Meta, state, config) - schemaErr = diags.HasErrors() + var schemas *terraform.Schemas + if isCloudMode(b) { + schemas, diags = c.GetSchemas(state) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } } - if schemaErr && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } - ss := state.SyncWrapper() // Get the resource and instance we're going to taint diff --git a/internal/command/untaint.go b/internal/command/untaint.go index d66b6d1bce7e..ebfa26d69c1a 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -3,7 +3,6 @@ package command import ( "fmt" "github.com/hashicorp/terraform/internal/terraform" - "os" "strings" "github.com/hashicorp/terraform/internal/addrs" @@ -167,24 +166,14 @@ func (c *UntaintCommand) Run(args []string) int { } // Get schemas, if possible, before writing state - schemas := &terraform.Schemas{} - path, err := os.Getwd() - schemaErr := err != nil - - if !schemaErr { - config, diags := c.loadConfig(path) - schemaErr = diags.HasErrors() - - if !schemaErr { - schemas, diags = getSchemas(&c.Meta, state, config) - schemaErr = diags.HasErrors() + var schemas *terraform.Schemas + if isCloudMode(b) { + schemas, diags = c.GetSchemas(state) + if diags.HasErrors() { + c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } } - if schemaErr && isCloudMode(b) { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) - } - obj.Status = states.ObjectReady ss.SetResourceInstanceCurrent(addr, obj, rs.ProviderConfig) From 40263cd861fa59ef32788ecf0db22a297492ce7b Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 11:21:06 -0500 Subject: [PATCH 08/18] undo taint test changes --- internal/command/meta_backend_migrate.go | 1 - internal/command/taint_test.go | 11 +++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/internal/command/meta_backend_migrate.go b/internal/command/meta_backend_migrate.go index ba0fbb55257d..00d9c8ed8538 100644 --- a/internal/command/meta_backend_migrate.go +++ b/internal/command/meta_backend_migrate.go @@ -434,7 +434,6 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { // includes preserving any lineage/serial information where possible, if // both managers support such metadata. log.Print("[TRACE] backendMigrateState: migration confirmed, so migrating") - if err := statemgr.Migrate(destinationState, sourceState); err != nil { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) diff --git a/internal/command/taint_test.go b/internal/command/taint_test.go index b62be299ffa7..4120feb5ae1d 100644 --- a/internal/command/taint_test.go +++ b/internal/command/taint_test.go @@ -1,8 +1,8 @@ package command import ( + "github.com/google/go-cmp/cmp" "os" - "regexp" "strings" "testing" @@ -381,13 +381,8 @@ Resource instance test_instance.bar was not found, but this is not an error because -allow-missing was set. `) - re, err := regexp.Compile(expected) - if err != nil { - t.Fatalf("Error compiling regexp: %s", err) - } - - if !re.MatchString(actual) { - t.Fatalf("wrong output\n expected: %s \n actual: %s", expected, actual) + if diff := cmp.Diff(expected, actual); diff != "" { + t.Fatalf("wrong output\n%s", diff) } } From b572e57fb3ea89b62cb4772d22d1ef71ddff9354 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 11:32:14 -0500 Subject: [PATCH 09/18] refactor GetSchemas to include an option to pass in a config --- internal/command/helper.go | 30 ---------------------- internal/command/import.go | 2 +- internal/command/meta.go | 13 +++++----- internal/command/show.go | 2 +- internal/command/state_mv.go | 2 +- internal/command/state_push.go | 2 +- internal/command/state_replace_provider.go | 2 +- internal/command/state_rm.go | 2 +- internal/command/taint.go | 2 +- internal/command/taint_test.go | 2 +- internal/command/untaint.go | 2 +- 11 files changed, 16 insertions(+), 45 deletions(-) diff --git a/internal/command/helper.go b/internal/command/helper.go index 46f3863354bc..2ef3dd5af89d 100644 --- a/internal/command/helper.go +++ b/internal/command/helper.go @@ -3,10 +3,6 @@ package command import ( "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/cloud" - "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/states" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/tfdiags" ) const failedToLoadSchemasMessage = ` @@ -24,29 +20,3 @@ func isCloudMode(b backend.Enhanced) bool { return ok } - -func getSchemas(c *Meta, state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - - if config != nil || state != nil { - opts, err := c.contextOpts() - if err != nil { - diags = diags.Append(err) - return nil, diags - } - tfCtx, ctxDiags := terraform.NewContext(opts) - diags = diags.Append(ctxDiags) - if ctxDiags.HasErrors() { - return nil, diags - } - var schemaDiags tfdiags.Diagnostics - schemas, schemaDiags := tfCtx.Schemas(config, state) - diags = diags.Append(schemaDiags) - if schemaDiags.HasErrors() { - return nil, diags - } - return schemas, diags - - } - return nil, diags -} diff --git a/internal/command/import.go b/internal/command/import.go index 08aa7f6ff005..24a0928aa169 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -251,7 +251,7 @@ func (c *ImportCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(newState) + schemas, diags = c.GetSchemas(newState, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/meta.go b/internal/command/meta.go index 8139a63262ba..6dc86c8a3d74 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -783,9 +783,8 @@ func (m *Meta) checkRequiredVersion() tfdiags.Diagnostics { } // GetSchemas loads and returns the schemas -func (c *Meta) GetSchemas(state *states.State) (*terraform.Schemas, tfdiags.Diagnostics) { +func (c *Meta) GetSchemas(state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - var config *configs.Config path, err := os.Getwd() if err != nil { @@ -793,10 +792,12 @@ func (c *Meta) GetSchemas(state *states.State) (*terraform.Schemas, tfdiags.Diag return nil, diags } - config, diags = c.loadConfig(path) - if diags.HasErrors() { - diags.Append(diags) - return nil, diags + if config == nil { + config, diags = c.loadConfig(path) + if diags.HasErrors() { + diags.Append(diags) + return nil, diags + } } if config != nil || state != nil { diff --git a/internal/command/show.go b/internal/command/show.go index c3486cbac70c..62e8dacafeaa 100644 --- a/internal/command/show.go +++ b/internal/command/show.go @@ -110,7 +110,7 @@ func (c *ShowCommand) show(path string) (*plans.Plan, *statefile.File, *configs. // Get schemas, if possible if config != nil || stateFile != nil { - schemas, diags = getSchemas(&c.Meta, stateFile.State, config) + schemas, diags = c.GetSchemas(stateFile.State, config) if diags.HasErrors() { return plan, stateFile, config, schemas, diags } diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index b00eb2d8ba48..8646974beaa2 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -396,7 +396,7 @@ func (c *StateMvCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(stateTo) + schemas, diags = c.GetSchemas(stateTo, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 8adf612f4076..8d275228c091 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -133,7 +133,7 @@ func (c *StatePushCommand) Run(args []string) int { var schemas *terraform.Schemas if isCloudMode(b) { var diags tfdiags.Diagnostics - schemas, diags = c.GetSchemas(srcStateFile.State) + schemas, diags = c.GetSchemas(srcStateFile.State, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index ec7062af8ca3..d8d160462115 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -171,7 +171,7 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state) + schemas, diags = c.GetSchemas(state, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index f7c9a937db1c..6eb5f2164168 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -121,7 +121,7 @@ func (c *StateRmCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state) + schemas, diags = c.GetSchemas(state, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/taint.go b/internal/command/taint.go index 547a0038e1b6..d5a257a77528 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -129,7 +129,7 @@ func (c *TaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state) + schemas, diags = c.GetSchemas(state, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } diff --git a/internal/command/taint_test.go b/internal/command/taint_test.go index 4120feb5ae1d..001d477082bc 100644 --- a/internal/command/taint_test.go +++ b/internal/command/taint_test.go @@ -1,11 +1,11 @@ package command import ( - "github.com/google/go-cmp/cmp" "os" "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/mitchellh/cli" "github.com/hashicorp/terraform/internal/addrs" diff --git a/internal/command/untaint.go b/internal/command/untaint.go index ebfa26d69c1a..140ecec3a108 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -168,7 +168,7 @@ func (c *UntaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state) + schemas, diags = c.GetSchemas(state, nil) if diags.HasErrors() { c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) } From bddf6a9b34e8202441669b734463f40712f9fe4a Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 14:13:18 -0500 Subject: [PATCH 10/18] updating to use the latest version of cloud/state.go and just pass schemas along to PersistState in the remote state --- .../remote-state/inmem/backend_test.go | 2 + internal/cloud/backend.go | 24 +- internal/cloud/backend_state_test.go | 117 +++-- internal/cloud/state.go | 430 ++---------------- internal/cloud/state_test.go | 11 +- internal/cloud/testing.go | 101 +--- .../command/state_replace_provider_test.go | 19 + 7 files changed, 151 insertions(+), 553 deletions(-) diff --git a/internal/backend/remote-state/inmem/backend_test.go b/internal/backend/remote-state/inmem/backend_test.go index 204858824d0a..b7e9a555a906 100644 --- a/internal/backend/remote-state/inmem/backend_test.go +++ b/internal/backend/remote-state/inmem/backend_test.go @@ -5,6 +5,8 @@ import ( "os" "testing" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/backend" statespkg "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index ded38f75f29f..669cc372270d 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -526,10 +526,15 @@ func (b *Cloud) DeleteWorkspace(name string) error { } // Configure the remote workspace name. - State := &State{tfeClient: b.client, organization: b.organization, workspace: &tfe.Workspace{ - Name: name, - }} - return State.Delete() + client := &remoteClient{ + client: b.client, + organization: b.organization, + workspace: &tfe.Workspace{ + Name: name, + }, + } + + return client.Delete() } // StateMgr implements backend.Enhanced. @@ -614,7 +619,16 @@ func (b *Cloud) StateMgr(name string) (statemgr.Full, error) { } } - return &State{tfeClient: b.client, organization: b.organization, workspace: workspace}, nil + client := &remoteClient{ + client: b.client, + organization: b.organization, + workspace: workspace, + + // This is optionally set during Terraform Enterprise runs. + runID: os.Getenv("TFE_RUN_ID"), + } + + return NewState(client), nil } // Operation implements backend.Enhanced. diff --git a/internal/cloud/backend_state_test.go b/internal/cloud/backend_state_test.go index 223ddb9153fa..3b9833c38e5e 100644 --- a/internal/cloud/backend_state_test.go +++ b/internal/cloud/backend_state_test.go @@ -2,68 +2,69 @@ package cloud import ( "bytes" - "io/ioutil" "os" "testing" + tfe "github.com/hashicorp/go-tfe" + "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/states/statefile" ) -func TestCloudState_impl(t *testing.T) { +func TestRemoteClient_impl(t *testing.T) { var _ remote.Client = new(remoteClient) } -func TestCloudState(t *testing.T) { - state := testCloudState(t) - TestState(t, state) +func TestRemoteClient(t *testing.T) { + client := testRemoteClient(t) + remote.TestClient(t, client) } -//func TestRemoteClient_stateVersionCreated(t *testing.T) { -// b, bCleanup := testBackendWithName(t) -// defer bCleanup() -// -// raw, err := b.StateMgr(testBackendSingleWorkspaceName) -// if err != nil { -// t.Fatalf("error: %v", err) -// } -// -// state := raw.(*State) -// -// err = state.WriteState(([]byte)(` -//{ -// "version": 4, -// "terraform_version": "1.3.0", -// "serial": 1, -// "lineage": "backend-change", -// "outputs": { -// "foo": { -// "type": "string", -// "value": "bar" -// } -// } -//}`)) -// if err != nil { -// t.Fatalf("expected no error, got %v", err) -// } -// -// stateVersionsAPI := b.client.StateVersions.(*MockStateVersions) -// if got, want := len(stateVersionsAPI.stateVersions), 1; got != want { -// t.Fatalf("wrong number of state versions in the mock client %d; want %d", got, want) -// } -// -// var stateVersion *tfe.StateVersion -// for _, sv := range stateVersionsAPI.stateVersions { -// stateVersion = sv -// } -// -// if stateVersionsAPI.outputStates[stateVersion.ID] == nil || len(stateVersionsAPI.outputStates[stateVersion.ID]) == 0 { -// t.Fatal("no state version outputs in the mock client") -// } -//} - -func TestCLoudState_TestRemoteLocks(t *testing.T) { +func TestRemoteClient_stateVersionCreated(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + raw, err := b.StateMgr(testBackendSingleWorkspaceName) + if err != nil { + t.Fatalf("error: %v", err) + } + + client := raw.(*State).Client + + err = client.Put(([]byte)(` +{ + "version": 4, + "terraform_version": "1.3.0", + "serial": 1, + "lineage": "backend-change", + "outputs": { + "foo": { + "type": "string", + "value": "bar" + } + } +}`)) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + stateVersionsAPI := b.client.StateVersions.(*MockStateVersions) + if got, want := len(stateVersionsAPI.stateVersions), 1; got != want { + t.Fatalf("wrong number of state versions in the mock client %d; want %d", got, want) + } + + var stateVersion *tfe.StateVersion + for _, sv := range stateVersionsAPI.stateVersions { + stateVersion = sv + } + + if stateVersionsAPI.outputStates[stateVersion.ID] == nil || len(stateVersionsAPI.outputStates[stateVersion.ID]) == 0 { + t.Fatal("no state version outputs in the mock client") + } +} + +func TestRemoteClient_TestRemoteLocks(t *testing.T) { b, bCleanup := testBackendWithName(t) defer bCleanup() @@ -77,30 +78,26 @@ func TestCLoudState_TestRemoteLocks(t *testing.T) { t.Fatalf("expected no error, got %v", err) } - TestCloudLocks(t, s1, s2) + remote.TestRemoteLocks(t, s1.(*State).Client, s2.(*State).Client) } -func TestCloudState_withRunID(t *testing.T) { +func TestRemoteClient_withRunID(t *testing.T) { // Set the TFE_RUN_ID environment variable before creating the client! if err := os.Setenv("TFE_RUN_ID", GenerateID("run-")); err != nil { t.Fatalf("error setting env var TFE_RUN_ID: %v", err) } // Create a new test client. - state := testCloudState(t) + client := testRemoteClient(t) // Create a new empty state. sf := statefile.New(states.NewState(), "", 0) var buf bytes.Buffer statefile.Write(sf, &buf) - jsonState, err := ioutil.ReadFile("../command/testdata/show-json-state/sensitive-variables/output.json") - - if err != nil { - t.Fatal(err) - } - - if err := state.uploadState(state.lineage, state.serial, state.forcePush, buf.Bytes(), jsonState); err != nil { - t.Fatalf("put: %s", err) + // Store the new state to verify (this will be done + // by the mock that is used) that the run ID is set. + if err := client.Put(buf.Bytes()); err != nil { + t.Fatalf("expected no error, got %v", err) } } diff --git a/internal/cloud/state.go b/internal/cloud/state.go index 44f5faa1e1bf..fac19add6f8a 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -1,59 +1,30 @@ package cloud import ( - "bytes" "context" - "crypto/md5" - "encoding/base64" "encoding/json" "errors" "fmt" "log" - "os" "strings" - "sync" - tfe "github.com/hashicorp/go-tfe" + "github.com/hashicorp/go-tfe" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" - uuid "github.com/hashicorp/go-uuid" - "github.com/hashicorp/terraform/internal/command/jsonstate" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" - "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terraform" ) -// State implements the State interfaces in the state package to handle -// reading and writing the remote state to TFC. This State on its own does no -// local caching so every persist will go to the remote storage and local -// writes will go to memory. +// State is similar to remote State and delegates to it, except in the case of output values, +// which use a separate methodology that ensures the caller is authorized to read cloud +// workspace outputs. type State struct { - mu sync.Mutex + Client *remoteClient - // Client Client - - // We track two pieces of meta data in addition to the state itself: - // - // lineage - the state's unique ID - // serial - the monotonic counter of "versions" of the state - // - // Both of these (along with state) have a sister field - // that represents the values read in from an existing source. - // All three of these values are used to determine if the new - // state has changed from an existing state we read in. - lineage, readLineage string - serial, readSerial uint64 - state, readState *states.State - disableLocks bool - tfeClient *tfe.Client - organization string - workspace *tfe.Workspace - stateUploadErr bool - forcePush bool - lockInfo *statemgr.LockInfo + delegate remote.State } var ErrStateVersionUnauthorizedUpgradeState = errors.New(strings.TrimSpace(` @@ -63,371 +34,70 @@ of authorization and therefore this error can usually be fixed by upgrading the remote state version. `)) -var _ statemgr.Full = (*State)(nil) -var _ statemgr.Migrator = (*State)(nil) - -// statemgr.Reader impl. -func (s *State) State() *states.State { - s.mu.Lock() - defer s.mu.Unlock() - - return s.state.DeepCopy() -} - -// StateForMigration is part of our implementation of statemgr.Migrator. -func (s *State) StateForMigration() *statefile.File { - s.mu.Lock() - defer s.mu.Unlock() - - return statefile.New(s.state.DeepCopy(), s.lineage, s.serial) -} - -// WriteStateForMigration is part of our implementation of statemgr.Migrator. -func (s *State) WriteStateForMigration(f *statefile.File, force bool) error { - s.mu.Lock() - defer s.mu.Unlock() - - if !force { - checkFile := statefile.New(s.state, s.lineage, s.serial) - if err := statemgr.CheckValidImport(f, checkFile); err != nil { - return err - } - } - - // The remote backend needs to pass the `force` flag through to its client. - // For backends that support such operations, inform the client - // that a force push has been requested - if force { - s.EnableForcePush() - } - - // We create a deep copy of the state here, because the caller also has - // a reference to the given object and can potentially go on to mutate - // it after we return, but we want the snapshot at this point in time. - s.state = f.State.DeepCopy() - s.lineage = f.Lineage - s.serial = f.Serial - - return nil -} - -// DisableLocks turns the Lock and Unlock methods into no-ops. This is intended -// to be called during initialization of a state manager and should not be -// called after any of the statemgr.Full interface methods have been called. -func (s *State) DisableLocks() { - s.disableLocks = true -} +// Proof that cloud State is a statemgr.Persistent interface +var _ statemgr.Persistent = (*State)(nil) -// StateSnapshotMeta returns the metadata from the most recently persisted -// or refreshed persistent state snapshot. -// -// This is an implementation of statemgr.PersistentMeta. -func (s *State) StateSnapshotMeta() statemgr.SnapshotMeta { - return statemgr.SnapshotMeta{ - Lineage: s.lineage, - Serial: s.serial, +func NewState(client *remoteClient) *State { + return &State{ + Client: client, + delegate: remote.State{Client: client}, } } -// statemgr.Writer impl. -func (s *State) WriteState(state *states.State) error { - s.mu.Lock() - defer s.mu.Unlock() - - // We create a deep copy of the state here, because the caller also has - // a reference to the given object and can potentially go on to mutate - // it after we return, but we want the snapshot at this point in time. - s.state = state.DeepCopy() - - return nil -} - -// statemgr.Persister impl. -func (s *State) PersistState(schemas *terraform.Schemas) error { - s.mu.Lock() - defer s.mu.Unlock() - - if s.readState != nil { - lineageUnchanged := s.readLineage != "" && s.lineage == s.readLineage - serialUnchanged := s.readSerial != 0 && s.serial == s.readSerial - stateUnchanged := statefile.StatesMarshalEqual(s.state, s.readState) - if stateUnchanged && lineageUnchanged && serialUnchanged { - // If the state, lineage or serial haven't changed at all then we have nothing to do. - return nil - } - s.serial++ - } else { - // 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() - if err != nil { - return fmt.Errorf("failed checking for existing remote state: %s", err) - } - if s.lineage == "" { // indicates that no state snapshot is present yet - lineage, err := uuid.GenerateUUID() - if err != nil { - return fmt.Errorf("failed to generate initial lineage: %v", err) - } - s.lineage = lineage - s.serial = 0 - } - } - - f := statefile.New(s.state, s.lineage, s.serial) - - var buf bytes.Buffer - err := statefile.Write(f, &buf) - if err != nil { - return err - } - - var jsonState []byte - if schemas != nil { - jsonState, err = jsonstate.Marshal(f, schemas) - if err != nil { - return err - } - } - - err = s.uploadState(s.lineage, s.serial, s.forcePush, buf.Bytes(), jsonState) - if err != nil { - s.stateUploadErr = true - return fmt.Errorf("error uploading state: %w", err) - } - // After we've successfully persisted, what we just wrote is our new - // reference state until someone calls RefreshState again. - // We've potentially overwritten (via force) the state, lineage - // and / or serial (and serial was incremented) so we copy over all - // three fields so everything matches the new state and a subsequent - // operation would correctly detect no changes to the lineage, serial or state. - s.readState = s.state.DeepCopy() - s.readLineage = s.lineage - s.readSerial = s.serial - return nil -} - -func (s *State) uploadState(lineage string, serial uint64, isForcePush bool, state, jsonState []byte) error { - ctx := context.Background() - - // Read the raw state into a Terraform state. - stateFile, err := statefile.Read(bytes.NewReader(state)) - if err != nil { - return fmt.Errorf("failed to read state: %w", err) - } - - ov, err := jsonstate.MarshalOutputs(stateFile.State.RootModule().OutputValues) - if err != nil { - return fmt.Errorf("failed to translate outputs: %w", err) - } - o, err := json.Marshal(ov) - if err != nil { - return fmt.Errorf("failed to marshal outputs to json: %w", err) - } - - options := tfe.StateVersionCreateOptions{ - Lineage: tfe.String(lineage), - Serial: tfe.Int64(int64(serial)), - MD5: tfe.String(fmt.Sprintf("%x", md5.Sum(state))), - State: tfe.String(base64.StdEncoding.EncodeToString(state)), - Force: tfe.Bool(isForcePush), - JSONState: tfe.String(base64.StdEncoding.EncodeToString(jsonState)), - JSONStateOutputs: tfe.String(base64.StdEncoding.EncodeToString(o)), - } - - // If we have a run ID, make sure to add it to the options - // so the state will be properly associated with the run. - runID := os.Getenv("TFE_RUN_ID") - if runID != "" { - options.Run = &tfe.Run{ID: runID} - } - // Create the new state. - _, err = s.tfeClient.StateVersions.Create(ctx, s.workspace.ID, options) - return err +// State delegates calls to read State to the remote State +func (s *State) State() *states.State { + return s.delegate.State() } -// Lock calls the Client's Lock method if it's implemented. +// Lock delegates calls to lock state to the remote State func (s *State) Lock(info *statemgr.LockInfo) (string, error) { - s.mu.Lock() - defer s.mu.Unlock() - - if s.disableLocks { - return "", nil - } - ctx := context.Background() - - lockErr := &statemgr.LockError{Info: s.lockInfo} - - // Lock the workspace. - _, err := s.tfeClient.Workspaces.Lock(ctx, s.workspace.ID, tfe.WorkspaceLockOptions{ - Reason: tfe.String("Locked by Terraform"), - }) - if err != nil { - if err == tfe.ErrWorkspaceLocked { - lockErr.Info = info - err = fmt.Errorf("%s (lock ID: \"%s/%s\")", err, s.organization, s.workspace.Name) - } - lockErr.Err = err - return "", lockErr - } - - s.lockInfo = info + return s.delegate.Lock(info) +} - return s.lockInfo.ID, nil +// Unlock delegates calls to unlock state to the remote State +func (s *State) Unlock(id string) error { + return s.delegate.Unlock(id) } -// statemgr.Refresher impl. +// RefreshState delegates calls to refresh State to the remote State func (s *State) RefreshState() error { - s.mu.Lock() - defer s.mu.Unlock() - return s.refreshState() + return s.delegate.RefreshState() } -// 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 { - payload, err := s.getStatePayload() - if err != nil { - return err - } - - // no remote state is OK - if payload == nil { - s.readState = nil - s.lineage = "" - s.serial = 0 - return nil - } - - stateFile, err := statefile.Read(bytes.NewReader(payload.Data)) - if err != nil { - return err - } - - s.lineage = stateFile.Lineage - s.serial = stateFile.Serial - s.state = stateFile.State - - // Properties from the remote must be separate so we can - // track changes as lineage, serial and/or state are mutated - s.readLineage = stateFile.Lineage - s.readSerial = stateFile.Serial - s.readState = s.state.DeepCopy() +// RefreshState delegates calls to refresh State to the remote State +func (s *State) PersistState(schemas *terraform.Schemas) error { + s.delegate.PersistState(schemas) return nil } -func (s *State) getStatePayload() (*remote.Payload, error) { - ctx := context.Background() - - sv, err := s.tfeClient.StateVersions.ReadCurrent(ctx, s.workspace.ID) - if err != nil { - if err == tfe.ErrResourceNotFound { - // If no state exists, then return nil. - return nil, nil - } - return nil, fmt.Errorf("error retrieving state: %v", err) - } - - state, err := s.tfeClient.StateVersions.Download(ctx, sv.DownloadURL) - if err != nil { - return nil, fmt.Errorf("error downloading state: %v", err) - } - - // If the state is empty, then return nil. - if len(state) == 0 { - return nil, nil - } - - // Get the MD5 checksum of the state. - sum := md5.Sum(state) - - return &remote.Payload{ - Data: state, - MD5: sum[:], - }, nil +// WriteState delegates calls to write State to the remote State +func (s *State) WriteState(state *states.State) error { + return s.delegate.WriteState(state) } -// Unlock calls the Client's Unlock method if it's implemented. -func (s *State) Unlock(id string) error { - s.mu.Lock() - defer s.mu.Unlock() - - if s.disableLocks { - return nil - } - - ctx := context.Background() - - // We first check if there was an error while uploading the latest - // state. If so, we will not unlock the workspace to prevent any - // changes from being applied until the correct state is uploaded. - if s.stateUploadErr { - return nil - } - - lockErr := &statemgr.LockError{Info: s.lockInfo} - - // With lock info this should be treated as a normal unlock. - if s.lockInfo != nil { - // Verify the expected lock ID. - if s.lockInfo.ID != id { - lockErr.Err = fmt.Errorf("lock ID does not match existing lock") - return lockErr - } +func (s *State) fallbackReadOutputsFromFullState() (map[string]*states.OutputValue, error) { + log.Printf("[DEBUG] falling back to reading full state") - // Unlock the workspace. - _, err := s.tfeClient.Workspaces.Unlock(ctx, s.workspace.ID) - if err != nil { - lockErr.Err = err - return lockErr - } - - return nil + if err := s.RefreshState(); err != nil { + return nil, fmt.Errorf("failed to load state: %w", err) } - // Verify the optional force-unlock lock ID. - if s.organization+"/"+s.workspace.Name != id { - lockErr.Err = fmt.Errorf( - "lock ID %q does not match existing lock ID \"%s/%s\"", - id, - s.organization, - s.workspace.Name, - ) - return lockErr + state := s.State() + if state == nil { + // We know that there is supposed to be state (and this is not simply a new workspace + // without state) because the fallback is only invoked when outputs are present but + // detailed types are not available. + return nil, ErrStateVersionUnauthorizedUpgradeState } - // Force unlock the workspace. - _, err := s.tfeClient.Workspaces.ForceUnlock(ctx, s.workspace.ID) - if err != nil { - lockErr.Err = err - return lockErr - } - - return nil -} - -// Delete the remote state. -func (s *State) Delete() error { - err := s.tfeClient.Workspaces.Delete(context.Background(), s.organization, s.workspace.Name) - if err != nil && err != tfe.ErrResourceNotFound { - return fmt.Errorf("error deleting workspace %s: %v", s.workspace.Name, err) - } - - return nil -} - -// EnableForcePush to allow the remote client to overwrite state -// by implementing remote.ClientForcePusher -func (s *State) EnableForcePush() { - s.forcePush = true + return state.RootModule().OutputValues, nil } // GetRootOutputValues fetches output values from Terraform Cloud func (s *State) GetRootOutputValues() (map[string]*states.OutputValue, error) { ctx := context.Background() - so, err := s.tfeClient.StateVersionOutputs.ReadCurrent(ctx, s.workspace.ID) + so, err := s.Client.client.StateVersionOutputs.ReadCurrent(ctx, s.Client.workspace.ID) if err != nil { return nil, fmt.Errorf("could not read state version outputs: %w", err) @@ -441,27 +111,13 @@ func (s *State) GetRootOutputValues() (map[string]*states.OutputValue, error) { // with a version of terraform < 1.3.0. In this case, we'll eject completely from this // function and fall back to the old behavior of reading the entire state file, which // requires a higher level of authorization. - log.Printf("[DEBUG] falling back to reading full state") - - if err := s.RefreshState(); err != nil { - return nil, fmt.Errorf("failed to load state: %w", err) - } - - state := s.State() - if state == nil { - // We know that there is supposed to be state (and this is not simply a new workspace - // without state) because the fallback is only invoked when outputs are present but - // detailed types are not available. - return nil, ErrStateVersionUnauthorizedUpgradeState - } - - return state.RootModule().OutputValues, nil + return s.fallbackReadOutputsFromFullState() } if output.Sensitive { // Since this is a sensitive value, the output must be requested explicitly in order to // read its value, which is assumed to be present by callers - sensitiveOutput, err := s.tfeClient.StateVersionOutputs.Read(ctx, output.ID) + sensitiveOutput, err := s.Client.client.StateVersionOutputs.Read(ctx, output.ID) if err != nil { return nil, fmt.Errorf("could not read state version output %s: %w", output.ID, err) } diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index 72e8cf5ceebb..738ae721a44d 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -27,9 +27,14 @@ func TestState_GetRootOutputValues(t *testing.T) { b, bCleanup := testBackendWithOutputs(t) defer bCleanup() - state := &State{tfeClient: b.client, organization: b.organization, workspace: &tfe.Workspace{ - ID: "ws-abcd", - }} + client := &remoteClient{ + client: b.client, + workspace: &tfe.Workspace{ + ID: "ws-abcd", + }, + } + + state := NewState(client) outputs, err := state.GetRootOutputValues() if err != nil { diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index 95ac257c6d3c..cfb49cf9b352 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -1,14 +1,10 @@ package cloud import ( - "bytes" "context" "encoding/json" "fmt" - "github.com/hashicorp/terraform/internal/states/statefile" - "github.com/hashicorp/terraform/internal/states/statemgr" "io" - "io/ioutil" "net/http" "net/http/httptest" "path" @@ -27,6 +23,7 @@ import ( "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/httpclient" "github.com/hashicorp/terraform/internal/providers" + "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/hashicorp/terraform/version" @@ -113,7 +110,7 @@ func testBackendNoOperations(t *testing.T) (*Cloud, func()) { return testBackend(t, obj) } -func testCloudState(t *testing.T) *State { +func testRemoteClient(t *testing.T) remote.Client { b, bCleanup := testBackendWithName(t) defer bCleanup() @@ -122,7 +119,7 @@ func testCloudState(t *testing.T) *State { t.Fatalf("error: %v", err) } - return raw.(*State) + return raw.(*State).Client } func testBackendWithOutputs(t *testing.T) (*Cloud, func()) { @@ -446,95 +443,3 @@ func testVariables(s terraform.ValueSourceType, vs ...string) map[string]backend } return vars } - -func TestState(t *testing.T, state *State) { - var buf bytes.Buffer - s := statemgr.TestFullInitialState() - sf := statefile.New(s, "stub-lineage", 2) - err := statefile.Write(sf, &buf) - if err != nil { - t.Fatalf("err: %s", err) - } - data := buf.Bytes() - - jsonState, err := ioutil.ReadFile("../command/testdata/show-json-state/sensitive-variables/output.json") - - if err != nil { - t.Fatal(err) - } - - if err := state.uploadState(state.lineage, state.serial, state.forcePush, data, jsonState); err != nil { - t.Fatalf("put: %s", err) - } - - payload, err := state.getStatePayload() - if err != nil { - t.Fatalf("get: %s", err) - } - if !bytes.Equal(payload.Data, data) { - t.Fatalf("expected full state %q\n\ngot: %q", string(payload.Data), string(data)) - } - - if err := state.Delete(); err != nil { - t.Fatalf("delete: %s", err) - } - - p, err := state.getStatePayload() - if err != nil { - t.Fatalf("get: %s", err) - } - if p != nil { - t.Fatalf("expected empty state, got: %q", string(p.Data)) - } -} - -func TestCloudLocks(t *testing.T, a, b statemgr.Full) { - lockerA, ok := a.(statemgr.Locker) - if !ok { - t.Fatal("client A not a statemgr.Locker") - } - - lockerB, ok := b.(statemgr.Locker) - if !ok { - t.Fatal("client B not a statemgr.Locker") - } - - infoA := statemgr.NewLockInfo() - infoA.Operation = "test" - infoA.Who = "clientA" - - infoB := statemgr.NewLockInfo() - infoB.Operation = "test" - infoB.Who = "clientB" - - lockIDA, err := lockerA.Lock(infoA) - if err != nil { - t.Fatal("unable to get initial lock:", err) - } - - _, err = lockerB.Lock(infoB) - if err == nil { - lockerA.Unlock(lockIDA) - t.Fatal("client B obtained lock while held by client A") - } - if _, ok := err.(*statemgr.LockError); !ok { - t.Errorf("expected a LockError, but was %t: %s", err, err) - } - - if err := lockerA.Unlock(lockIDA); err != nil { - t.Fatal("error unlocking client A", err) - } - - lockIDB, err := lockerB.Lock(infoB) - if err != nil { - t.Fatal("unable to obtain lock from client B") - } - - if lockIDB == lockIDA { - t.Fatalf("duplicate lock IDs: %q", lockIDB) - } - - if err = lockerB.Unlock(lockIDB); err != nil { - t.Fatal("error unlocking client B:", err) - } -} diff --git a/internal/command/state_replace_provider_test.go b/internal/command/state_replace_provider_test.go index e86e5d669f33..397a895ab61b 100644 --- a/internal/command/state_replace_provider_test.go +++ b/internal/command/state_replace_provider_test.go @@ -3,6 +3,8 @@ package command import ( "bytes" "fmt" + "github.com/hashicorp/go-version" + "github.com/hashicorp/terraform/internal/states/statefile" "path/filepath" "regexp" "strings" @@ -64,6 +66,23 @@ func TestStateReplaceProvider(t *testing.T) { }) t.Run("Schemas not initialized and JSON output not generated", func(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath("init-cloud-simple"), td) + defer testChdir(t, td)() + + // Some of the tests need a non-empty placeholder state file to work + // with. + fakeStateFile := &statefile.File{ + Lineage: "boop", + Serial: 4, + TerraformVersion: version.Must(version.NewVersion("1.0.0")), + State: state, + } + var fakeStateBuf bytes.Buffer + err := statefile.WriteForTest(fakeStateFile, &fakeStateBuf) + if err != nil { + t.Error(err) + } statePath := testStateFile(t, state) ui := new(cli.MockUi) From 485a1f6777556c2b002726a4c7c5c34128f186e3 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 14:25:15 -0500 Subject: [PATCH 11/18] remove test for error --- .../command/state_replace_provider_test.go | 61 ------------------- 1 file changed, 61 deletions(-) diff --git a/internal/command/state_replace_provider_test.go b/internal/command/state_replace_provider_test.go index 397a895ab61b..9c86cf7797d1 100644 --- a/internal/command/state_replace_provider_test.go +++ b/internal/command/state_replace_provider_test.go @@ -2,11 +2,7 @@ package command import ( "bytes" - "fmt" - "github.com/hashicorp/go-version" - "github.com/hashicorp/terraform/internal/states/statefile" "path/filepath" - "regexp" "strings" "testing" @@ -65,63 +61,6 @@ func TestStateReplaceProvider(t *testing.T) { ) }) - t.Run("Schemas not initialized and JSON output not generated", func(t *testing.T) { - td := t.TempDir() - testCopyDir(t, testFixturePath("init-cloud-simple"), td) - defer testChdir(t, td)() - - // Some of the tests need a non-empty placeholder state file to work - // with. - fakeStateFile := &statefile.File{ - Lineage: "boop", - Serial: 4, - TerraformVersion: version.Must(version.NewVersion("1.0.0")), - State: state, - } - var fakeStateBuf bytes.Buffer - err := statefile.WriteForTest(fakeStateFile, &fakeStateBuf) - if err != nil { - t.Error(err) - } - statePath := testStateFile(t, state) - - ui := new(cli.MockUi) - view, _ := testView(t) - c := &StateReplaceProviderCommand{ - StateMeta{ - Meta: Meta{ - Ui: ui, - View: view, - }, - }, - } - - inputBuf := &bytes.Buffer{} - ui.InputReader = inputBuf - inputBuf.WriteString("yes\n") - - args := []string{ - "-state", statePath, - "hashicorp/aws", - "acmecorp/aws", - } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) - } - - // Check for the warning - actual := strings.TrimSpace(ui.ErrorWriter.String()) - expected := strings.TrimSpace(fmt.Sprintf(failedToLoadSchemasMessage, "")) - re, err := regexp.Compile(expected) - if err != nil { - t.Fatalf("Error compiling regexp: %s", err) - } - - if !re.MatchString(actual) { - t.Fatalf("wrong output\n expected: %s \n actual: %s", expected, actual) - } - }) - t.Run("happy path", func(t *testing.T) { statePath := testStateFile(t, state) From b504dd148981d1c79b3c208098af95c967cb3dca Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 14:29:07 -0500 Subject: [PATCH 12/18] update from code consistency checks --- internal/command/command_test.go | 3 ++- internal/command/meta.go | 5 +++-- internal/command/state_mv.go | 3 ++- internal/command/state_push.go | 5 +++-- internal/command/state_replace_provider.go | 3 ++- internal/command/state_rm.go | 3 ++- internal/command/taint.go | 3 ++- internal/command/untaint.go | 3 ++- 8 files changed, 18 insertions(+), 10 deletions(-) diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 43c4640822eb..fa498b438270 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -7,7 +7,6 @@ import ( "encoding/base64" "encoding/json" "fmt" - "github.com/google/go-cmp/cmp" "io" "io/ioutil" "net/http" @@ -20,6 +19,8 @@ import ( "syscall" "testing" + "github.com/google/go-cmp/cmp" + svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform/internal/addrs" diff --git a/internal/command/meta.go b/internal/command/meta.go index 6dc86c8a3d74..d3ebf2d9e623 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -6,8 +6,6 @@ import ( "errors" "flag" "fmt" - "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/states" "io/ioutil" "log" "os" @@ -16,6 +14,9 @@ import ( "strings" "time" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/states" + plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/terraform-svchost/disco" "github.com/mitchellh/cli" diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 8646974beaa2..5e0db0b6d6a3 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -2,9 +2,10 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/terraform" "strings" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/command/arguments" diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 8d275228c091..1a0e75b0a806 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -2,12 +2,13 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/tfdiags" "io" "os" "strings" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index d8d160462115..8c42754d8d35 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -2,9 +2,10 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/terraform" "strings" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index 6eb5f2164168..95ad180f400f 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -2,9 +2,10 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/terraform" "strings" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" diff --git a/internal/command/taint.go b/internal/command/taint.go index d5a257a77528..c3cd740aeda7 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -2,9 +2,10 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/terraform" "strings" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" diff --git a/internal/command/untaint.go b/internal/command/untaint.go index 140ecec3a108..55e7865ad9c3 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -2,9 +2,10 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/terraform" "strings" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" From dbf99f17b14eb33e9295d6185b91fc09cbb77ba8 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Mon, 29 Aug 2022 16:26:06 -0500 Subject: [PATCH 13/18] add test and removed backend state from cloud --- internal/cloud/backend.go | 24 +- internal/cloud/backend_state.go | 196 ------------ internal/cloud/backend_state_test.go | 103 ------- internal/cloud/state.go | 429 ++++++++++++++++++++++++--- internal/cloud/state_test.go | 70 ++++- internal/cloud/testing.go | 57 +++- 6 files changed, 507 insertions(+), 372 deletions(-) delete mode 100644 internal/cloud/backend_state.go delete mode 100644 internal/cloud/backend_state_test.go diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 669cc372270d..ded38f75f29f 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -526,15 +526,10 @@ func (b *Cloud) DeleteWorkspace(name string) error { } // Configure the remote workspace name. - client := &remoteClient{ - client: b.client, - organization: b.organization, - workspace: &tfe.Workspace{ - Name: name, - }, - } - - return client.Delete() + State := &State{tfeClient: b.client, organization: b.organization, workspace: &tfe.Workspace{ + Name: name, + }} + return State.Delete() } // StateMgr implements backend.Enhanced. @@ -619,16 +614,7 @@ func (b *Cloud) StateMgr(name string) (statemgr.Full, error) { } } - client := &remoteClient{ - client: b.client, - organization: b.organization, - workspace: workspace, - - // This is optionally set during Terraform Enterprise runs. - runID: os.Getenv("TFE_RUN_ID"), - } - - return NewState(client), nil + return &State{tfeClient: b.client, organization: b.organization, workspace: workspace}, nil } // Operation implements backend.Enhanced. diff --git a/internal/cloud/backend_state.go b/internal/cloud/backend_state.go deleted file mode 100644 index f8cb9f24551d..000000000000 --- a/internal/cloud/backend_state.go +++ /dev/null @@ -1,196 +0,0 @@ -package cloud - -import ( - "bytes" - "context" - "crypto/md5" - "encoding/base64" - "encoding/json" - "errors" - "fmt" - - tfe "github.com/hashicorp/go-tfe" - - "github.com/hashicorp/terraform/internal/command/jsonstate" - "github.com/hashicorp/terraform/internal/states/remote" - "github.com/hashicorp/terraform/internal/states/statefile" - "github.com/hashicorp/terraform/internal/states/statemgr" -) - -type remoteClient struct { - client *tfe.Client - lockInfo *statemgr.LockInfo - organization string - runID string - stateUploadErr bool - workspace *tfe.Workspace - forcePush bool -} - -// Get the remote state. -func (r *remoteClient) Get() (*remote.Payload, error) { - ctx := context.Background() - - sv, err := r.client.StateVersions.ReadCurrent(ctx, r.workspace.ID) - if err != nil { - if err == tfe.ErrResourceNotFound { - // If no state exists, then return nil. - return nil, nil - } - return nil, fmt.Errorf("failed to retrieve state: %w", err) - } - - state, err := r.client.StateVersions.Download(ctx, sv.DownloadURL) - if err != nil { - return nil, fmt.Errorf("failed to download state: %w", err) - } - - // If the state is empty, then return nil. - if len(state) == 0 { - return nil, nil - } - - // Get the MD5 checksum of the state. - sum := md5.Sum(state) - - return &remote.Payload{ - Data: state, - MD5: sum[:], - }, nil -} - -// Put the remote state. -func (r *remoteClient) Put(state []byte) error { - ctx := context.Background() - - // Read the raw state into a Terraform state. - stateFile, err := statefile.Read(bytes.NewReader(state)) - if err != nil { - return fmt.Errorf("failed to read state: %w", err) - } - - ov, err := jsonstate.MarshalOutputs(stateFile.State.RootModule().OutputValues) - if err != nil { - return fmt.Errorf("failed to translate outputs: %w", err) - } - o, err := json.Marshal(ov) - if err != nil { - return fmt.Errorf("failed to marshal outputs to json: %w", err) - } - - options := tfe.StateVersionCreateOptions{ - Lineage: tfe.String(stateFile.Lineage), - Serial: tfe.Int64(int64(stateFile.Serial)), - MD5: tfe.String(fmt.Sprintf("%x", md5.Sum(state))), - State: tfe.String(base64.StdEncoding.EncodeToString(state)), - Force: tfe.Bool(r.forcePush), - JSONStateOutputs: tfe.String(base64.StdEncoding.EncodeToString(o)), - } - - // If we have a run ID, make sure to add it to the options - // so the state will be properly associated with the run. - if r.runID != "" { - options.Run = &tfe.Run{ID: r.runID} - } - - // Create the new state. - _, err = r.client.StateVersions.Create(ctx, r.workspace.ID, options) - if err != nil { - r.stateUploadErr = true - return fmt.Errorf("failed to upload state: %w", err) - } - - return nil -} - -// Delete the remote state. -func (r *remoteClient) Delete() error { - err := r.client.Workspaces.Delete(context.Background(), r.organization, r.workspace.Name) - if err != nil && err != tfe.ErrResourceNotFound { - return fmt.Errorf("failed to delete workspace %s: %w", r.workspace.Name, err) - } - - return nil -} - -// EnableForcePush to allow the remote client to overwrite state -// by implementing remote.ClientForcePusher -func (r *remoteClient) EnableForcePush() { - r.forcePush = true -} - -// Lock the remote state. -func (r *remoteClient) Lock(info *statemgr.LockInfo) (string, error) { - ctx := context.Background() - - lockErr := &statemgr.LockError{Info: r.lockInfo} - - // Lock the workspace. - _, err := r.client.Workspaces.Lock(ctx, r.workspace.ID, tfe.WorkspaceLockOptions{ - Reason: tfe.String("Locked by Terraform"), - }) - if err != nil { - if err == tfe.ErrWorkspaceLocked { - lockErr.Info = info - err = fmt.Errorf("%s (lock ID: \"%s/%s\")", err, r.organization, r.workspace.Name) - } - lockErr.Err = err - return "", lockErr - } - - r.lockInfo = info - - return r.lockInfo.ID, nil -} - -// Unlock the remote state. -func (r *remoteClient) Unlock(id string) error { - ctx := context.Background() - - // We first check if there was an error while uploading the latest - // state. If so, we will not unlock the workspace to prevent any - // changes from being applied until the correct state is uploaded. - if r.stateUploadErr { - return nil - } - - lockErr := &statemgr.LockError{Info: r.lockInfo} - - // With lock info this should be treated as a normal unlock. - if r.lockInfo != nil { - // Verify the expected lock ID. - if r.lockInfo.ID != id { - lockErr.Err = errors.New("lock ID does not match existing lock") - return lockErr - } - - // Unlock the workspace. - _, err := r.client.Workspaces.Unlock(ctx, r.workspace.ID) - if err != nil { - lockErr.Err = err - return lockErr - } - - return nil - } - - // Verify the optional force-unlock lock ID. - if r.organization+"/"+r.workspace.Name != id { - lockErr.Err = fmt.Errorf( - "lock ID %q does not match existing lock ID \"%s/%s\"", - id, - r.organization, - r.workspace.Name, - ) - return lockErr - } - - // Force unlock the workspace. - _, err := r.client.Workspaces.ForceUnlock(ctx, r.workspace.ID) - if err != nil { - lockErr.Err = err - return lockErr - } - - return nil -} diff --git a/internal/cloud/backend_state_test.go b/internal/cloud/backend_state_test.go deleted file mode 100644 index 3b9833c38e5e..000000000000 --- a/internal/cloud/backend_state_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package cloud - -import ( - "bytes" - "os" - "testing" - - tfe "github.com/hashicorp/go-tfe" - - "github.com/hashicorp/terraform/internal/states" - "github.com/hashicorp/terraform/internal/states/remote" - "github.com/hashicorp/terraform/internal/states/statefile" -) - -func TestRemoteClient_impl(t *testing.T) { - var _ remote.Client = new(remoteClient) -} - -func TestRemoteClient(t *testing.T) { - client := testRemoteClient(t) - remote.TestClient(t, client) -} - -func TestRemoteClient_stateVersionCreated(t *testing.T) { - b, bCleanup := testBackendWithName(t) - defer bCleanup() - - raw, err := b.StateMgr(testBackendSingleWorkspaceName) - if err != nil { - t.Fatalf("error: %v", err) - } - - client := raw.(*State).Client - - err = client.Put(([]byte)(` -{ - "version": 4, - "terraform_version": "1.3.0", - "serial": 1, - "lineage": "backend-change", - "outputs": { - "foo": { - "type": "string", - "value": "bar" - } - } -}`)) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - stateVersionsAPI := b.client.StateVersions.(*MockStateVersions) - if got, want := len(stateVersionsAPI.stateVersions), 1; got != want { - t.Fatalf("wrong number of state versions in the mock client %d; want %d", got, want) - } - - var stateVersion *tfe.StateVersion - for _, sv := range stateVersionsAPI.stateVersions { - stateVersion = sv - } - - if stateVersionsAPI.outputStates[stateVersion.ID] == nil || len(stateVersionsAPI.outputStates[stateVersion.ID]) == 0 { - t.Fatal("no state version outputs in the mock client") - } -} - -func TestRemoteClient_TestRemoteLocks(t *testing.T) { - b, bCleanup := testBackendWithName(t) - defer bCleanup() - - s1, err := b.StateMgr(testBackendSingleWorkspaceName) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - s2, err := b.StateMgr(testBackendSingleWorkspaceName) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - remote.TestRemoteLocks(t, s1.(*State).Client, s2.(*State).Client) -} - -func TestRemoteClient_withRunID(t *testing.T) { - // Set the TFE_RUN_ID environment variable before creating the client! - if err := os.Setenv("TFE_RUN_ID", GenerateID("run-")); err != nil { - t.Fatalf("error setting env var TFE_RUN_ID: %v", err) - } - - // Create a new test client. - client := testRemoteClient(t) - - // Create a new empty state. - sf := statefile.New(states.NewState(), "", 0) - var buf bytes.Buffer - statefile.Write(sf, &buf) - - // Store the new state to verify (this will be done - // by the mock that is used) that the run ID is set. - if err := client.Put(buf.Bytes()); err != nil { - t.Fatalf("expected no error, got %v", err) - } -} diff --git a/internal/cloud/state.go b/internal/cloud/state.go index fac19add6f8a..f9e5c0c3af0a 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -1,30 +1,59 @@ package cloud import ( + "bytes" "context" + "crypto/md5" + "encoding/base64" "encoding/json" "errors" "fmt" "log" + "os" "strings" + "sync" - "github.com/hashicorp/go-tfe" + tfe "github.com/hashicorp/go-tfe" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" + uuid "github.com/hashicorp/go-uuid" + "github.com/hashicorp/terraform/internal/command/jsonstate" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" + "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terraform" ) -// State is similar to remote State and delegates to it, except in the case of output values, -// which use a separate methodology that ensures the caller is authorized to read cloud -// workspace outputs. +// State implements the State interfaces in the state package to handle +// reading and writing the remote state to TFC. This State on its own does no +// local caching so every persist will go to the remote storage and local +// writes will go to memory. type State struct { - Client *remoteClient + mu sync.Mutex - delegate remote.State + // Client Client + + // We track two pieces of meta data in addition to the state itself: + // + // lineage - the state's unique ID + // serial - the monotonic counter of "versions" of the state + // + // Both of these (along with state) have a sister field + // that represents the values read in from an existing source. + // All three of these values are used to determine if the new + // state has changed from an existing state we read in. + lineage, readLineage string + serial, readSerial uint64 + state, readState *states.State + disableLocks bool + tfeClient *tfe.Client + organization string + workspace *tfe.Workspace + stateUploadErr bool + forcePush bool + lockInfo *statemgr.LockInfo } var ErrStateVersionUnauthorizedUpgradeState = errors.New(strings.TrimSpace(` @@ -34,70 +63,370 @@ of authorization and therefore this error can usually be fixed by upgrading the remote state version. `)) -// Proof that cloud State is a statemgr.Persistent interface -var _ statemgr.Persistent = (*State)(nil) +var _ statemgr.Full = (*State)(nil) +var _ statemgr.Migrator = (*State)(nil) + +// statemgr.Reader impl. +func (s *State) State() *states.State { + s.mu.Lock() + defer s.mu.Unlock() + + return s.state.DeepCopy() +} + +// StateForMigration is part of our implementation of statemgr.Migrator. +func (s *State) StateForMigration() *statefile.File { + s.mu.Lock() + defer s.mu.Unlock() + + return statefile.New(s.state.DeepCopy(), s.lineage, s.serial) +} + +// WriteStateForMigration is part of our implementation of statemgr.Migrator. +func (s *State) WriteStateForMigration(f *statefile.File, force bool) error { + s.mu.Lock() + defer s.mu.Unlock() + + if !force { + checkFile := statefile.New(s.state, s.lineage, s.serial) + if err := statemgr.CheckValidImport(f, checkFile); err != nil { + return err + } + } -func NewState(client *remoteClient) *State { - return &State{ - Client: client, - delegate: remote.State{Client: client}, + // The remote backend needs to pass the `force` flag through to its client. + // For backends that support such operations, inform the client + // that a force push has been requested + if force { + s.EnableForcePush() } + + // We create a deep copy of the state here, because the caller also has + // a reference to the given object and can potentially go on to mutate + // it after we return, but we want the snapshot at this point in time. + s.state = f.State.DeepCopy() + s.lineage = f.Lineage + s.serial = f.Serial + + return nil } -// State delegates calls to read State to the remote State -func (s *State) State() *states.State { - return s.delegate.State() +// DisableLocks turns the Lock and Unlock methods into no-ops. This is intended +// to be called during initialization of a state manager and should not be +// called after any of the statemgr.Full interface methods have been called. +func (s *State) DisableLocks() { + s.disableLocks = true } -// Lock delegates calls to lock state to the remote State -func (s *State) Lock(info *statemgr.LockInfo) (string, error) { - return s.delegate.Lock(info) +// StateSnapshotMeta returns the metadata from the most recently persisted +// or refreshed persistent state snapshot. +// +// This is an implementation of statemgr.PersistentMeta. +func (s *State) StateSnapshotMeta() statemgr.SnapshotMeta { + return statemgr.SnapshotMeta{ + Lineage: s.lineage, + Serial: s.serial, + } } -// Unlock delegates calls to unlock state to the remote State -func (s *State) Unlock(id string) error { - return s.delegate.Unlock(id) +// statemgr.Writer impl. +func (s *State) WriteState(state *states.State) error { + s.mu.Lock() + defer s.mu.Unlock() + + // We create a deep copy of the state here, because the caller also has + // a reference to the given object and can potentially go on to mutate + // it after we return, but we want the snapshot at this point in time. + s.state = state.DeepCopy() + + return nil +} + +// statemgr.Persister impl. +func (s *State) PersistState(schemas *terraform.Schemas) error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.readState != nil { + lineageUnchanged := s.readLineage != "" && s.lineage == s.readLineage + serialUnchanged := s.readSerial != 0 && s.serial == s.readSerial + stateUnchanged := statefile.StatesMarshalEqual(s.state, s.readState) + if stateUnchanged && lineageUnchanged && serialUnchanged { + // If the state, lineage or serial haven't changed at all then we have nothing to do. + return nil + } + s.serial++ + } else { + // 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() + if err != nil { + return fmt.Errorf("failed checking for existing remote state: %s", err) + } + if s.lineage == "" { // indicates that no state snapshot is present yet + lineage, err := uuid.GenerateUUID() + if err != nil { + return fmt.Errorf("failed to generate initial lineage: %v", err) + } + s.lineage = lineage + s.serial = 0 + } + } + + f := statefile.New(s.state, s.lineage, s.serial) + + var buf bytes.Buffer + err := statefile.Write(f, &buf) + if err != nil { + return err + } + + var jsonState []byte + if schemas != nil { + jsonState, err = jsonstate.Marshal(f, schemas) + if err != nil { + return err + } + } + + stateFile, err := statefile.Read(bytes.NewReader(buf.Bytes())) + if err != nil { + return fmt.Errorf("failed to read state: %w", err) + } + + ov, err := jsonstate.MarshalOutputs(stateFile.State.RootModule().OutputValues) + if err != nil { + return fmt.Errorf("failed to translate outputs: %w", err) + } + jsonStateOutputs, err := json.Marshal(ov) + if err != nil { + return fmt.Errorf("failed to marshal outputs to json: %w", err) + } + + err = s.uploadState(s.lineage, s.serial, s.forcePush, buf.Bytes(), jsonState, jsonStateOutputs) + if err != nil { + s.stateUploadErr = true + return fmt.Errorf("error uploading state: %w", err) + } + // After we've successfully persisted, what we just wrote is our new + // reference state until someone calls RefreshState again. + // We've potentially overwritten (via force) the state, lineage + // and / or serial (and serial was incremented) so we copy over all + // three fields so everything matches the new state and a subsequent + // operation would correctly detect no changes to the lineage, serial or state. + s.readState = s.state.DeepCopy() + s.readLineage = s.lineage + s.readSerial = s.serial + return nil +} + +func (s *State) uploadState(lineage string, serial uint64, isForcePush bool, state, jsonState, jsonStateOutputs []byte) error { + ctx := context.Background() + + options := tfe.StateVersionCreateOptions{ + Lineage: tfe.String(lineage), + Serial: tfe.Int64(int64(serial)), + MD5: tfe.String(fmt.Sprintf("%x", md5.Sum(state))), + State: tfe.String(base64.StdEncoding.EncodeToString(state)), + Force: tfe.Bool(isForcePush), + JSONState: tfe.String(base64.StdEncoding.EncodeToString(jsonState)), + JSONStateOutputs: tfe.String(base64.StdEncoding.EncodeToString(jsonStateOutputs)), + } + + // If we have a run ID, make sure to add it to the options + // so the state will be properly associated with the run. + runID := os.Getenv("TFE_RUN_ID") + if runID != "" { + options.Run = &tfe.Run{ID: runID} + } + // Create the new state. + _, err := s.tfeClient.StateVersions.Create(ctx, s.workspace.ID, options) + return err +} + +// Lock calls the Client's Lock method if it's implemented. +func (s *State) Lock(info *statemgr.LockInfo) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if s.disableLocks { + return "", nil + } + ctx := context.Background() + + lockErr := &statemgr.LockError{Info: s.lockInfo} + + // Lock the workspace. + _, err := s.tfeClient.Workspaces.Lock(ctx, s.workspace.ID, tfe.WorkspaceLockOptions{ + Reason: tfe.String("Locked by Terraform"), + }) + if err != nil { + if err == tfe.ErrWorkspaceLocked { + lockErr.Info = info + err = fmt.Errorf("%s (lock ID: \"%s/%s\")", err, s.organization, s.workspace.Name) + } + lockErr.Err = err + return "", lockErr + } + + s.lockInfo = info + + return s.lockInfo.ID, nil } -// RefreshState delegates calls to refresh State to the remote State +// statemgr.Refresher impl. func (s *State) RefreshState() error { - return s.delegate.RefreshState() + s.mu.Lock() + defer s.mu.Unlock() + return s.refreshState() } -// RefreshState delegates calls to refresh State to the remote State -func (s *State) PersistState(schemas *terraform.Schemas) error { - s.delegate.PersistState(schemas) +// 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 { + payload, err := s.getStatePayload() + if err != nil { + return err + } + + // no remote state is OK + if payload == nil { + s.readState = nil + s.lineage = "" + s.serial = 0 + return nil + } + + stateFile, err := statefile.Read(bytes.NewReader(payload.Data)) + if err != nil { + return err + } + + s.lineage = stateFile.Lineage + s.serial = stateFile.Serial + s.state = stateFile.State + + // Properties from the remote must be separate so we can + // track changes as lineage, serial and/or state are mutated + s.readLineage = stateFile.Lineage + s.readSerial = stateFile.Serial + s.readState = s.state.DeepCopy() return nil } -// WriteState delegates calls to write State to the remote State -func (s *State) WriteState(state *states.State) error { - return s.delegate.WriteState(state) +func (s *State) getStatePayload() (*remote.Payload, error) { + ctx := context.Background() + + sv, err := s.tfeClient.StateVersions.ReadCurrent(ctx, s.workspace.ID) + if err != nil { + if err == tfe.ErrResourceNotFound { + // If no state exists, then return nil. + return nil, nil + } + return nil, fmt.Errorf("error retrieving state: %v", err) + } + + state, err := s.tfeClient.StateVersions.Download(ctx, sv.DownloadURL) + if err != nil { + return nil, fmt.Errorf("error downloading state: %v", err) + } + + // If the state is empty, then return nil. + if len(state) == 0 { + return nil, nil + } + + // Get the MD5 checksum of the state. + sum := md5.Sum(state) + + return &remote.Payload{ + Data: state, + MD5: sum[:], + }, nil } -func (s *State) fallbackReadOutputsFromFullState() (map[string]*states.OutputValue, error) { - log.Printf("[DEBUG] falling back to reading full state") +// Unlock calls the Client's Unlock method if it's implemented. +func (s *State) Unlock(id string) error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.disableLocks { + return nil + } + + ctx := context.Background() + + // We first check if there was an error while uploading the latest + // state. If so, we will not unlock the workspace to prevent any + // changes from being applied until the correct state is uploaded. + if s.stateUploadErr { + return nil + } + + lockErr := &statemgr.LockError{Info: s.lockInfo} + + // With lock info this should be treated as a normal unlock. + if s.lockInfo != nil { + // Verify the expected lock ID. + if s.lockInfo.ID != id { + lockErr.Err = fmt.Errorf("lock ID does not match existing lock") + return lockErr + } - if err := s.RefreshState(); err != nil { - return nil, fmt.Errorf("failed to load state: %w", err) + // Unlock the workspace. + _, err := s.tfeClient.Workspaces.Unlock(ctx, s.workspace.ID) + if err != nil { + lockErr.Err = err + return lockErr + } + + return nil } - state := s.State() - if state == nil { - // We know that there is supposed to be state (and this is not simply a new workspace - // without state) because the fallback is only invoked when outputs are present but - // detailed types are not available. - return nil, ErrStateVersionUnauthorizedUpgradeState + // Verify the optional force-unlock lock ID. + if s.organization+"/"+s.workspace.Name != id { + lockErr.Err = fmt.Errorf( + "lock ID %q does not match existing lock ID \"%s/%s\"", + id, + s.organization, + s.workspace.Name, + ) + return lockErr } - return state.RootModule().OutputValues, nil + // Force unlock the workspace. + _, err := s.tfeClient.Workspaces.ForceUnlock(ctx, s.workspace.ID) + if err != nil { + lockErr.Err = err + return lockErr + } + + return nil +} + +// Delete the remote state. +func (s *State) Delete() error { + err := s.tfeClient.Workspaces.Delete(context.Background(), s.organization, s.workspace.Name) + if err != nil && err != tfe.ErrResourceNotFound { + return fmt.Errorf("error deleting workspace %s: %v", s.workspace.Name, err) + } + + return nil +} + +// EnableForcePush to allow the remote client to overwrite state +// by implementing remote.ClientForcePusher +func (s *State) EnableForcePush() { + s.forcePush = true } // GetRootOutputValues fetches output values from Terraform Cloud func (s *State) GetRootOutputValues() (map[string]*states.OutputValue, error) { ctx := context.Background() - so, err := s.Client.client.StateVersionOutputs.ReadCurrent(ctx, s.Client.workspace.ID) + so, err := s.tfeClient.StateVersionOutputs.ReadCurrent(ctx, s.workspace.ID) if err != nil { return nil, fmt.Errorf("could not read state version outputs: %w", err) @@ -111,13 +440,27 @@ func (s *State) GetRootOutputValues() (map[string]*states.OutputValue, error) { // with a version of terraform < 1.3.0. In this case, we'll eject completely from this // function and fall back to the old behavior of reading the entire state file, which // requires a higher level of authorization. - return s.fallbackReadOutputsFromFullState() + log.Printf("[DEBUG] falling back to reading full state") + + if err := s.RefreshState(); err != nil { + return nil, fmt.Errorf("failed to load state: %w", err) + } + + state := s.State() + if state == nil { + // We know that there is supposed to be state (and this is not simply a new workspace + // without state) because the fallback is only invoked when outputs are present but + // detailed types are not available. + return nil, ErrStateVersionUnauthorizedUpgradeState + } + + return state.RootModule().OutputValues, nil } if output.Sensitive { // Since this is a sensitive value, the output must be requested explicitly in order to // read its value, which is assumed to be present by callers - sensitiveOutput, err := s.Client.client.StateVersionOutputs.Read(ctx, output.ID) + sensitiveOutput, err := s.tfeClient.StateVersionOutputs.Read(ctx, output.ID) if err != nil { return nil, fmt.Errorf("could not read state version output %s: %w", output.ID, err) } diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index 738ae721a44d..4d62975f09a6 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -1,6 +1,9 @@ package cloud import ( + "bytes" + "github.com/hashicorp/terraform/internal/states/statefile" + "io/ioutil" "testing" "github.com/hashicorp/go-tfe" @@ -27,14 +30,9 @@ func TestState_GetRootOutputValues(t *testing.T) { b, bCleanup := testBackendWithOutputs(t) defer bCleanup() - client := &remoteClient{ - client: b.client, - workspace: &tfe.Workspace{ - ID: "ws-abcd", - }, - } - - state := NewState(client) + state := &State{tfeClient: b.client, organization: b.organization, workspace: &tfe.Workspace{ + ID: "ws-abcd", + }} outputs, err := state.GetRootOutputValues() if err != nil { @@ -81,3 +79,59 @@ func TestState_GetRootOutputValues(t *testing.T) { } } } + +func TestState(t *testing.T) { + var buf bytes.Buffer + s := statemgr.TestFullInitialState() + sf := statefile.New(s, "stub-lineage", 2) + err := statefile.Write(sf, &buf) + if err != nil { + t.Fatalf("err: %s", err) + } + data := buf.Bytes() + + state := testCloudState(t) + + jsonState, err := ioutil.ReadFile("../command/testdata/show-json-state/sensitive-variables/output.json") + if err != nil { + t.Fatal(err) + } + + jsonStateOutputs := []byte(` +{ + "version": 4, + "terraform_version": "1.3.0", + "serial": 1, + "lineage": "backend-change", + "outputs": { + "foo": { + "type": "string", + "value": "bar" + } + } +}`) + + if err := state.uploadState(state.lineage, state.serial, state.forcePush, data, jsonState, jsonStateOutputs); err != nil { + t.Fatalf("put: %s", err) + } + + payload, err := state.getStatePayload() + if err != nil { + t.Fatalf("get: %s", err) + } + if !bytes.Equal(payload.Data, data) { + t.Fatalf("expected full state %q\n\ngot: %q", string(payload.Data), string(data)) + } + + if err := state.Delete(); err != nil { + t.Fatalf("delete: %s", err) + } + + p, err := state.getStatePayload() + if err != nil { + t.Fatalf("get: %s", err) + } + if p != nil { + t.Fatalf("expected empty state, got: %q", string(p.Data)) + } +} diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index cfb49cf9b352..ad6b98c90f00 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/hashicorp/terraform/internal/states/statemgr" "io" "net/http" "net/http/httptest" @@ -23,7 +24,6 @@ import ( "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/httpclient" "github.com/hashicorp/terraform/internal/providers" - "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/hashicorp/terraform/version" @@ -110,7 +110,7 @@ func testBackendNoOperations(t *testing.T) (*Cloud, func()) { return testBackend(t, obj) } -func testRemoteClient(t *testing.T) remote.Client { +func testCloudState(t *testing.T) *State { b, bCleanup := testBackendWithName(t) defer bCleanup() @@ -119,7 +119,7 @@ func testRemoteClient(t *testing.T) remote.Client { t.Fatalf("error: %v", err) } - return raw.(*State).Client + return raw.(*State) } func testBackendWithOutputs(t *testing.T) (*Cloud, func()) { @@ -443,3 +443,54 @@ func testVariables(s terraform.ValueSourceType, vs ...string) map[string]backend } return vars } + +func TestCloudLocks(t *testing.T, a, b statemgr.Full) { + lockerA, ok := a.(statemgr.Locker) + if !ok { + t.Fatal("client A not a statemgr.Locker") + } + + lockerB, ok := b.(statemgr.Locker) + if !ok { + t.Fatal("client B not a statemgr.Locker") + } + + infoA := statemgr.NewLockInfo() + infoA.Operation = "test" + infoA.Who = "clientA" + + infoB := statemgr.NewLockInfo() + infoB.Operation = "test" + infoB.Who = "clientB" + + lockIDA, err := lockerA.Lock(infoA) + if err != nil { + t.Fatal("unable to get initial lock:", err) + } + + _, err = lockerB.Lock(infoB) + if err == nil { + lockerA.Unlock(lockIDA) + t.Fatal("client B obtained lock while held by client A") + } + if _, ok := err.(*statemgr.LockError); !ok { + t.Errorf("expected a LockError, but was %t: %s", err, err) + } + + if err := lockerA.Unlock(lockIDA); err != nil { + t.Fatal("error unlocking client A", err) + } + + lockIDB, err := lockerB.Lock(infoB) + if err != nil { + t.Fatal("unable to obtain lock from client B") + } + + if lockIDB == lockIDA { + t.Fatalf("duplicate lock IDs: %q", lockIDB) + } + + if err = lockerB.Unlock(lockIDB); err != nil { + t.Fatal("error unlocking client B:", err) + } +} From 7e5b7b283eff5b1c775568a076045d2e4904ee18 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 30 Aug 2022 09:49:09 -0500 Subject: [PATCH 14/18] updates for code consistency --- internal/cloud/state_test.go | 3 ++- internal/cloud/testing.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index 4d62975f09a6..e004f8eeb0f7 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -2,10 +2,11 @@ package cloud import ( "bytes" - "github.com/hashicorp/terraform/internal/states/statefile" "io/ioutil" "testing" + "github.com/hashicorp/terraform/internal/states/statefile" + "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform/internal/states/statemgr" diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index ad6b98c90f00..68315dddb03f 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/hashicorp/terraform/internal/states/statemgr" "io" "net/http" "net/http/httptest" @@ -12,6 +11,8 @@ import ( "testing" "time" + "github.com/hashicorp/terraform/internal/states/statemgr" + tfe "github.com/hashicorp/go-tfe" svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform-svchost/auth" From de8bd5826f53113076e3576bb36458883d5c9f39 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 30 Aug 2022 17:01:44 -0500 Subject: [PATCH 15/18] first part of code review comments --- go.mod | 2 +- go.sum | 4 +- internal/cloud/state.go | 4 +- internal/cloud/state_test.go | 71 +++++++++++++++++++--- internal/cloud/testing.go | 53 ---------------- internal/command/helper.go | 18 ++++-- internal/command/import.go | 4 +- internal/command/meta.go | 13 ++-- internal/command/meta_backend_migrate.go | 7 ++- internal/command/show.go | 2 +- internal/command/state_mv.go | 4 +- internal/command/state_push.go | 4 +- internal/command/state_replace_provider.go | 4 +- internal/command/state_rm.go | 4 +- internal/command/taint.go | 4 +- internal/command/untaint.go | 4 +- 16 files changed, 107 insertions(+), 95 deletions(-) diff --git a/go.mod b/go.mod index d47eb0612cb3..e66e875b56ba 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ require ( github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-plugin v1.4.3 github.com/hashicorp/go-retryablehttp v0.7.1 - github.com/hashicorp/go-tfe v1.8.1-0.20220826155809-b28335218b42 + github.com/hashicorp/go-tfe v1.9.0 github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f diff --git a/go.sum b/go.sum index 073db9ca669b..3a2c9803f62e 100644 --- a/go.sum +++ b/go.sum @@ -375,8 +375,8 @@ github.com/hashicorp/go-slug v0.10.0/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu4 github.com/hashicorp/go-sockaddr v1.0.0 h1:GeH6tui99pF4NJgfnhp+L6+FfobzVW3Ah46sLo0ICXs= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4= -github.com/hashicorp/go-tfe v1.8.1-0.20220826155809-b28335218b42 h1:3Ejb02U7WAl9NBRYArj8Hyw0dvHEB5NO1AsX06wbDq4= -github.com/hashicorp/go-tfe v1.8.1-0.20220826155809-b28335218b42/go.mod h1:uSWi2sPw7tLrqNIiASid9j3SprbbkPSJ/2s3X0mMemg= +github.com/hashicorp/go-tfe v1.9.0 h1:jkmyo7WKNA7gZDegG5imndoC4sojWXhqMufO+KcHqrU= +github.com/hashicorp/go-tfe v1.9.0/go.mod h1:uSWi2sPw7tLrqNIiASid9j3SprbbkPSJ/2s3X0mMemg= github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= diff --git a/internal/cloud/state.go b/internal/cloud/state.go index f9e5c0c3af0a..471c062d095a 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -33,8 +33,6 @@ import ( type State struct { mu sync.Mutex - // Client Client - // We track two pieces of meta data in addition to the state itself: // // lineage - the state's unique ID @@ -142,7 +140,7 @@ func (s *State) WriteState(state *states.State) error { return nil } -// statemgr.Persister impl. +// PersistState uploads a snapshot of the latest state as a StateVersion to Terraform Cloud func (s *State) PersistState(schemas *terraform.Schemas) error { s.mu.Lock() defer s.mu.Unlock() diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index e004f8eeb0f7..ee819d339fcf 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -5,10 +5,8 @@ import ( "io/ioutil" "testing" + tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform/internal/states/statefile" - - "github.com/hashicorp/go-tfe" - "github.com/hashicorp/terraform/internal/states/statemgr" ) @@ -100,10 +98,6 @@ func TestState(t *testing.T) { jsonStateOutputs := []byte(` { - "version": 4, - "terraform_version": "1.3.0", - "serial": 1, - "lineage": "backend-change", "outputs": { "foo": { "type": "string", @@ -136,3 +130,66 @@ func TestState(t *testing.T) { t.Fatalf("expected empty state, got: %q", string(p.Data)) } } + +func TestCloudLocks(t *testing.T) { + back, bCleanup := testBackendWithName(t) + defer bCleanup() + + a, err := back.StateMgr(testBackendSingleWorkspaceName) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + b, err := back.StateMgr(testBackendSingleWorkspaceName) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + lockerA, ok := a.(statemgr.Locker) + if !ok { + t.Fatal("client A not a statemgr.Locker") + } + + lockerB, ok := b.(statemgr.Locker) + if !ok { + t.Fatal("client B not a statemgr.Locker") + } + + infoA := statemgr.NewLockInfo() + infoA.Operation = "test" + infoA.Who = "clientA" + + infoB := statemgr.NewLockInfo() + infoB.Operation = "test" + infoB.Who = "clientB" + + lockIDA, err := lockerA.Lock(infoA) + if err != nil { + t.Fatal("unable to get initial lock:", err) + } + + _, err = lockerB.Lock(infoB) + if err == nil { + lockerA.Unlock(lockIDA) + t.Fatal("client B obtained lock while held by client A") + } + if _, ok := err.(*statemgr.LockError); !ok { + t.Errorf("expected a LockError, but was %t: %s", err, err) + } + + if err := lockerA.Unlock(lockIDA); err != nil { + t.Fatal("error unlocking client A", err) + } + + lockIDB, err := lockerB.Lock(infoB) + if err != nil { + t.Fatal("unable to obtain lock from client B") + } + + if lockIDB == lockIDA { + t.Fatalf("duplicate lock IDs: %q", lockIDB) + } + + if err = lockerB.Unlock(lockIDB); err != nil { + t.Fatal("error unlocking client B:", err) + } +} diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index 68315dddb03f..6cfe898337d2 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -11,8 +11,6 @@ import ( "testing" "time" - "github.com/hashicorp/terraform/internal/states/statemgr" - tfe "github.com/hashicorp/go-tfe" svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform-svchost/auth" @@ -444,54 +442,3 @@ func testVariables(s terraform.ValueSourceType, vs ...string) map[string]backend } return vars } - -func TestCloudLocks(t *testing.T, a, b statemgr.Full) { - lockerA, ok := a.(statemgr.Locker) - if !ok { - t.Fatal("client A not a statemgr.Locker") - } - - lockerB, ok := b.(statemgr.Locker) - if !ok { - t.Fatal("client B not a statemgr.Locker") - } - - infoA := statemgr.NewLockInfo() - infoA.Operation = "test" - infoA.Who = "clientA" - - infoB := statemgr.NewLockInfo() - infoB.Operation = "test" - infoB.Who = "clientB" - - lockIDA, err := lockerA.Lock(infoA) - if err != nil { - t.Fatal("unable to get initial lock:", err) - } - - _, err = lockerB.Lock(infoB) - if err == nil { - lockerA.Unlock(lockIDA) - t.Fatal("client B obtained lock while held by client A") - } - if _, ok := err.(*statemgr.LockError); !ok { - t.Errorf("expected a LockError, but was %t: %s", err, err) - } - - if err := lockerA.Unlock(lockIDA); err != nil { - t.Fatal("error unlocking client A", err) - } - - lockIDB, err := lockerB.Lock(infoB) - if err != nil { - t.Fatal("unable to obtain lock from client B") - } - - if lockIDB == lockIDA { - t.Fatalf("duplicate lock IDs: %q", lockIDB) - } - - if err = lockerB.Unlock(lockIDB); err != nil { - t.Fatal("error unlocking client B:", err) - } -} diff --git a/internal/command/helper.go b/internal/command/helper.go index 2ef3dd5af89d..acb6c26af0df 100644 --- a/internal/command/helper.go +++ b/internal/command/helper.go @@ -6,13 +6,19 @@ import ( ) const failedToLoadSchemasMessage = ` -Terraform failed to load schemas, which will in turn affect its ability to generate the -external JSON state file. This will not have any adverse effects on Terraforms ability -to maintain state information, but may have adverse effects on any external integrations -relying on this format. The file should be created on the next successful "terraform apply" -however, historic state information may be missing if the affected integration relies on that +Warning: Failed to update data for external integrations -%s +Terraform was unable to generate a description of the updated +state for use with external integrations in Terraform Cloud. +Any integrations configured for this workspace which depend on +information from the state may not work correctly when using the +result of this action. + +This problem occurs when Terraform cannot read the schema for +one or more of the providers used in the state. The next successful +apply will correct the problem by re-generating the JSON description +of the state: + terraform apply ` func isCloudMode(b backend.Enhanced) bool { diff --git a/internal/command/import.go b/internal/command/import.go index 24a0928aa169..5a2f1b56f32f 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -251,9 +251,9 @@ func (c *ImportCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(newState, nil) + schemas, diags = c.MaybeGetSchemas(newState, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } diff --git a/internal/command/meta.go b/internal/command/meta.go index d3ebf2d9e623..4b51ddb5945b 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -14,9 +14,6 @@ import ( "strings" "time" - "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/states" - plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/terraform-svchost/disco" "github.com/mitchellh/cli" @@ -30,11 +27,13 @@ import ( "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/command/webbrowser" "github.com/hashicorp/terraform/internal/command/workdir" + "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/getproviders" legacy "github.com/hashicorp/terraform/internal/legacy/terraform" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/provisioners" + "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" @@ -783,8 +782,12 @@ func (m *Meta) checkRequiredVersion() tfdiags.Diagnostics { return nil } -// GetSchemas loads and returns the schemas -func (c *Meta) GetSchemas(state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { +// MaybeGetSchemas attempts to load and return the schemas +// If there is not enough information to return the schemas, +// it could potentially return nil without errors. It is the +// responsibility of the caller to handle the lack of schema +// information accordingly +func (c *Meta) MaybeGetSchemas(state *states.State, config *configs.Config) (*terraform.Schemas, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics path, err := os.Getwd() diff --git a/internal/command/meta_backend_migrate.go b/internal/command/meta_backend_migrate.go index 00d9c8ed8538..46161cd623a8 100644 --- a/internal/command/meta_backend_migrate.go +++ b/internal/command/meta_backend_migrate.go @@ -438,9 +438,10 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) } - // Fetching schemas during init might be more of a hassle than we want to attempt - // in the case that we're migrating to TFC backend, the initial JSON state won't - // be generated and stored. + // The backend is currently handled before providers are installed during init, + // so requiring schemas here could lead to a catch-22 where it requires some manual + // intervention to proceed far enough for provider installation. To avoid this, + // when migrating to TFC backend, the initial JSON varient of state won't be generated and stored. if err := destinationState.PersistState(nil); err != nil { return fmt.Errorf(strings.TrimSpace(errBackendStateCopy), opts.SourceType, opts.DestinationType, err) diff --git a/internal/command/show.go b/internal/command/show.go index 62e8dacafeaa..3e58a979a870 100644 --- a/internal/command/show.go +++ b/internal/command/show.go @@ -110,7 +110,7 @@ func (c *ShowCommand) show(path string) (*plans.Plan, *statefile.File, *configs. // Get schemas, if possible if config != nil || stateFile != nil { - schemas, diags = c.GetSchemas(stateFile.State, config) + schemas, diags = c.MaybeGetSchemas(stateFile.State, config) if diags.HasErrors() { return plan, stateFile, config, schemas, diags } diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 5e0db0b6d6a3..6f9db6d1020c 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -397,9 +397,9 @@ func (c *StateMvCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(stateTo, nil) + schemas, diags = c.MaybeGetSchemas(stateTo, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 1a0e75b0a806..5c6d87cf1848 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -134,9 +134,9 @@ func (c *StatePushCommand) Run(args []string) int { var schemas *terraform.Schemas if isCloudMode(b) { var diags tfdiags.Diagnostics - schemas, diags = c.GetSchemas(srcStateFile.State, nil) + schemas, diags = c.MaybeGetSchemas(srcStateFile.State, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 8c42754d8d35..9b9dd27d9b7f 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -172,9 +172,9 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state, nil) + schemas, diags = c.MaybeGetSchemas(state, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index 95ad180f400f..cd415246e60d 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -122,9 +122,9 @@ func (c *StateRmCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state, nil) + schemas, diags = c.MaybeGetSchemas(state, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } diff --git a/internal/command/taint.go b/internal/command/taint.go index c3cd740aeda7..bd6df2be33ba 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -130,9 +130,9 @@ func (c *TaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state, nil) + schemas, diags = c.MaybeGetSchemas(state, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } diff --git a/internal/command/untaint.go b/internal/command/untaint.go index 55e7865ad9c3..b0bde0ddafba 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -169,9 +169,9 @@ func (c *UntaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.GetSchemas(state, nil) + schemas, diags = c.MaybeGetSchemas(state, nil) if diags.HasErrors() { - c.Ui.Warn(fmt.Sprintf(failedToLoadSchemasMessage, err)) + c.Ui.Warn(failedToLoadSchemasMessage) } } From 5eaa4c45c0c62ab52ffd301432e23367ec67b3d7 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 30 Aug 2022 17:27:15 -0500 Subject: [PATCH 16/18] fix imports --- internal/cloud/state.go | 2 +- internal/command/state_mv.go | 3 +-- internal/command/state_push.go | 5 ++--- internal/command/state_replace_provider.go | 3 +-- internal/command/state_rm.go | 3 +-- internal/command/taint.go | 3 +-- internal/command/untaint.go | 3 +-- 7 files changed, 8 insertions(+), 14 deletions(-) diff --git a/internal/cloud/state.go b/internal/cloud/state.go index 471c062d095a..04d9f7773439 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -13,10 +13,10 @@ import ( "strings" "sync" - tfe "github.com/hashicorp/go-tfe" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" + tfe "github.com/hashicorp/go-tfe" uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/terraform/internal/command/jsonstate" "github.com/hashicorp/terraform/internal/states" diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 6f9db6d1020c..279c1a1eb8d0 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -4,14 +4,13 @@ import ( "fmt" "strings" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/mitchellh/cli" ) diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 5c6d87cf1848..7745e7e8739a 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -6,14 +6,13 @@ import ( "os" "strings" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/tfdiags" "github.com/mitchellh/cli" ) diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 9b9dd27d9b7f..da5dc82a2020 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -4,13 +4,12 @@ import ( "fmt" "strings" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/mitchellh/cli" ) diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index cd415246e60d..2f7cb21d9a87 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -4,12 +4,11 @@ import ( "fmt" "strings" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/mitchellh/cli" ) diff --git a/internal/command/taint.go b/internal/command/taint.go index bd6df2be33ba..c1e21e07bd2e 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -4,13 +4,12 @@ import ( "fmt" "strings" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" ) diff --git a/internal/command/untaint.go b/internal/command/untaint.go index b0bde0ddafba..21ffefe9b269 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -4,13 +4,12 @@ import ( "fmt" "strings" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" ) From 4d749e2813a8d39729bc42a0da8f4a4af7f3895c Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 30 Aug 2022 17:52:51 -0500 Subject: [PATCH 17/18] add warning to diags and show at the end of each command --- internal/command/import.go | 7 ++++--- internal/command/meta.go | 4 ++-- internal/command/state_mv.go | 7 ++++--- internal/command/state_push.go | 6 ++---- internal/command/state_replace_provider.go | 8 +++++--- internal/command/state_rm.go | 7 ++++--- internal/command/taint.go | 8 +++++--- internal/command/untaint.go | 8 +++++--- 8 files changed, 31 insertions(+), 24 deletions(-) diff --git a/internal/command/import.go b/internal/command/import.go index 5a2f1b56f32f..d2a9917a8519 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -251,9 +251,10 @@ func (c *ImportCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.MaybeGetSchemas(newState, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags = c.MaybeGetSchemas(newState, nil) + if schemaDiags.HasErrors() { + diags = diags.Append(schemaDiags) } } diff --git a/internal/command/meta.go b/internal/command/meta.go index 4b51ddb5945b..de5a92118888 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -792,14 +792,14 @@ func (c *Meta) MaybeGetSchemas(state *states.State, config *configs.Config) (*te path, err := os.Getwd() if err != nil { - diags.Append(err) + diags.Append(tfdiags.SimpleWarning(failedToLoadSchemasMessage)) return nil, diags } if config == nil { config, diags = c.loadConfig(path) if diags.HasErrors() { - diags.Append(diags) + diags.Append(tfdiags.SimpleWarning(failedToLoadSchemasMessage)) return nil, diags } } diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 279c1a1eb8d0..da316d32a688 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -396,9 +396,10 @@ func (c *StateMvCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.MaybeGetSchemas(stateTo, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags = c.MaybeGetSchemas(stateTo, nil) + if schemaDiags.HasErrors() { + diags = diags.Append(schemaDiags) } } diff --git a/internal/command/state_push.go b/internal/command/state_push.go index 7745e7e8739a..c738dc70e36a 100644 --- a/internal/command/state_push.go +++ b/internal/command/state_push.go @@ -131,12 +131,9 @@ func (c *StatePushCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas + var diags tfdiags.Diagnostics if isCloudMode(b) { - var diags tfdiags.Diagnostics schemas, diags = c.MaybeGetSchemas(srcStateFile.State, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) - } } if err := stateMgr.WriteState(srcStateFile.State); err != nil { @@ -148,6 +145,7 @@ func (c *StatePushCommand) Run(args []string) int { return 1 } + c.showDiagnostics(diags) return 0 } diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index da5dc82a2020..75d9f05ef482 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -171,9 +171,10 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.MaybeGetSchemas(state, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags = c.MaybeGetSchemas(state, nil) + if schemaDiags.HasErrors() { + diags = diags.Append(schemaDiags) } } @@ -187,6 +188,7 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { return 1 } + c.showDiagnostics(diags) c.Ui.Output(fmt.Sprintf("\nSuccessfully replaced provider for %d resources.", len(willReplace))) return 0 } diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index 2f7cb21d9a87..f49081f4cd4f 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -121,9 +121,10 @@ func (c *StateRmCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.MaybeGetSchemas(state, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags = c.MaybeGetSchemas(state, nil) + if schemaDiags.HasErrors() { + diags = diags.Append(schemaDiags) } } diff --git a/internal/command/taint.go b/internal/command/taint.go index c1e21e07bd2e..859fa299209b 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -129,9 +129,10 @@ func (c *TaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.MaybeGetSchemas(state, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags = c.MaybeGetSchemas(state, nil) + if schemaDiags.HasErrors() { + diags = diags.Append(schemaDiags) } } @@ -186,6 +187,7 @@ func (c *TaintCommand) Run(args []string) int { return 1 } + c.showDiagnostics(diags) c.Ui.Output(fmt.Sprintf("Resource instance %s has been marked as tainted.", addr)) return 0 } diff --git a/internal/command/untaint.go b/internal/command/untaint.go index 21ffefe9b269..1fd0e2eed727 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -168,9 +168,10 @@ func (c *UntaintCommand) Run(args []string) int { // Get schemas, if possible, before writing state var schemas *terraform.Schemas if isCloudMode(b) { - schemas, diags = c.MaybeGetSchemas(state, nil) - if diags.HasErrors() { - c.Ui.Warn(failedToLoadSchemasMessage) + var schemaDiags tfdiags.Diagnostics + schemas, schemaDiags = c.MaybeGetSchemas(state, nil) + if schemaDiags.HasErrors() { + diags = diags.Append(schemaDiags) } } @@ -186,6 +187,7 @@ func (c *UntaintCommand) Run(args []string) int { return 1 } + c.showDiagnostics(diags) c.Ui.Output(fmt.Sprintf("Resource instance %s has been successfully untainted.", addr)) return 0 } From 37b7e6ebceb21001d34d112957dbbcd7fedb9c65 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 30 Aug 2022 18:03:57 -0500 Subject: [PATCH 18/18] don't check diags for errors --- internal/command/import.go | 4 +--- internal/command/state_mv.go | 4 +--- internal/command/state_replace_provider.go | 4 +--- internal/command/state_rm.go | 4 +--- internal/command/taint.go | 4 +--- internal/command/untaint.go | 4 +--- 6 files changed, 6 insertions(+), 18 deletions(-) diff --git a/internal/command/import.go b/internal/command/import.go index d2a9917a8519..28984d51f65e 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -253,9 +253,7 @@ func (c *ImportCommand) Run(args []string) int { if isCloudMode(b) { var schemaDiags tfdiags.Diagnostics schemas, schemaDiags = c.MaybeGetSchemas(newState, nil) - if schemaDiags.HasErrors() { - diags = diags.Append(schemaDiags) - } + diags = diags.Append(schemaDiags) } // Persist the final state diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index da316d32a688..feb650ac886f 100644 --- a/internal/command/state_mv.go +++ b/internal/command/state_mv.go @@ -398,9 +398,7 @@ func (c *StateMvCommand) Run(args []string) int { if isCloudMode(b) { var schemaDiags tfdiags.Diagnostics schemas, schemaDiags = c.MaybeGetSchemas(stateTo, nil) - if schemaDiags.HasErrors() { - diags = diags.Append(schemaDiags) - } + diags = diags.Append(schemaDiags) } // Write the new state diff --git a/internal/command/state_replace_provider.go b/internal/command/state_replace_provider.go index 75d9f05ef482..42fdc6255907 100644 --- a/internal/command/state_replace_provider.go +++ b/internal/command/state_replace_provider.go @@ -173,9 +173,7 @@ func (c *StateReplaceProviderCommand) Run(args []string) int { if isCloudMode(b) { var schemaDiags tfdiags.Diagnostics schemas, schemaDiags = c.MaybeGetSchemas(state, nil) - if schemaDiags.HasErrors() { - diags = diags.Append(schemaDiags) - } + diags = diags.Append(schemaDiags) } // Write the updated state diff --git a/internal/command/state_rm.go b/internal/command/state_rm.go index f49081f4cd4f..77d4b1823b2a 100644 --- a/internal/command/state_rm.go +++ b/internal/command/state_rm.go @@ -123,9 +123,7 @@ func (c *StateRmCommand) Run(args []string) int { if isCloudMode(b) { var schemaDiags tfdiags.Diagnostics schemas, schemaDiags = c.MaybeGetSchemas(state, nil) - if schemaDiags.HasErrors() { - diags = diags.Append(schemaDiags) - } + diags = diags.Append(schemaDiags) } if err := stateMgr.WriteState(state); err != nil { diff --git a/internal/command/taint.go b/internal/command/taint.go index 859fa299209b..e4da31d9b2ef 100644 --- a/internal/command/taint.go +++ b/internal/command/taint.go @@ -131,9 +131,7 @@ func (c *TaintCommand) Run(args []string) int { if isCloudMode(b) { var schemaDiags tfdiags.Diagnostics schemas, schemaDiags = c.MaybeGetSchemas(state, nil) - if schemaDiags.HasErrors() { - diags = diags.Append(schemaDiags) - } + diags = diags.Append(schemaDiags) } ss := state.SyncWrapper() diff --git a/internal/command/untaint.go b/internal/command/untaint.go index 1fd0e2eed727..d02a794cfeab 100644 --- a/internal/command/untaint.go +++ b/internal/command/untaint.go @@ -170,9 +170,7 @@ func (c *UntaintCommand) Run(args []string) int { if isCloudMode(b) { var schemaDiags tfdiags.Diagnostics schemas, schemaDiags = c.MaybeGetSchemas(state, nil) - if schemaDiags.HasErrors() { - diags = diags.Append(schemaDiags) - } + diags = diags.Append(schemaDiags) } obj.Status = states.ObjectReady