Skip to content

Commit

Permalink
keyring: set MinQueryIndex on stale queries
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgross committed Oct 20, 2022
1 parent 958fab4 commit e8a0ec1
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
5 changes: 3 additions & 2 deletions nomad/encrypter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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
}
}
Expand Down
15 changes: 14 additions & 1 deletion nomad/keyring_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
Expand Down

0 comments on commit e8a0ec1

Please sign in to comment.