Repository: godaddy/kubernetes-external-secrets
Pull request #214 (2019-11-13 14:12:07)

Vault: stringify JSON response for compatibility with KV backend
Hello, it's me again 👋 

Due to changes in https://github.com/godaddy/kubernetes-external-secrets/pull/206,
we now need to stringify the response from the backend which currently comes as
an object rather than a JSON string.

Fixes issue such as:

```
{"level":30,"time":1573653228258,"pid":19,"hostname":"external-secrets-8zq4bq","msg":"running poll on the secret example/example","v":1}
{"level":40,"time":1573653228468,"pid":19,"hostname":"external-secrets-8zq4bq","msg":"Failed to JSON.parse value for 'secrets/data/example/credentials', please verify that your secret value is correctly formatted as JSON. To use plain text secret remove the 'property: apiToken'","v":1}
```

Pluies: cc @Flydiverny 
Pluies: > can you update the test so the build passes ?
Eh! My turn to "doh" :)

Done on this PR; and I'm working on tests that will run `vaultBackend.getSecretManifestData` to test more than the (fairly straightforward) inner loop of `_get`. I also want to include tests with broken data or failed responses from Vault to test edge cases and ensure they're handled correctly. I'll open a separate PR once this is ready 👍 

Patch:
diff --git a/lib/backends/vault-backend.js b/lib/backends/vault-backend.js
index 68687d2..5fcd652 100644
--- a/lib/backends/vault-backend.js
+++ b/lib/backends/vault-backend.js
@@ -53,7 +53,7 @@ class VaultBackend extends KVBackend {
     this._logger.debug(`reading secret key ${key} from vault`)
     const secretResponse = await this._client.read(key)
 
-    return secretResponse.data.data
+    return JSON.stringify(secretResponse.data.data)
   }
 }
 
diff --git a/lib/poller.js b/lib/poller.js
index c26946d..ebbf7db 100644
--- a/lib/poller.js
+++ b/lib/poller.js
@@ -159,17 +159,25 @@ class Poller {
   }
 
   async _updateStatus (status) {
-    this._logger.debug(`updating status for ${this._namespace}/${this._name} to: ${status}`)
-    await this._status.put({
-      body: {
-        ...this._externalSecret,
-        status: {
-          lastSync: `${new Date().toISOString()}`,
-          observedGeneration: this._externalSecret.metadata.generation,
-          status
+    try {
+      this._logger.debug(`updating status for ${this._namespace}/${this._name} to: ${status}`)
+      await this._status.put({
+        body: {
+          ...this._externalSecret,
+          status: {
+            lastSync: `${new Date().toISOString()}`,
+            observedGeneration: this._externalSecret.metadata.generation,
+            status
+          }
         }
+      })
+    } catch (err) {
+      if (err.statusCode !== 409) {
+        this._logger.error(err, `failure while updating status for externalsecret ${this._namespace}/${this._name}`)
+        throw err
       }
-    })
+      this._logger.info(`status update failed for externalsecret ${this._namespace}/${this._name}, due to modification, new poller should start`)
+    }
   }
 
   /**


Test patch:
diff --git a/lib/backends/vault-backend.test.js b/lib/backends/vault-backend.test.js
index 8ef5dd2..8d1aa20 100644
--- a/lib/backends/vault-backend.test.js
+++ b/lib/backends/vault-backend.test.js
@@ -69,7 +69,7 @@ describe('VaultBackend', () => {
       sinon.assert.calledWith(clientMock.read, 'fakeSecretKey')
 
       // ... and expect to get its proper value
-      expect(secretPropertyValue).equals('fakeSecretPropertyValue')
+      expect(secretPropertyValue).equals('"fakeSecretPropertyValue"')
     })
 
     it('returns secret property value after renewing token if a token exists', async () => {
@@ -93,7 +93,7 @@ describe('VaultBackend', () => {
       sinon.assert.calledWith(clientMock.read, 'fakeSecretKey')
 
       // ... and expect to get its proper value
-      expect(secretPropertyValue).equals('fakeSecretPropertyValue')
+      expect(secretPropertyValue).equals('"fakeSecretPropertyValue"')
     })
   })
 })
diff --git a/lib/poller.test.js b/lib/poller.test.js
index d9abd73..05132c5 100644
--- a/lib/poller.test.js
+++ b/lib/poller.test.js
@@ -371,6 +371,38 @@ describe('Poller', () => {
     })
   })
 
+  describe('_updateStatus', () => {
+    it('handles 409 - Conflict', async () => {
+      const conflictError = new Error('Conflict')
+      conflictError.statusCode = 409
+      externalSecretsApiMock.status.put.throws(conflictError)
+
+      const poller = pollerFactory()
+
+      await poller._updateStatus('SUCCESS')
+
+      expect(loggerMock.info.calledWith(`status update failed for externalsecret ${poller._namespace}/${poller._name}, due to modification, new poller should start`)).to.equal(true)
+    })
+
+    it('rethrows other errors', async () => {
+      const notFoundError = new Error('Not Found')
+      notFoundError.statusCode = 404
+      externalSecretsApiMock.status.put.throws(notFoundError)
+
+      const poller = pollerFactory()
+      let error
+
+      try {
+        await poller._updateStatus('SUCCESS')
+      } catch (err) {
+        error = err
+      }
+
+      expect(error).to.not.equal(undefined)
+      expect(error.message).equals('Not Found')
+    })
+  })
+
   describe('_scheduleNextPoll', () => {
     let poller
     let clock