From 9457a13c7ccff55f4eb2d50981c22517fdac5f08 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Mon, 18 Jul 2022 14:19:29 -0400
Subject: [PATCH] fsm: one-time token expiration should be deterministic
(#13737)
When applying a raft log to expire ACL tokens, we need to use a
timestamp provided by the leader so that the result is deterministic
across servers. Use leader's timestamp from RPC call
---
.changelog/13737.txt | 3 +++
nomad/acl_endpoint.go | 2 ++
nomad/fsm.go | 2 +-
nomad/state/state_store.go | 8 ++++----
nomad/state/state_store_test.go | 12 ++++++------
nomad/structs/structs.go | 1 +
6 files changed, 17 insertions(+), 11 deletions(-)
create mode 100644 .changelog/13737.txt
diff --git a/.changelog/13737.txt b/.changelog/13737.txt
new file mode 100644
index 0000000000..3130f221c0
--- /dev/null
+++ b/.changelog/13737.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+acl: Fixed a bug where the timestamp for expiring one-time tokens was not deterministic between servers
+```
diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index 23a9e9913a..6c61ce46d3 100644
--- a/nomad/acl_endpoint.go
+++ b/nomad/acl_endpoint.go
@@ -1013,6 +1013,8 @@ func (a *ACL) ExpireOneTimeTokens(args *structs.OneTimeTokenExpireRequest, reply
}
}
+ args.Timestamp = time.Now() // use the leader's timestamp
+
// Expire token via raft; because this is the only write in the RPC the
// caller can safely retry with the same token if the raft write fails
_, index, err := a.srv.raftApply(structs.OneTimeTokenExpireRequestType, args)
diff --git a/nomad/fsm.go b/nomad/fsm.go
index 0b4cc1305f..4339dcf723 100644
--- a/nomad/fsm.go
+++ b/nomad/fsm.go
@@ -1229,7 +1229,7 @@ func (n *nomadFSM) applyOneTimeTokenExpire(msgType structs.MessageType, buf []by
panic(fmt.Errorf("failed to decode request: %v", err))
}
- if err := n.state.ExpireOneTimeTokens(msgType, index); err != nil {
+ if err := n.state.ExpireOneTimeTokens(msgType, index, req.Timestamp); err != nil {
n.logger.Error("ExpireOneTimeTokens failed", "error", err)
return err
}
diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go
index 430bd60815..7069f66f44 100644
--- a/nomad/state/state_store.go
+++ b/nomad/state/state_store.go
@@ -5852,11 +5852,11 @@ func (s *StateStore) DeleteOneTimeTokens(msgType structs.MessageType, index uint
}
// ExpireOneTimeTokens deletes tokens that have expired
-func (s *StateStore) ExpireOneTimeTokens(msgType structs.MessageType, index uint64) error {
+func (s *StateStore) ExpireOneTimeTokens(msgType structs.MessageType, index uint64, timestamp time.Time) error {
txn := s.db.WriteTxnMsgT(msgType, index)
defer txn.Abort()
- iter, err := s.oneTimeTokensExpiredTxn(txn, nil)
+ iter, err := s.oneTimeTokensExpiredTxn(txn, nil, timestamp)
if err != nil {
return err
}
@@ -5887,14 +5887,14 @@ func (s *StateStore) ExpireOneTimeTokens(msgType structs.MessageType, index uint
}
// oneTimeTokensExpiredTxn returns an iterator over all expired one-time tokens
-func (s *StateStore) oneTimeTokensExpiredTxn(txn *txn, ws memdb.WatchSet) (memdb.ResultIterator, error) {
+func (s *StateStore) oneTimeTokensExpiredTxn(txn *txn, ws memdb.WatchSet, timestamp time.Time) (memdb.ResultIterator, error) {
iter, err := txn.Get("one_time_token", "id")
if err != nil {
return nil, fmt.Errorf("one-time token lookup failed: %v", err)
}
ws.Add(iter.WatchCh())
- iter = memdb.NewFilterIterator(iter, expiredOneTimeTokenFilter(time.Now()))
+ iter = memdb.NewFilterIterator(iter, expiredOneTimeTokenFilter(timestamp))
return iter, nil
}
diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go
index 5f3c9f4e17..6e37c4b55e 100644
--- a/nomad/state/state_store_test.go
+++ b/nomad/state/state_store_test.go
@@ -8912,9 +8912,9 @@ func TestStateStore_OneTimeTokens(t *testing.T) {
// now verify expiration
- getExpiredTokens := func() []*structs.OneTimeToken {
+ getExpiredTokens := func(now time.Time) []*structs.OneTimeToken {
txn := state.db.ReadTxn()
- iter, err := state.oneTimeTokensExpiredTxn(txn, nil)
+ iter, err := state.oneTimeTokensExpiredTxn(txn, nil, now)
require.NoError(t, err)
results := []*structs.OneTimeToken{}
@@ -8930,7 +8930,7 @@ func TestStateStore_OneTimeTokens(t *testing.T) {
return results
}
- results = getExpiredTokens()
+ results = getExpiredTokens(time.Now())
require.Len(t, results, 2)
// results aren't ordered
@@ -8942,10 +8942,10 @@ func TestStateStore_OneTimeTokens(t *testing.T) {
// clear the expired tokens and verify they're gone
index++
- require.NoError(t,
- state.ExpireOneTimeTokens(structs.MsgTypeTestSetup, index))
+ require.NoError(t, state.ExpireOneTimeTokens(
+ structs.MsgTypeTestSetup, index, time.Now()))
- results = getExpiredTokens()
+ results = getExpiredTokens(time.Now())
require.Len(t, results, 0)
// query the unexpired token
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index 800531e1b4..4d6e9bc386 100644
--- a/nomad/structs/structs.go
+++ b/nomad/structs/structs.go
@@ -12077,6 +12077,7 @@ type OneTimeTokenDeleteRequest struct {
// OneTimeTokenExpireRequest is a request to delete all expired one-time tokens
type OneTimeTokenExpireRequest struct {
+ Timestamp time.Time
WriteRequest
}
--
GitLab