chore: update log messages to use the same name format: namespace/external-secret-name (#194)

* chore: update log messages to use the same name format: namespace/external-secret-name
* chore: improve log message when no role is set
* fix: eliminate extended version of secretDescriptor

Author: Flydiverny

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) {

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

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)
 

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)

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

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)
     })
   })