fix(vault): handle token renewal failures (#497)

* Cache Vault clients/tokens on a per-role basis. * Clear vault token if errors when renewing or self-inspecting
external-secrets · Oct 6, 2020 · c3c27bc · c3c27bc
1 parent ab36718
commit c3c27bc
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 9 deletions.
diff --git a/config/index.js b/config/index.js
@@ -83,6 +83,7 @@ const vaultFactory = () => vault(vaultOptions)
 // expires and with at least one remaining poll opportunty to retry renewal if it fails.
 const vaultTokenRenewThreshold = envConfig.vaultTokenRenewThreshold
   ? Number(envConfig.vaultTokenRenewThreshold) : 3 * envConfig.pollerIntervalMilliseconds / 1000
+
 const vaultBackend = new VaultBackend({
   vaultFactory: vaultFactory,
   tokenRenewThreshold: vaultTokenRenewThreshold,

diff --git a/lib/backends/vault-backend.js b/lib/backends/vault-backend.js
@@ -53,6 +53,29 @@ class VaultBackend extends KVBackend {
       this._clients.set(clientCacheKey, client)
     }
 
+    // If we already have a cached token then inspect it...
+    if (client.token) {
+      try {
+        this._logger.debug(`checking vault token expiry for role ${vaultRoleGet} on ${vaultMountPointGet}`)
+        const tokenStatus = await client.tokenLookupSelf()
+        this._logger.debug(`vault token (role ${vaultRoleGet} on ${vaultMountPointGet}) valid for ${tokenStatus.data.ttl} seconds, renews at ${this._tokenRenewThreshold}`)
+
+        // If it needs renewing, renew it.
+        if (Number(tokenStatus.data.ttl) <= this._tokenRenewThreshold) {
+          this._logger.debug(`renewing role ${vaultRoleGet} on ${vaultMountPointGet} vault token`)
+          if (!(await client.tokenRenewSelf())) {
+            this._logger.debug(`cached token renewal failed.  Clearing cached token for role ${vaultRoleGet} on ${vaultMountPointGet}`)
+            client.token = null
+          }
+        }
+      } catch {
+        // If it can't be inspected/renewed, we clear the token.
+        this._logger.debug(`cached token operation failed.  Clearing cached token for role ${vaultRoleGet} on ${vaultMountPointGet}`)
+        client.token = null
+      }
+    }
+
+    // If we don't have a token here we either never had one or we just failed to renew it, so get a new one by logging-in
     if (!client.token) {
       const jwt = this._fetchServiceAccountToken()
       this._logger.debug(`fetching new token from vault for role ${vaultRoleGet} on ${vaultMountPointGet}`)
@@ -61,15 +84,6 @@ class VaultBackend extends KVBackend {
         role: vaultRoleGet,
         jwt: jwt
       })
-    } else {
-      this._logger.debug(`checking vault token expiry for role ${vaultRoleGet} on ${vaultMountPointGet}`)
-      const tokenStatus = await client.tokenLookupSelf()
-      this._logger.debug(`vault token (role ${vaultRoleGet} on ${vaultMountPointGet}) valid for ${tokenStatus.data.ttl} seconds, renews at ${this._tokenRenewThreshold}`)
-
-      if (Number(tokenStatus.data.ttl) <= this._tokenRenewThreshold) {
-        this._logger.debug(`renewing role ${vaultRoleGet} on ${vaultMountPointGet} vault token`)
-        await client.tokenRenewSelf()
-      }
     }
 
     this._logger.debug(`reading secret key ${key} from vault`)

diff --git a/lib/backends/vault-backend.test.js b/lib/backends/vault-backend.test.js
@@ -40,6 +40,7 @@ describe('VaultBackend', () => {
   }
 
   const vaultTokenRenewThreshold = 30
+
   const mockTokenLookupResultMustRenew = {
     data: {
       ttl: 15
@@ -213,6 +214,93 @@ describe('VaultBackend', () => {
       expect(secretPropertyValue).equals(quotedSecretValue)
     })
 
+    it('returns secret property value after failed renew token and then relogin', async () => {
+      clientMock.token = 'an-existing-token'
+      clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultMustRenew)
+      clientMock.tokenRenewSelf = sinon.stub().returns(false)
+
+      const secretPropertyValue = await vaultBackend._get({
+        specOptions: {
+          vaultMountPoint: mountPoint,
+          vaultRole: role
+        },
+        key: secretKey
+      })
+
+      // check the token happened ...
+      sinon.assert.calledOnce(clientMock.tokenLookupSelf)
+
+      // ... then try to renew the token ...
+      sinon.assert.calledOnce(clientMock.tokenRenewSelf)
+
+      // ... now relogin due to failure to renew ...
+      sinon.assert.calledOnce(clientMock.kubernetesLogin)
+
+      // ... then we fetch the secret ...
+      sinon.assert.calledWith(clientMock.read, secretKey)
+
+      // ... and expect to get its proper value
+      expect(secretPropertyValue).equals(quotedSecretValue)
+    })
+
+    it('returns secret property value after errored token inspection and then relogin', async () => {
+      clientMock.token = 'an-existing-token'
+      clientMock.tokenLookupSelf = sinon.stub().throws(new Error())
+      clientMock.tokenRenewSelf = sinon.stub().returns(false)
+
+      const secretPropertyValue = await vaultBackend._get({
+        specOptions: {
+          vaultMountPoint: mountPoint,
+          vaultRole: role
+        },
+        key: secretKey
+      })
+
+      // check the token happened ...
+      sinon.assert.calledOnce(clientMock.tokenLookupSelf)
+
+      // ... didn't try to renew the token ...
+      sinon.assert.notCalled(clientMock.tokenRenewSelf)
+
+      // ... but instead just relogin due to error upon inspection ...
+      sinon.assert.calledOnce(clientMock.kubernetesLogin)
+
+      // ... then we fetch the secret ...
+      sinon.assert.calledWith(clientMock.read, secretKey)
+
+      // ... and expect to get its proper value
+      expect(secretPropertyValue).equals(quotedSecretValue)
+    })
+
+    it('returns secret property value after errored token renewal and then relogin', async () => {
+      clientMock.token = 'an-existing-token'
+      clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultMustRenew)
+      clientMock.tokenRenewSelf = sinon.stub().throws(new Error())
+
+      const secretPropertyValue = await vaultBackend._get({
+        specOptions: {
+          vaultMountPoint: mountPoint,
+          vaultRole: role
+        },
+        key: secretKey
+      })
+
+      // check the token happened ...
+      sinon.assert.calledOnce(clientMock.tokenLookupSelf)
+
+      // ... didn't try to renew the token ...
+      sinon.assert.calledOnce(clientMock.tokenRenewSelf)
+
+      // ... but instead just relogin due to error upon renewal ...
+      sinon.assert.calledOnce(clientMock.kubernetesLogin)
+
+      // ... then we fetch the secret ...
+      sinon.assert.calledWith(clientMock.read, secretKey)
+
+      // ... and expect to get its proper value
+      expect(secretPropertyValue).equals(quotedSecretValue)
+    })
+
     it('returns secret property value if a token exists that does not need renewal', async () => {
       clientMock.token = 'an-existing-token'
       clientMock.tokenLookupSelf = sinon.stub().returns(mockTokenLookupResultNoRenew)