From e8a0ec12b395c2396c412f36c32e2713f8aed3f3 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 20 Oct 2022 13:14:08 -0400 Subject: [PATCH] keyring: set MinQueryIndex on stale queries When keyring replication makes a stale query to non-leader peers to find a key the leader doesn't have, we need to make sure the peer we're querying has had a chance to catch up to the most current index for that key. Otherwise it's possible for newly-added servers to query another newly-added server and get a non-error nil response for that key ID. Ensure that we're setting the correct reply index in the blocking query. Note that the "not found" case does not return an error, just an empty key. So as a belt-and-suspenders, update the handling of empty responses so that we don't break the loop early if we hit a server that doesn't have the key. --- nomad/encrypter.go | 5 +++-- nomad/keyring_endpoint.go | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/nomad/encrypter.go b/nomad/encrypter.go index 388482b4e8f8..9b6a13864b87 100644 --- a/nomad/encrypter.go +++ b/nomad/encrypter.go @@ -464,7 +464,8 @@ START: getReq := &structs.KeyringGetRootKeyRequest{ KeyID: keyID, QueryOptions: structs.QueryOptions{ - Region: krr.srv.config.Region, + Region: krr.srv.config.Region, + MinQueryIndex: keyMeta.ModifyIndex - 1, }, } getResp := &structs.KeyringGetRootKeyResponse{} @@ -482,7 +483,7 @@ START: getReq.AllowStale = true for _, peer := range krr.getAllPeers() { err = krr.srv.forwardServer(peer, "Keyring.Get", getReq, getResp) - if err == nil { + if err == nil && getResp.Key != nil { break } } diff --git a/nomad/keyring_endpoint.go b/nomad/keyring_endpoint.go index 78d4808ce298..9b7e27ad90f7 100644 --- a/nomad/keyring_endpoint.go +++ b/nomad/keyring_endpoint.go @@ -264,7 +264,20 @@ func (k *Keyring) Get(args *structs.KeyringGetRootKeyRequest, reply *structs.Key Key: key, } reply.Key = rootKey - reply.Index = keyMeta.ModifyIndex + + // Use the last index that affected the policy table + index, err := s.Index(state.TableRootKeyMeta) + if err != nil { + return err + } + + // Ensure we never set the index to zero, otherwise a blocking query + // cannot be used. We floor the index at one, since realistically + // the first write must have a higher index. + if index == 0 { + index = 1 + } + reply.Index = index return nil }, }