From 2576544db8ebeb78bba41651734a0a941603ac8d Mon Sep 17 00:00:00 2001 From: Joshua Feierman Date: Wed, 1 Feb 2023 12:49:55 -0500 Subject: [PATCH] fix: remote state behavior This makes the behavior of remote state consistent with local state in regards to the initial serial number of the generated / pushed state. Previously remote state's initial push would have a serial number of 0, whereas local state had a serial of > 0. This causes issues with the logic around, for example, ensuring that a plan file cannot be applied if state is stale (see https://github.com/hashicorp/terraform/issues/30670 for example). --- internal/backend/local/backend_local.go | 3 +++ internal/states/remote/state.go | 8 +++++++- internal/states/remote/state_test.go | 20 ++++++++++++++------ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 312b31ee2c..2e0e3445e7 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -284,7 +284,10 @@ func (b *Local) localRunForPlanFile(op *backend.Operation, pf *planfile.Reader, )) return nil, snap, diags } + log.Printf("[DEBUG] backend/local: priorStateFile: %v", *priorStateFile) + if currentStateMeta != nil { + log.Printf("[DEBUG] backend/local: currentStateMeta: %v", *currentStateMeta) // If the caller sets this, we require that the stored prior state // has the same metadata, which is an extra safety check that nothing // has changed since the plan was created. (All of the "real-world" diff --git a/internal/states/remote/state.go b/internal/states/remote/state.go index 412adc3eb4..ebff37c1cf 100644 --- a/internal/states/remote/state.go +++ b/internal/states/remote/state.go @@ -3,6 +3,7 @@ package remote import ( "bytes" "fmt" + "log" "sync" uuid "github.com/hashicorp/go-uuid" @@ -158,6 +159,9 @@ func (s *State) PersistState(schemas *terraform.Schemas) error { s.mu.Lock() defer s.mu.Unlock() + log.Printf("[DEBUG] states/remote: state read serial is: %d; serial is: %d", s.readSerial, s.serial) + log.Printf("[DEBUG] states/remote: state read lineage is: %s; lineage is: %s", s.readLineage, s.lineage) + if s.readState != nil { lineageUnchanged := s.readLineage != "" && s.lineage == s.readLineage serialUnchanged := s.readSerial != 0 && s.serial == s.readSerial @@ -175,13 +179,15 @@ func (s *State) PersistState(schemas *terraform.Schemas) error { if err != nil { return fmt.Errorf("failed checking for existing remote state: %s", err) } + log.Printf("[DEBUG] states/remote: after refresh, state read serial is: %d; serial is: %d", s.readSerial, s.serial) + log.Printf("[DEBUG] states/remote: after refresh, state read lineage is: %s; lineage is: %s", s.readLineage, s.lineage) 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 + s.serial = 1 } } diff --git a/internal/states/remote/state_test.go b/internal/states/remote/state_test.go index a260ac31e3..57ec93377c 100644 --- a/internal/states/remote/state_test.go +++ b/internal/states/remote/state_test.go @@ -98,24 +98,26 @@ func TestStatePersist(t *testing.T) { Provider: tfaddr.Provider{Namespace: "local"}, }, ) - mgr.readState = nil return s, func() {} }, expectedRequests: []mockClientRequest{ + // Expect an initial refresh, which returns nothing since there is no remote state. { Method: "Get", Content: nil, }, + // Expect a second refresh, since the read state is nil { Method: "Get", Content: nil, }, + // Expect an initial push with values and a serial of 1 { Method: "Put", Content: map[string]interface{}{ "version": 4.0, // encoding/json decodes this as float64 by default "lineage": "mock-lineage", - "serial": 0.0, // encoding/json decodes this as float64 by default + "serial": 1.0, // encoding/json decodes this as float64 by default "terraform_version": version.Version, "outputs": map[string]interface{}{}, "resources": []interface{}{ @@ -140,6 +142,7 @@ func TestStatePersist(t *testing.T) { }, }, }, + // If lineage changes, expect the serial to increment { name: "change lineage", mutationFunc: func(mgr *State) (*states.State, func()) { @@ -156,7 +159,7 @@ func TestStatePersist(t *testing.T) { Content: map[string]interface{}{ "version": 4.0, // encoding/json decodes this as float64 by default "lineage": "some-new-lineage", - "serial": 1.0, // encoding/json decodes this as float64 by default + "serial": 2.0, // encoding/json decodes this as float64 by default "terraform_version": version.Version, "outputs": map[string]interface{}{}, "resources": []interface{}{}, @@ -166,6 +169,7 @@ func TestStatePersist(t *testing.T) { }, currentState: nil, }, + // If the remote serial is incremented, then we increment it once more. { name: "change serial", mutationFunc: func(mgr *State) (*states.State, func()) { @@ -181,7 +185,7 @@ func TestStatePersist(t *testing.T) { Content: map[string]interface{}{ "version": 4.0, // encoding/json decodes this as float64 by default "lineage": "mock-lineage", - "serial": 3.0, // encoding/json decodes this as float64 by default + "serial": 4.0, // encoding/json decodes this as float64 by default "terraform_version": version.Version, "outputs": map[string]interface{}{}, "resources": []interface{}{}, @@ -191,6 +195,7 @@ func TestStatePersist(t *testing.T) { }, currentState: nil, }, + // Adding an output should cause the serial to increment as well. { name: "add output to state", mutationFunc: func(mgr *State) (*states.State, func()) { @@ -204,7 +209,7 @@ func TestStatePersist(t *testing.T) { Content: map[string]interface{}{ "version": 4.0, // encoding/json decodes this as float64 by default "lineage": "mock-lineage", - "serial": 2.0, // encoding/json decodes this as float64 by default + "serial": 3.0, // encoding/json decodes this as float64 by default "terraform_version": version.Version, "outputs": map[string]interface{}{ "foo": map[string]interface{}{ @@ -219,6 +224,7 @@ func TestStatePersist(t *testing.T) { }, currentState: nil, }, + // ...as should changing an output { name: "mutate state bar -> baz", mutationFunc: func(mgr *State) (*states.State, func()) { @@ -232,7 +238,7 @@ func TestStatePersist(t *testing.T) { Content: map[string]interface{}{ "version": 4.0, // encoding/json decodes this as float64 by default "lineage": "mock-lineage", - "serial": 3.0, // encoding/json decodes this as float64 by default + "serial": 4.0, // encoding/json decodes this as float64 by default "terraform_version": version.Version, "outputs": map[string]interface{}{ "foo": map[string]interface{}{ @@ -255,6 +261,8 @@ func TestStatePersist(t *testing.T) { }, noRequest: true, }, + // If the remote state's serial is less (force push), then we + // increment it once from there. { name: "reset serial (force push style)", mutationFunc: func(mgr *State) (*states.State, func()) {