From ab36cf031c80253d9c2827852f8ecbeae3d6ff5b Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Tue, 7 Nov 2023 19:50:19 -0500 Subject: [PATCH] vault: avoid continual renewal of invalid token (#18985) A series of errors may happen when a token is invalidated while the Vault client is waiting to renew it. The token may have been invalidated for several reasons, such as the alloc finished running and it's now terminal or the token may have been change directly on Vault out-of-band. Most of the errors are caused by retries that will never succeed until Vault fully removes the token from its state. This commit prevents the retries by making the error `invalid lease ID` a fatal error. In earlier versions of Vault, this case was covered by the error `lease not found or lease is not renewable`, which is already considered to be a fatal error by Nomad: https://github.com/hashicorp/vault/blob/2d0cde4ccc0323591d9414342cb15f5cb70271d7/vault/expiration.go#L636-L639 But https://github.com/hashicorp/vault/pull/5346 introduced an earlier `nil` check that generates a different error message: https://github.com/hashicorp/vault/blob/750ab337eaa0b049d9cf1535c00e860129e5e9a0/vault/expiration.go#L1362-L1364 Both errors happen for the same reason (`le == nil`) and so should be considered fatal on renewal. --- .changelog/18985.txt | 3 +++ client/vaultclient/vaultclient.go | 1 + 2 files changed, 4 insertions(+) create mode 100644 .changelog/18985.txt diff --git a/.changelog/18985.txt b/.changelog/18985.txt new file mode 100644 index 00000000000..432582a858f --- /dev/null +++ b/.changelog/18985.txt @@ -0,0 +1,3 @@ +```release-note:bug +vault: Fixed an issue that could cause Nomad to attempt to renew a Vault token that is already expired +``` diff --git a/client/vaultclient/vaultclient.go b/client/vaultclient/vaultclient.go index e9f59de5e6b..75d36335e14 100644 --- a/client/vaultclient/vaultclient.go +++ b/client/vaultclient/vaultclient.go @@ -512,6 +512,7 @@ func (c *vaultClient) renew(req *vaultClientRenewalRequest) error { fatal := false if renewalErr != nil && (strings.Contains(renewalErr.Error(), "lease not found or lease is not renewable") || + strings.Contains(renewalErr.Error(), "invalid lease ID") || strings.Contains(renewalErr.Error(), "lease is not renewable") || strings.Contains(renewalErr.Error(), "token not found") || strings.Contains(renewalErr.Error(), "permission denied")) {