From 9fa648492a47cf46f30342bd5a8fbfde786ea6a9 Mon Sep 17 00:00:00 2001 From: Tobias Krischer Date: Thu, 6 Oct 2022 19:38:23 +0200 Subject: [PATCH 1/3] only allow state push with correct lock id --- pkg/lock/local/local.go | 13 ++++++ pkg/lock/locker.go | 1 + pkg/lock/postgres/postgres.go | 13 ++++++ pkg/lock/redis/redis.go | 26 +++++++++++ pkg/lock/util/locktest.go | 6 +++ pkg/server/handler.go | 20 +++++++- pkg/server/handler_test.go | 88 +++++++++++++++++++++++------------ 7 files changed, 136 insertions(+), 31 deletions(-) diff --git a/pkg/lock/local/local.go b/pkg/lock/local/local.go index 784681a..86b2851 100644 --- a/pkg/lock/local/local.go +++ b/pkg/lock/local/local.go @@ -1,6 +1,7 @@ package local import ( + "fmt" "sync" "github.com/nimbolus/terraform-backend/pkg/terraform" @@ -63,3 +64,15 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { return true, nil } + +func (l *Lock) GetLock(s *terraform.State) ([]byte, error) { + l.mutex.Lock() + defer l.mutex.Unlock() + + lock, ok := l.db[s.ID] + if !ok { + return nil, fmt.Errorf("no lock found for state %s", s.ID) + } + + return lock, nil +} diff --git a/pkg/lock/locker.go b/pkg/lock/locker.go index 4cc9224..dd43e8f 100644 --- a/pkg/lock/locker.go +++ b/pkg/lock/locker.go @@ -8,4 +8,5 @@ type Locker interface { GetName() string Lock(s *terraform.State) (ok bool, err error) Unlock(s *terraform.State) (ok bool, err error) + GetLock(s *terraform.State) ([]byte, error) } diff --git a/pkg/lock/postgres/postgres.go b/pkg/lock/postgres/postgres.go index d090b97..06e0fd2 100644 --- a/pkg/lock/postgres/postgres.go +++ b/pkg/lock/postgres/postgres.go @@ -130,3 +130,16 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { return true, nil } + +func (l *Lock) GetLock(s *terraform.State) ([]byte, error) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + var lock []byte + + if err := l.db.QueryRowContext(ctx, `SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&lock); err != nil { + return nil, err + } + + return lock, nil +} diff --git a/pkg/lock/redis/redis.go b/pkg/lock/redis/redis.go index 277ab8e..4ec3eab 100644 --- a/pkg/lock/redis/redis.go +++ b/pkg/lock/redis/redis.go @@ -134,6 +134,32 @@ func (r *Lock) Unlock(s *terraform.State) (unlocked bool, err error) { return true, nil } +func (r *Lock) GetLock(s *terraform.State) (lock []byte, err error) { + mutex := r.client.NewMutex(lockKey, redsync.WithExpiry(12*time.Hour), redsync.WithTries(1), redsync.WithGenValueFunc(func() (string, error) { + return uuid.New().String(), nil + })) + + // lock the global redis mutex + if err := mutex.Lock(); err != nil { + log.Errorf("failed to lock redsync mutex: %v", err) + + return nil, err + } + + defer func() { + // unlock the global redis mutex + if _, mutErr := mutex.Unlock(); mutErr != nil { + log.Errorf("failed to unlock redsync mutex: %v", mutErr) + + if err != nil { + err = multierr.Append(err, mutErr) + } + } + }() + + return r.getLock(s) +} + func (r *Lock) setLock(s *terraform.State) error { ctx := context.Background() diff --git a/pkg/lock/util/locktest.go b/pkg/lock/util/locktest.go index 0f5810d..91c2166 100644 --- a/pkg/lock/util/locktest.go +++ b/pkg/lock/util/locktest.go @@ -43,6 +43,12 @@ func LockTest(t *testing.T, l lock.Locker) { t.Error(err) } + if lock, err := l.GetLock(&s1); err != nil { + t.Error(err) + } else if string(lock) != string(s1.Lock) { + t.Errorf("lock is not equal: %s != %s", lock, s1.Lock) + } + if locked, err := l.Lock(&s1); err != nil || !locked { t.Error("should be able to lock twice from the same process") } diff --git a/pkg/server/handler.go b/pkg/server/handler.go index dce28b1..4e0322a 100644 --- a/pkg/server/handler.go +++ b/pkg/server/handler.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "net/http" + "strings" "github.com/gorilla/mux" log "github.com/sirupsen/logrus" @@ -63,7 +64,7 @@ func StateHandler(store storage.Storage, locker lock.Locker, kms kms.KMS) func(h case http.MethodGet: Get(w, state, store, kms) case http.MethodPost: - Post(w, state, body, store, kms) + Post(req, w, state, body, locker, store, kms) case http.MethodDelete: Delete(w, state, store) default: @@ -128,7 +129,22 @@ func Get(w http.ResponseWriter, state *terraform.State, store storage.Storage, k HTTPResponse(w, http.StatusOK, string(state.Data)) } -func Post(w http.ResponseWriter, state *terraform.State, body []byte, store storage.Storage, kms kms.KMS) { +func Post(r *http.Request, w http.ResponseWriter, state *terraform.State, body []byte, locker lock.Locker, store storage.Storage, kms kms.KMS) { + reqLockID := r.URL.Query().Get("ID") + + lockID, err := locker.GetLock(state) + if err != nil { + log.Warnf("failed to get lock for state with id %s: %v", state.ID, err) + HTTPResponse(w, http.StatusInternalServerError, "") + return + } + + if !strings.Contains(string(lockID), fmt.Sprintf(`"ID":"%s"`, reqLockID)) { + log.Warnf("attempting to write state with wrong lock %s (expected %s)", reqLockID, lockID) + HTTPResponse(w, http.StatusBadRequest, "") + return + } + log.Debugf("save state with id %s", state.ID) data, err := kms.Encrypt(body) diff --git a/pkg/server/handler_test.go b/pkg/server/handler_test.go index c25fe38..da0d3ea 100644 --- a/pkg/server/handler_test.go +++ b/pkg/server/handler_test.go @@ -15,36 +15,83 @@ import ( "github.com/gorilla/mux" "github.com/gruntwork-io/terratest/modules/terraform" + localkms "github.com/nimbolus/terraform-backend/pkg/kms/local" locallock "github.com/nimbolus/terraform-backend/pkg/lock/local" "github.com/nimbolus/terraform-backend/pkg/storage/filesystem" ) -var terraformBinary = flag.String("tf", "terraform", "terraform binary") - -func TestServerHandler(t *testing.T) { - s := httptest.NewServer(NewStateHandler()) - defer s.Close() - - address, err := url.JoinPath(s.URL, "/state/project1/example") +func NewStateHandler(t *testing.T) http.Handler { + store, err := filesystem.NewFileSystemStorage(filepath.Join("./handler_test", "storage")) if err != nil { t.Fatal(err) } - terraformOptions := terraform.WithDefaultRetryableErrors(t, &terraform.Options{ + locker := locallock.NewLock() + + key := "x8DiIkAKRQT7cF55NQLkAZk637W3bGVOUjGeMX5ZGXY=" + kms, _ := localkms.NewKMS(key) + + r := mux.NewRouter().StrictSlash(true) + r.HandleFunc("/state/{project}/{name}", StateHandler(store, locker, kms)) + + return r +} + +var terraformBinary = flag.String("tf", "terraform", "terraform binary") + +func terraformOptions(t *testing.T, addr string) *terraform.Options { + return terraform.WithDefaultRetryableErrors(t, &terraform.Options{ TerraformDir: "./handler_test", TerraformBinary: *terraformBinary, Vars: map[string]interface{}{}, Reconfigure: true, BackendConfig: map[string]interface{}{ - "address": address, - "lock_address": address, - "unlock_address": address, + "address": addr, + "lock_address": addr, + "unlock_address": addr, "username": "basic", "password": "some-random-secret", }, - Lock: true, + LockTimeout: "200ms", + Lock: true, }) +} + +func TestServerHandler_VerifyLockOnPush(t *testing.T) { + s := httptest.NewServer(NewStateHandler(t)) + defer s.Close() + + address, err := url.JoinPath(s.URL, "/state/project1/example") + if err != nil { + t.Fatal(err) + } + + simulateLock(t, address, true) + + for _, doLock := range []bool{true, false} { + terraformOptions := terraformOptions(t, address) + terraformOptions.Lock = doLock + + _, err = terraform.InitAndApplyE(t, terraformOptions) + if err == nil { + t.Fatal("expected error") + } + + simulateLock(t, address, false) + } +} + +func TestServerHandler(t *testing.T) { + s := httptest.NewServer(NewStateHandler(t)) + defer s.Close() + + address, err := url.JoinPath(s.URL, "/state/project1/example") + if err != nil { + t.Fatal(err) + } + + terraformOptions := terraformOptions(t, address) // Clean up resources with "terraform destroy" at the end of the test. defer terraform.Destroy(t, terraformOptions) @@ -91,20 +138,3 @@ func simulateLock(t *testing.T, address string, lock bool) { t.Fatal(err) } } - -func NewStateHandler() http.Handler { - store, err := filesystem.NewFileSystemStorage(filepath.Join("./handler_test", "storage")) - if err != nil { - panic(err) - } - - locker := locallock.NewLock() - - key := "x8DiIkAKRQT7cF55NQLkAZk637W3bGVOUjGeMX5ZGXY=" - kms, _ := localkms.NewKMS(key) - - r := mux.NewRouter().StrictSlash(true) - r.HandleFunc("/state/{project}/{name}", StateHandler(store, locker, kms)) - - return r -} From 16f078e722fa8476e419a1fd7cb3fd4ca5088645 Mon Sep 17 00:00:00 2001 From: Tobias Krischer Date: Thu, 6 Oct 2022 21:26:23 +0200 Subject: [PATCH 2/3] use LockInfo struct --- pkg/lock/local/local.go | 12 ++++----- pkg/lock/locker.go | 2 +- pkg/lock/postgres/postgres.go | 48 ++++++++++++++++++++++++++--------- pkg/lock/redis/redis.go | 34 +++++++++++++++++-------- pkg/lock/redis/redis_test.go | 6 ++--- pkg/lock/util/locktest.go | 25 +++++++++++++++--- pkg/server/handler.go | 24 ++++++++++++------ pkg/server/handler_test.go | 21 +++++++-------- pkg/terraform/terraform.go | 22 +++++++++++++++- 9 files changed, 139 insertions(+), 55 deletions(-) diff --git a/pkg/lock/local/local.go b/pkg/lock/local/local.go index 86b2851..d536bc8 100644 --- a/pkg/lock/local/local.go +++ b/pkg/lock/local/local.go @@ -11,12 +11,12 @@ const Name = "local" type Lock struct { mutex sync.Mutex - db map[string][]byte + db map[string]terraform.LockInfo } func NewLock() *Lock { return &Lock{ - db: make(map[string][]byte), + db: make(map[string]terraform.LockInfo), } } @@ -30,7 +30,7 @@ func (l *Lock) Lock(s *terraform.State) (bool, error) { lock, ok := l.db[s.ID] if ok { - if string(lock) == string(s.Lock) { + if lock.Equal(s.Lock) { // you already have the lock return true, nil } @@ -54,7 +54,7 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { return false, nil } - if string(lock) != string(s.Lock) { + if !lock.Equal(s.Lock) { s.Lock = lock return false, nil @@ -65,13 +65,13 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { return true, nil } -func (l *Lock) GetLock(s *terraform.State) ([]byte, error) { +func (l *Lock) GetLock(s *terraform.State) (terraform.LockInfo, error) { l.mutex.Lock() defer l.mutex.Unlock() lock, ok := l.db[s.ID] if !ok { - return nil, fmt.Errorf("no lock found for state %s", s.ID) + return terraform.LockInfo{}, fmt.Errorf("no lock found for state %s", s.ID) } return lock, nil diff --git a/pkg/lock/locker.go b/pkg/lock/locker.go index dd43e8f..18c444e 100644 --- a/pkg/lock/locker.go +++ b/pkg/lock/locker.go @@ -8,5 +8,5 @@ type Locker interface { GetName() string Lock(s *terraform.State) (ok bool, err error) Unlock(s *terraform.State) (ok bool, err error) - GetLock(s *terraform.State) ([]byte, error) + GetLock(s *terraform.State) (terraform.LockInfo, error) } diff --git a/pkg/lock/postgres/postgres.go b/pkg/lock/postgres/postgres.go index 06e0fd2..412e219 100644 --- a/pkg/lock/postgres/postgres.go +++ b/pkg/lock/postgres/postgres.go @@ -3,6 +3,7 @@ package postgres import ( "context" "database/sql" + "encoding/json" "fmt" "time" @@ -67,11 +68,16 @@ func (l *Lock) Lock(s *terraform.State) (bool, error) { defer tx.Rollback() - var lock []byte + var rawLock []byte - if err := tx.QueryRow(`SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&lock); err != nil { + if err := tx.QueryRow(`SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&rawLock); err != nil { if err == sql.ErrNoRows { - if _, err := tx.Exec(`INSERT INTO `+l.table+` (state_id, lock_data) VALUES ($1, $2)`, s.ID, s.Lock); err != nil { + lockBytes, err := json.Marshal(s.Lock) + if err != nil { + return false, err + } + + if _, err := tx.Exec(`INSERT INTO `+l.table+` (state_id, lock_data) VALUES ($1, $2)`, s.ID, lockBytes); err != nil { return false, err } @@ -85,7 +91,13 @@ func (l *Lock) Lock(s *terraform.State) (bool, error) { return false, err } - if string(lock) == string(s.Lock) { + var lock terraform.LockInfo + + if err := json.Unmarshal(rawLock, &lock); err != nil { + return false, err + } + + if lock.Equal(s.Lock) { // you already have the lock return true, nil } @@ -106,9 +118,9 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { defer tx.Rollback() - var lock []byte + var rawLock []byte - if err := tx.QueryRow(`SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&lock); err != nil { + if err := tx.QueryRow(`SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&rawLock); err != nil { if err == sql.ErrNoRows { return false, nil } @@ -116,11 +128,17 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { return false, err } - if string(lock) != string(s.Lock) { + var lock terraform.LockInfo + + if err := json.Unmarshal(rawLock, &lock); err != nil { + return false, err + } + + if !lock.Equal(s.Lock) { return false, nil } - if _, err := tx.Exec(`DELETE FROM `+l.table+` WHERE state_id = $1 AND lock_data = $2`, s.ID, s.Lock); err != nil { + if _, err := tx.Exec(`DELETE FROM `+l.table+` WHERE state_id = $1 AND lock_data = $2`, s.ID, rawLock); err != nil { return false, err } @@ -131,14 +149,20 @@ func (l *Lock) Unlock(s *terraform.State) (bool, error) { return true, nil } -func (l *Lock) GetLock(s *terraform.State) ([]byte, error) { +func (l *Lock) GetLock(s *terraform.State) (terraform.LockInfo, error) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - var lock []byte + var rawLock []byte - if err := l.db.QueryRowContext(ctx, `SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&lock); err != nil { - return nil, err + if err := l.db.QueryRowContext(ctx, `SELECT lock_data FROM `+l.table+` WHERE state_id = $1`, s.ID).Scan(&rawLock); err != nil { + return terraform.LockInfo{}, err + } + + var lock terraform.LockInfo + + if err := json.Unmarshal(rawLock, &lock); err != nil { + return terraform.LockInfo{}, err } return lock, nil diff --git a/pkg/lock/redis/redis.go b/pkg/lock/redis/redis.go index 4ec3eab..15e430b 100644 --- a/pkg/lock/redis/redis.go +++ b/pkg/lock/redis/redis.go @@ -3,6 +3,7 @@ package redis import ( "context" "encoding/base64" + "encoding/json" "errors" "fmt" "time" @@ -86,7 +87,7 @@ func (r *Lock) Lock(s *terraform.State) (locked bool, err error) { } // state is locked - if string(lock) == string(s.Lock) { + if lock.Equal(s.Lock) { return true, nil } @@ -123,7 +124,7 @@ func (r *Lock) Unlock(s *terraform.State) (unlocked bool, err error) { return false, nil } - if string(lock) != string(s.Lock) { + if !lock.Equal(s.Lock) { return false, nil } @@ -134,7 +135,7 @@ func (r *Lock) Unlock(s *terraform.State) (unlocked bool, err error) { return true, nil } -func (r *Lock) GetLock(s *terraform.State) (lock []byte, err error) { +func (r *Lock) GetLock(s *terraform.State) (lock terraform.LockInfo, err error) { mutex := r.client.NewMutex(lockKey, redsync.WithExpiry(12*time.Hour), redsync.WithTries(1), redsync.WithGenValueFunc(func() (string, error) { return uuid.New().String(), nil })) @@ -143,7 +144,7 @@ func (r *Lock) GetLock(s *terraform.State) (lock []byte, err error) { if err := mutex.Lock(); err != nil { log.Errorf("failed to lock redsync mutex: %v", err) - return nil, err + return terraform.LockInfo{}, err } defer func() { @@ -170,7 +171,14 @@ func (r *Lock) setLock(s *terraform.State) error { defer conn.Close() - reply, err := redigo.String(conn.Do("SET", s.ID, base64.StdEncoding.EncodeToString(s.Lock), "NX", "PX", int(12*time.Hour/time.Millisecond))) + rawLock, err := json.Marshal(s.Lock) + if err != nil { + return err + } + + lockString := base64.StdEncoding.EncodeToString(rawLock) + + reply, err := redigo.String(conn.Do("SET", s.ID, lockString, "NX", "PX", int(12*time.Hour/time.Millisecond))) if err != nil { return err } @@ -182,24 +190,30 @@ func (r *Lock) setLock(s *terraform.State) error { return nil } -func (r *Lock) getLock(s *terraform.State) ([]byte, error) { +func (r *Lock) getLock(s *terraform.State) (terraform.LockInfo, error) { ctx := context.Background() conn, err := r.pool.GetContext(ctx) if err != nil { - return nil, err + return terraform.LockInfo{}, err } defer conn.Close() value, err := redigo.String(conn.Do("GET", s.ID)) if err != nil { - return nil, err + return terraform.LockInfo{}, err } - lock, err := base64.StdEncoding.DecodeString(value) + rawLock, err := base64.StdEncoding.DecodeString(value) if err != nil { - return nil, err + return terraform.LockInfo{}, err + } + + var lock terraform.LockInfo + + if err := json.Unmarshal(rawLock, &lock); err != nil { + return terraform.LockInfo{}, err } return lock, nil diff --git a/pkg/lock/redis/redis_test.go b/pkg/lock/redis/redis_test.go index 7d1e950..4c17ee3 100644 --- a/pkg/lock/redis/redis_test.go +++ b/pkg/lock/redis/redis_test.go @@ -32,7 +32,7 @@ func TestGetLock(t *testing.T) { ID: terraform.GetStateID("test", "test"), Project: "test", Name: "test", - Lock: []byte(expectedLock), + Lock: terraform.LockInfo{ID: expectedLock}, } { @@ -49,8 +49,8 @@ func TestGetLock(t *testing.T) { t.Error(err) } - if string(lock) != string(expectedLock) { - t.Errorf("lock mismatch: %s != %s", string(lock), string(expectedLock)) + if lock.ID != expectedLock { + t.Errorf("lock mismatch: %s != %s", lock.ID, expectedLock) } } diff --git a/pkg/lock/util/locktest.go b/pkg/lock/util/locktest.go index 91c2166..ba078ca 100644 --- a/pkg/lock/util/locktest.go +++ b/pkg/lock/util/locktest.go @@ -2,6 +2,7 @@ package util import ( "testing" + "time" "github.com/google/uuid" "github.com/spf13/viper" @@ -19,7 +20,15 @@ func LockTest(t *testing.T, l lock.Locker) { ID: terraform.GetStateID("test", "test"), Project: "test", Name: "test", - Lock: []byte(uuid.New().String()), + Lock: terraform.LockInfo{ + ID: uuid.New().String(), + Path: "", + Operation: "LockTest", + Who: "test", + Version: "0.0.0", + Created: time.Now().String(), + Info: "", + }, } t.Logf("s1: %s", s1.Lock) @@ -27,7 +36,15 @@ func LockTest(t *testing.T, l lock.Locker) { ID: terraform.GetStateID("test", "test"), Project: "test", Name: "test", - Lock: []byte(uuid.New().String()), + Lock: terraform.LockInfo{ + ID: uuid.New().String(), + Path: "", + Operation: "LockTest", + Who: "test", + Version: "0.0.0", + Created: time.Now().String(), + Info: "", + }, } t.Logf("s2: %s", s2.Lock) @@ -45,7 +62,7 @@ func LockTest(t *testing.T, l lock.Locker) { if lock, err := l.GetLock(&s1); err != nil { t.Error(err) - } else if string(lock) != string(s1.Lock) { + } else if !lock.Equal(s1.Lock) { t.Errorf("lock is not equal: %s != %s", lock, s1.Lock) } @@ -57,7 +74,7 @@ func LockTest(t *testing.T, l lock.Locker) { t.Error("should not be able to lock twice from different processes") } - if string(s2.Lock) != string(s1.Lock) { + if !s2.Lock.Equal(s1.Lock) { t.Error("failed Lock() should return the lock information of the current lock") } diff --git a/pkg/server/handler.go b/pkg/server/handler.go index 4e0322a..48810f7 100644 --- a/pkg/server/handler.go +++ b/pkg/server/handler.go @@ -1,10 +1,10 @@ package server import ( + "encoding/json" "fmt" "io" "net/http" - "strings" "github.com/gorilla/mux" log "github.com/sirupsen/logrus" @@ -77,14 +77,18 @@ func StateHandler(store storage.Storage, locker lock.Locker, kms kms.KMS) func(h func Lock(w http.ResponseWriter, state *terraform.State, body []byte, locker lock.Locker) { log.Debugf("try to lock state with id %s", state.ID) - state.Lock = body + + if err := json.Unmarshal(body, &state.Lock); err != nil { + log.Errorf("failed to unmarshal lock info: %v", err) + HTTPResponse(w, http.StatusInternalServerError, "") + } if ok, err := locker.Lock(state); err != nil { log.Errorf("failed to lock state with id %s: %v", state.ID, err) HTTPResponse(w, http.StatusInternalServerError, "") } else if !ok { log.Warnf("state with id %s is already locked by %s", state.ID, state.Lock) - HTTPResponse(w, http.StatusLocked, string(state.Lock)) + HTTPResponse(w, http.StatusLocked, state.Lock.ID) } else { log.Debugf("state with id %s was locked successfully", state.ID) HTTPResponse(w, http.StatusOK, "") @@ -93,14 +97,18 @@ func Lock(w http.ResponseWriter, state *terraform.State, body []byte, locker loc func Unlock(w http.ResponseWriter, state *terraform.State, body []byte, locker lock.Locker) { log.Debugf("try to unlock state with id %s", state.ID) - state.Lock = body + + if err := json.Unmarshal(body, &state.Lock); err != nil { + log.Errorf("failed to unmarshal lock info: %v", err) + HTTPResponse(w, http.StatusInternalServerError, "") + } if ok, err := locker.Unlock(state); err != nil { log.Errorf("failed to unlock state with id %s: %v", state.ID, err) HTTPResponse(w, http.StatusInternalServerError, "") } else if !ok { log.Warnf("failed to unlock state with id %s: %v", state.ID, err) - HTTPResponse(w, http.StatusBadRequest, string(state.Lock)) + HTTPResponse(w, http.StatusBadRequest, state.Lock.ID) } else { log.Debugf("state with id %s was unlocked successfully", state.ID) HTTPResponse(w, http.StatusOK, "") @@ -132,15 +140,15 @@ func Get(w http.ResponseWriter, state *terraform.State, store storage.Storage, k func Post(r *http.Request, w http.ResponseWriter, state *terraform.State, body []byte, locker lock.Locker, store storage.Storage, kms kms.KMS) { reqLockID := r.URL.Query().Get("ID") - lockID, err := locker.GetLock(state) + lock, err := locker.GetLock(state) if err != nil { log.Warnf("failed to get lock for state with id %s: %v", state.ID, err) HTTPResponse(w, http.StatusInternalServerError, "") return } - if !strings.Contains(string(lockID), fmt.Sprintf(`"ID":"%s"`, reqLockID)) { - log.Warnf("attempting to write state with wrong lock %s (expected %s)", reqLockID, lockID) + if lock.ID != reqLockID { + log.Warnf("attempting to write state with wrong lock %s (expected %s)", reqLockID, lock.ID) HTTPResponse(w, http.StatusBadRequest, "") return } diff --git a/pkg/server/handler_test.go b/pkg/server/handler_test.go index da0d3ea..762f21d 100644 --- a/pkg/server/handler_test.go +++ b/pkg/server/handler_test.go @@ -19,6 +19,7 @@ import ( localkms "github.com/nimbolus/terraform-backend/pkg/kms/local" locallock "github.com/nimbolus/terraform-backend/pkg/lock/local" "github.com/nimbolus/terraform-backend/pkg/storage/filesystem" + tf "github.com/nimbolus/terraform-backend/pkg/terraform" ) func NewStateHandler(t *testing.T) http.Handler { @@ -111,20 +112,20 @@ func TestServerHandler(t *testing.T) { terraform.ApplyAndIdempotent(t, terraformOptions) } -func simulateLock(t *testing.T, address string, lock bool) { +func simulateLock(t *testing.T, address string, doLock bool) { method := "LOCK" - if !lock { + if !doLock { method = "UNLOCK" } - postBody, _ := json.Marshal(map[string]string{ - "ID": "cf290ef3-6090-410e-9784-d017a4b1536a", - "Path": "", - "Operation": "simulateLock", - "Who": "simulator", - "Version": "0.0.0", - "Created": "2021-01-01T00:00:00Z", - "Info": "", + postBody, _ := json.Marshal(&tf.LockInfo{ + ID: "cf290ef3-6090-410e-9784-d017a4b1536a", + Path: "", + Operation: "simulateLock", + Who: "simulator", + Version: "0.0.0", + Created: "2021-01-01T00:00:00Z", + Info: "", }) req, err := http.NewRequest(method, address, bytes.NewBuffer(postBody)) diff --git a/pkg/terraform/terraform.go b/pkg/terraform/terraform.go index 8fb9106..70d4e9d 100644 --- a/pkg/terraform/terraform.go +++ b/pkg/terraform/terraform.go @@ -8,7 +8,7 @@ import ( type State struct { ID string Data []byte - Lock []byte + Lock LockInfo Project string Name string } @@ -18,3 +18,23 @@ func GetStateID(project, id string) string { hash := sha256.Sum256([]byte(path)) return fmt.Sprintf("%x", hash[:]) } + +type LockInfo struct { + ID string `json:"ID"` + Path string `json:"Path"` + Operation string `json:"Operation"` + Who string `json:"Who"` + Version string `json:"Version"` + Created string `json:"Created"` + Info string `json:"Info"` +} + +func (l LockInfo) Equal(r LockInfo) bool { + return l.ID == r.ID && + l.Path == r.Path && + l.Operation == r.Operation && + l.Who == r.Who && + l.Version == r.Version && + l.Created == r.Created && + l.Info == r.Info +} From 43193339bcfb54cf02f19d2b66c19b2f77dc05c6 Mon Sep 17 00:00:00 2001 From: Tobias Krischer Date: Mon, 10 Oct 2022 15:16:33 +0200 Subject: [PATCH 3/3] fix some http status codes --- pkg/server/handler.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/server/handler.go b/pkg/server/handler.go index 48810f7..33709d7 100644 --- a/pkg/server/handler.go +++ b/pkg/server/handler.go @@ -80,7 +80,7 @@ func Lock(w http.ResponseWriter, state *terraform.State, body []byte, locker loc if err := json.Unmarshal(body, &state.Lock); err != nil { log.Errorf("failed to unmarshal lock info: %v", err) - HTTPResponse(w, http.StatusInternalServerError, "") + HTTPResponse(w, http.StatusBadRequest, "") } if ok, err := locker.Lock(state); err != nil { @@ -100,7 +100,7 @@ func Unlock(w http.ResponseWriter, state *terraform.State, body []byte, locker l if err := json.Unmarshal(body, &state.Lock); err != nil { log.Errorf("failed to unmarshal lock info: %v", err) - HTTPResponse(w, http.StatusInternalServerError, "") + HTTPResponse(w, http.StatusBadRequest, "") } if ok, err := locker.Unlock(state); err != nil { @@ -143,7 +143,7 @@ func Post(r *http.Request, w http.ResponseWriter, state *terraform.State, body [ lock, err := locker.GetLock(state) if err != nil { log.Warnf("failed to get lock for state with id %s: %v", state.ID, err) - HTTPResponse(w, http.StatusInternalServerError, "") + HTTPResponse(w, http.StatusBadRequest, "") return }