Repository: godaddy/kubernetes-external-secrets
Pull request #194 (2019-11-04 22:07:42)

chore: update log messages to use the same name format: namespace/external-secret-name
Improve log messages to be abit more consistent.
Decreased log level for some to reduce spam (eg addPoller will log on debug level as start will log similar info right after)


Patch:
diff --git a/lib/backends/kv-backend.js b/lib/backends/kv-backend.js
index 6992f7c..676f702 100644
--- a/lib/backends/kv-backend.js
+++ b/lib/backends/kv-backend.js
@@ -25,7 +25,7 @@ class KVBackend extends AbstractBackend {
    */
   _fetchSecretPropertyValues ({ externalData, roleArn }) {
     return Promise.all(externalData.map(async secretProperty => {
-      this._logger.info(`fetching secret property ${secretProperty.name} with role: ${roleArn}`)
+      this._logger.info(`fetching secret property ${secretProperty.name} with role: ${roleArn || 'no role set'}`)
       const value = await this._get({ secretKey: secretProperty.key, roleArn })
 
       if ('property' in secretProperty) {
diff --git a/lib/daemon.js b/lib/daemon.js
index e6a1270..c3afcc2 100644
--- a/lib/daemon.js
+++ b/lib/daemon.js
@@ -39,7 +39,7 @@ class Daemon {
    */
   _removePoller (pollerId) {
     if (this._pollers[pollerId]) {
-      this._logger.info(`stopping and removing poller ${pollerId}`)
+      this._logger.debug(`stopping and removing poller ${pollerId}`)
       this._pollers[pollerId].stop()
       delete this._pollers[pollerId]
     }
@@ -50,7 +50,7 @@ class Daemon {
   }
 
   _addPoller (descriptor) {
-    this._logger.info('spinning up poller for', descriptor.name, 'in', descriptor.namespace)
+    this._logger.debug(`spinning up poller for ${descriptor.namespace}/${descriptor.name}`)
 
     const poller = this._pollerFactory.createPoller(descriptor)
 
diff --git a/lib/poller.js b/lib/poller.js
index cae0f8c..d19523b 100644
--- a/lib/poller.js
+++ b/lib/poller.js
@@ -45,11 +45,10 @@ class Poller {
     this._customResourceManifest = customResourceManifest
 
     this._externalSecret = externalSecret
+    this._secretDescriptor = externalSecret.secretDescriptor
 
     const { name, uid, namespace } = externalSecret.metadata
 
-    this._secretDescriptor = { ...externalSecret.secretDescriptor, name }
-
     this._ownerReference = {
       apiVersion: externalSecret.apiVersion,
       controller: true,
@@ -79,7 +78,7 @@ class Poller {
       apiVersion: 'v1',
       kind: 'Secret',
       metadata: {
-        name: secretDescriptor.name,
+        name: this._name,
         ownerReferences: [
           this._ownerReference
         ]
@@ -94,7 +93,7 @@ class Poller {
    * @returns {Promise} Promise object that always resolves.
    */
   async _poll () {
-    this._logger.info(`running poll on the secret ${this._name}`)
+    this._logger.info(`running poll on the secret ${this._namespace}/${this._name}`)
 
     try {
       await this._upsertKubernetesSecret()
@@ -107,7 +106,7 @@ class Poller {
         status: 'success'
       })
     } catch (err) {
-      this._logger.error(err, `failure while polling the secret ${this._name}`)
+      this._logger.error(err, `failure while polling the secret ${this._namespace}/${this._name}`)
       await this._updateStatus(`ERROR, ${err.message}`)
 
       this._metrics.observeSync({
@@ -131,11 +130,11 @@ class Poller {
     const verdict = this._isPermitted(ns.body, this._secretDescriptor)
 
     if (!verdict.allowed) {
-      throw (new Error(`not allowed to fetch secret: ${this._name}: ${verdict.reason}`))
+      throw (new Error(`not allowed to fetch secret: ${this._namespace}/${this._name}: ${verdict.reason}`))
     }
 
     const secretManifest = await this._createSecretManifest()
-    this._logger.info(`upserting secret ${this._name} in ${this._namespace}`)
+    this._logger.info(`upserting secret ${this._namespace}/${this._name}`)
 
     try {
       return await kubeNamespace.secrets.post({ body: secretManifest })
@@ -146,6 +145,7 @@ class Poller {
   }
 
   async _updateStatus (status) {
+    this._logger.debug(`updating status for ${this._namespace}/${this._name} to: ${status}`)
     await this._status.put({
       body: {
         ...this._externalSecret,
@@ -224,7 +224,7 @@ class Poller {
 
       return this._setNextPoll(nextPollIn)
     } catch (err) {
-      this._logger.error(err, 'status check went boom for %s in %s', this._name, this._namespace)
+      this._logger.error(err, `status check went boom for ${this._namespace}/${this._name}`)
     }
   }
 
@@ -239,7 +239,7 @@ class Poller {
     }
 
     this._timeoutId = setTimeout(this._poll.bind(this), nextPollIn)
-    this._logger.debug('Next poll for %s in %s in %s', this._name, this._namespace, nextPollIn)
+    this._logger.debug(`next poll for ${this._namespace}/${this._name} in ${nextPollIn} ms`)
   }
 
   /**
@@ -250,7 +250,7 @@ class Poller {
   start () {
     if (this._timeoutId) return this
 
-    this._logger.info(`starting poller on the secret ${this._name}`)
+    this._logger.info(`starting poller for ${this._namespace}/${this._name}`)
     this._scheduleNextPoll()
 
     return this
@@ -263,7 +263,7 @@ class Poller {
   stop () {
     if (!this._timeoutId) return this
 
-    this._logger.info(`stopping poller on the secret ${this._name}`)
+    this._logger.info(`stopping poller for ${this._namespace}/${this._name}`)
 
     clearTimeout(this._timeoutId)
     this._timeoutId = null


Test patch:
diff --git a/lib/backends/kv-backend.test.js b/lib/backends/kv-backend.test.js
index 8db1ff1..21aed28 100644
--- a/lib/backends/kv-backend.test.js
+++ b/lib/backends/kv-backend.test.js
@@ -80,8 +80,8 @@ describe('SecretsManagerBackend', () => {
         }]
       })
 
-      expect(loggerMock.info.calledWith('fetching secret property fakePropertyName1 with role: undefined')).to.equal(true)
-      expect(loggerMock.info.calledWith('fetching secret property fakePropertyName2 with role: undefined')).to.equal(true)
+      expect(loggerMock.info.calledWith('fetching secret property fakePropertyName1 with role: no role set')).to.equal(true)
+      expect(loggerMock.info.calledWith('fetching secret property fakePropertyName2 with role: no role set')).to.equal(true)
       expect(kvBackend._get.calledWith({
         secretKey: 'fakePropertyKey1',
         roleArn: undefined
diff --git a/lib/daemon.test.js b/lib/daemon.test.js
index fe4fe0b..dc4c41b 100644
--- a/lib/daemon.test.js
+++ b/lib/daemon.test.js
@@ -15,6 +15,7 @@ describe('Daemon', () => {
   beforeEach(() => {
     loggerMock = sinon.mock()
     loggerMock.info = sinon.stub()
+    loggerMock.debug = sinon.stub()
 
     pollerMock = sinon.mock()
     pollerMock.start = sinon.stub().returns(pollerMock)
diff --git a/lib/poller.test.js b/lib/poller.test.js
index 1261001..e6f37ad 100644
--- a/lib/poller.test.js
+++ b/lib/poller.test.js
@@ -218,7 +218,7 @@ describe('Poller', () => {
       poller._upsertKubernetesSecret.resolves()
 
       await poller._poll()
-      expect(loggerMock.info.calledWith(`running poll on the secret ${poller._secretDescriptor.name}`)).to.equal(true)
+      expect(loggerMock.info.calledWith(`running poll on the secret ${poller._namespace}/${poller._name}`)).to.equal(true)
 
       expect(metricsMock.observeSync.getCall(0).args[0]).to.deep.equal({
         name: 'fakeSecretName',
@@ -241,7 +241,7 @@ describe('Poller', () => {
         backend: 'fakeBackendType',
         status: 'error' })
       expect(poller._updateStatus.calledWith(`ERROR, ${error.message}`)).to.equal(true)
-      expect(loggerMock.error.calledWith(error, `failure while polling the secret ${poller._secretDescriptor.name}`)).to.equal(true)
+      expect(loggerMock.error.calledWith(error, `failure while polling the secret ${poller._namespace}/${poller._name}`)).to.equal(true)
     })
   })
 
@@ -358,7 +358,7 @@ describe('Poller', () => {
       externalSecretsApiMock.status.get = sinon.stub().throws(error)
 
       await poller._scheduleNextPoll()
-      expect(loggerMock.error.calledWith(error, 'status check went boom for %s in %s', 'fakeSecretName', 'fakeNamespace')).to.equal(true)
+      expect(loggerMock.error.calledWith(error, 'status check went boom for fakeNamespace/fakeSecretName')).to.equal(true)
     })
   })
 
@@ -457,7 +457,7 @@ describe('Poller', () => {
       }
 
       expect(error).to.not.equal(undefined)
-      expect(error.message).equals('not allowed to fetch secret: fakeSecretName: namspace does not allow to assume role arn:aws:iam::123456789012:role/test-role')
+      expect(error.message).equals('not allowed to fetch secret: fakeNamespace/fakeSecretName: namspace does not allow to assume role arn:aws:iam::123456789012:role/test-role')
     })
 
     it('fails storing secret', async () => {
@@ -496,7 +496,7 @@ describe('Poller', () => {
 
       poller.start()
 
-      expect(loggerMock.info.calledWith(`starting poller on the secret ${poller._secretDescriptor.name}`)).to.equal(true)
+      expect(loggerMock.info.calledWith(`starting poller for ${poller._namespace}/${poller._name}`)).to.equal(true)
       expect(poller._scheduleNextPoll.called).to.equal(true)
     })
   })
@@ -516,7 +516,7 @@ describe('Poller', () => {
 
       poller.stop()
 
-      expect(loggerMock.info.calledWith(`stopping poller on the secret ${poller._secretDescriptor.name}`)).to.equal(true)
+      expect(loggerMock.info.calledWith(`stopping poller for ${poller._namespace}/${poller._name}`)).to.equal(true)
       expect(poller._timeoutId).to.equal(null)
     })
   })