feat: Upsert secrets only when needed (#782)

Author: stephenthedev

charts/kubernetes-external-secrets/templates/rbac.yaml

@@ -15,7 +15,7 @@ metadata:
 rules:
   - apiGroups: [""]
     resources: ["secrets"]
-    verbs: ["create", "update"]
+    verbs: ["create", "update", "get"]
   - apiGroups: [""]
     resources: ["namespaces"]
     verbs: ["get", "watch", "list"]

lib/poller.js

@@ -163,14 +163,57 @@ class Poller {
     }
 
     const secretManifest = await this._createSecretManifest()
-    this._logger.info(`upserting secret ${this._namespace}/${this._name}`)
+    const kubeSecret = kubeNamespace.secrets(this._name)
+    let existingSecret
 
     try {
-      return await kubeNamespace.secrets.post({ body: secretManifest })
+      this._logger.info(`getting secret ${this._namespace}/${this._name}`)
+      const secretResponse = await kubeSecret.get()
+      existingSecret = secretResponse.body
     } catch (err) {
-      if (err.statusCode !== 409) throw err
-      return kubeNamespace.secrets(this._name).put({ body: secretManifest })
+      if (err.statusCode !== 404) throw err
+      // do nothing if the secret is not found
     }
+
+    if (existingSecret && this._equalSecretData(existingSecret, secretManifest)) {
+      this._logger.info(`skipping secret ${this._namespace}/${this._name} upsert, objects are the same`)
+      return Promise.resolve(true)
+    } else if (existingSecret === undefined) {
+      this._logger.info(`creating secret ${this._namespace}/${this._name}`)
+      return await kubeNamespace.secrets.post({ body: secretManifest })
+    } else {
+      this._logger.info(`updating secret ${this._namespace}/${this._name}`)
+      return kubeSecret.put({ body: secretManifest })
+    }
+  }
+
+  /**
+   * Checks if a secret and the desired secret manifest are equal
+   *
+   * @param   {Object}  kubeSecret      An actual kubernetes secret from the kube-client
+   * @param   {Object}  secretManifest  A hash representing the desired secret
+   *
+   * @return  {Boolean}                  Boolean if they are the same or not
+   */
+  _equalSecretData (kubeSecret, secretManifest) {
+    let result = true
+    const liveSecret = clonedeep(kubeSecret)
+    const desiredSecret = clonedeep(secretManifest)
+
+    // Only use annotations and labels for metadata checking
+    const secrets = [liveSecret, desiredSecret]
+    secrets.forEach((s) => {
+      s.metadata = {
+        labels: s.metadata.labels,
+        annotations: s.metadata.annotations
+      }
+    })
+
+    result = result ? JSON.stringify(liveSecret.metadata) === JSON.stringify(desiredSecret.metadata) : false
+
+    // check secret data
+    result = result ? JSON.stringify(liveSecret.data) === JSON.stringify(desiredSecret.data) : false
+    return result
   }
 
   async _updateStatus (status) {

lib/poller.test.js

@@ -677,6 +677,11 @@ describe('Poller', () => {
     })
 
     it('creates new secret', async () => {
+      const notFoundError = new Error('Not Found')
+      notFoundError.statusCode = 404
+      const kubeSecret = sinon.mock()
+      kubeSecret.get = sinon.stub().throws(notFoundError)
+      kubeNamespaceMock.secrets = sinon.stub().returns(kubeSecret)
       kubeNamespaceMock.secrets.post = sinon.stub().resolves()
 
       await poller._upsertKubernetesSecret()
@@ -696,18 +701,88 @@ describe('Poller', () => {
       })).to.equal(true)
     })
 
+    it("doesn't update a secret if it hasn't changed", async () => {
+      const kubeSecret = sinon.mock()
+      kubeSecret.put = sinon.stub()
+      kubeSecret.get = sinon.stub().returns({
+        body: {
+          metadata: {
+            name: 'fakeSecretName'
+          },
+          data: {
+            fakePropertyName: 'ZmFrZVByb3BlcnR5VmFsdWU='
+          }
+        }
+      })
+      kubeNamespaceMock.secrets = sinon.stub().returns(kubeSecret)
+      kubeNamespaceMock.secrets.post = sinon.stub()
+
+      const result = await poller._upsertKubernetesSecret()
+      expect(result).to.equal(true)
+      expect(kubeSecret.put.called).to.equal(false)
+      expect(kubeNamespaceMock.secrets.post.called).to.equal(false)
+    })
+
     it('updates secret', async () => {
-      const conflictError = new Error('Conflict')
-      conflictError.statusCode = 409
-      kubeNamespaceMock.secrets.post = sinon.stub().throws(conflictError)
-      kubeNamespaceMock.put = sinon.stub().resolves()
+      const kubeSecret = sinon.mock()
+      kubeSecret.get = sinon.stub().returns({
+        body: {
+          metadata: {
+            name: 'fakeSecretName'
+          },
+          data: {
+            fakePropertyName: 'differentValue'
+          }
+        }
+      })
+      kubeNamespaceMock.secrets = sinon.stub().returns(kubeSecret)
+      kubeSecret.put = sinon.stub().resolves()
       kubeNamespaceMock.get = sinon.stub().resolves(fakeNamespace)
 
       await poller._upsertKubernetesSecret()
 
       expect(kubeNamespaceMock.secrets.calledWith('fakeSecretName')).to.equal(true)
 
-      expect(kubeNamespaceMock.put.calledWith({
+      expect(kubeSecret.put.calledWith({
+        body: {
+          apiVersion: 'v1',
+          kind: 'Secret',
+          metadata: {
+            name: 'fakeSecretName'
+          },
+          type: 'some-type',
+          data: {
+            fakePropertyName: 'ZmFrZVByb3BlcnR5VmFsdWU='
+          }
+        }
+      })).to.equal(true)
+    })
+
+    it('updates secret if the custom metadata has changed', async () => {
+      const kubeSecret = sinon.mock()
+      kubeSecret.get = sinon.stub().returns({
+        body: {
+          metadata: {
+            creationTimestamp: new Date().toDateString(),
+            name: 'fakeSecretName',
+            labels: {
+              myFakeLabel: 'test'
+            }
+          },
+          data: {
+            fakePropertyName: 'ZmFrZVByb3BlcnR5VmFsdWU='
+          }
+        }
+      })
+      kubeNamespaceMock.secrets = sinon.stub().returns(kubeSecret)
+      kubeSecret.put = sinon.stub().resolves()
+      kubeNamespaceMock.get = sinon.stub().resolves(fakeNamespace)
+
+      await poller._upsertKubernetesSecret()
+
+      expect(kubeNamespaceMock.secrets.calledWith('fakeSecretName')).to.equal(true)
+
+      expect(kubeSecret.put.calledWith({
         body: {
           apiVersion: 'v1',
           kind: 'Secret',
@@ -746,7 +821,9 @@ describe('Poller', () => {
     it('fails storing secret', async () => {
       const internalErrorServer = new Error('Internal Error Server')
       internalErrorServer.statusCode = 500
-
+      const kubeSecret = sinon.mock()
+      kubeNamespaceMock.secrets = sinon.stub().returns(kubeSecret)
+      kubeSecret.get = sinon.stub().throws({ statusCode: 404 })
       kubeNamespaceMock.secrets.post = sinon.stub().throws(internalErrorServer)
 
       let error
@@ -856,7 +933,7 @@ describe('Poller', () => {
       }
     })
   })
-  describe('nameing conventions', () => {
+  describe('naming conventions', () => {
     let poller
     beforeEach(() => {
       poller = pollerFactory()