Repository: godaddy/kubernetes-external-secrets
Pull request #292 (2020-02-28 14:18:25)

Fixed dataFrom property in naming convention verification

Flydiverny: #291 
Strange-Account: What test case(s) do you mean?
Flydiverny: I think adding a variation or two to the existing test case for naming conventions
https://github.com/godaddy/kubernetes-external-secrets/blob/master/lib/poller.test.js#L757

to make sure it tests the codepath for `dataFrom` usage also and not only the `data` variant, so they work independently, and combined.


Patch:
diff --git a/lib/poller.js b/lib/poller.js
index ea61251..ac6a336 100644
--- a/lib/poller.js
+++ b/lib/poller.js
@@ -161,6 +161,9 @@ class Poller {
   }
 
   async _updateStatus (status) {
+    // Remove newlines, spaces, and tabs from status string
+    status = status.replace(/(\r\n|\n|\r| (?= ))/gm, '').replace(/\t/g, ' ')
+
     try {
       this._logger.debug(`updating status for ${this._namespace}/${this._name} to: ${status}`)
       await this._status.put({
@@ -203,12 +206,28 @@ class Poller {
     const externalData = descriptor.data || descriptor.properties
     const namingConvention = namespace.metadata.annotations[this._namingPermittedAnnotation]
 
-    if (namingConvention) {
+    // Testing data property
+    if (namingConvention && externalData) {
       externalData.forEach((secretProperty, index) => {
         const reNaming = new RegExp(namingConvention)
         if (!reNaming.test(secretProperty.key)) {
           allowed = false
-          reason = `key name does not match naming convention ${namingConvention}`
+          reason = `key name ${secretProperty.key} does not match naming convention ${namingConvention}`
+          return {
+            allowed, reason
+          }
+        }
+      })
+    }
+
+    // Testing DataFrom property
+    const externalDataFrom = descriptor.dataFrom
+    if (namingConvention && externalDataFrom) {
+      externalDataFrom.forEach((secretProperty, index) => {
+        const reNaming = new RegExp(namingConvention)
+        if (!reNaming.test(secretProperty)) {
+          allowed = false
+          reason = `key name ${secretProperty} does not match naming convention ${namingConvention}`
           return {
             allowed, reason
           }
diff --git a/package-lock.json b/package-lock.json
index d912e1d..20efda9 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -783,20 +783,46 @@
       "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz",
       "integrity": "sha1-x57Zf380y48robyXkLzDZkdLS3k="
     },
+    "atomic-sleep": {
+      "version": "1.0.0",
+      "resolved": "https://registry.npmjs.org/atomic-sleep/-/atomic-sleep-1.0.0.tgz",
+      "integrity": "sha512-kNOjDqAh7px0XWNI+4QbzoiR/nTkHAWNud2uvnJquD1/x5a7EQZMJT0AczqK0Qn67oY/TTQ1LbUKajZpp3I9tQ=="
+    },
     "aws-sdk": {
-      "version": "2.575.0",
-      "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.575.0.tgz",
-      "integrity": "sha512-GgqiThKKmN9CJcGDApJq+TuHkCYIx7A7QF01KPCn2nvxC5efrrJt/0GtrPXe+yJlKh3cl1HQTVobWiSfMV/xhA==",
-      "requires": {
-        "buffer": "^4.9.1",
-        "events": "^1.1.1",
-        "ieee754": "^1.1.13",
-        "jmespath": "^0.15.0",
-        "querystring": "^0.2.0",
-        "sax": "^1.2.1",
-        "url": "^0.10.3",
-        "uuid": "^3.3.2",
-        "xml2js": "^0.4.19"
+      "version": "2.628.0",
+      "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.628.0.tgz",
+      "integrity": "sha512-k0yQqqgTuwCWqPvLe09vGGSJ88oKaWPPCC6uyxQKv7pPQcz9cFZjmjLe6QdGJZeCmeb3rGIZlG+N4Z+9LnwCNg==",
+      "requires": {
+        "buffer": "4.9.1",
+        "events": "1.1.1",
+        "ieee754": "1.1.13",
+        "jmespath": "0.15.0",
+        "querystring": "0.2.0",
+        "sax": "1.2.1",
+        "url": "0.10.3",
+        "uuid": "3.3.2",
+        "xml2js": "0.4.19"
+      },
+      "dependencies": {
+        "sax": {
+          "version": "1.2.1",
+          "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.1.tgz",
+          "integrity": "sha1-e45lYZCyKOgaZq6nSEgNgozS03o="
+        },
+        "xml2js": {
+          "version": "0.4.19",
+          "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.19.tgz",
+          "integrity": "sha512-esZnJZJOiJR9wWKMyuvSE1y6Dq5LCuJanqhxslH2bxM6duahNZ+HMpCLhBQGZkbX6xRf8x1Y2eJlgt2q3qo49Q==",
+          "requires": {
+            "sax": ">=0.6.0",
+            "xmlbuilder": "~9.0.1"
+          }
+        },
+        "xmlbuilder": {
+          "version": "9.0.7",
+          "resolved": "https://registry.npmjs.org/xmlbuilder/-/xmlbuilder-9.0.7.tgz",
+          "integrity": "sha1-Ey7mPS7FVlxVfiD0wi35rKaGsQ0="
+        }
       }
     },
     "aws-sign2": {
@@ -959,9 +985,9 @@
       "dev": true
     },
     "buffer": {
-      "version": "4.9.2",
-      "resolved": "https://registry.npmjs.org/buffer/-/buffer-4.9.2.tgz",
-      "integrity": "sha512-xq+q3SRMOxGivLhBNaUdC64hDTQwejJ+H0T/NB1XMtTVEwNTrfFF3gAxiyW0Bu/xWEGhjVKgUcMhCrUy2+uCWg==",
+      "version": "4.9.1",
+      "resolved": "https://registry.npmjs.org/buffer/-/buffer-4.9.1.tgz",
+      "integrity": "sha1-bRu2AbB6TvztlwlBMgkwJ8lbwpg=",
       "requires": {
         "base64-js": "^1.0.2",
         "ieee754": "^1.1.4",
@@ -2423,7 +2449,7 @@
     },
     "events": {
       "version": "1.1.1",
-      "resolved": "http://registry.npmjs.org/events/-/events-1.1.1.tgz",
+      "resolved": "https://registry.npmjs.org/events/-/events-1.1.1.tgz",
       "integrity": "sha1-nr23Y1rQmccNzEwqH1AEKI6L2SQ="
     },
     "execa": {
@@ -5601,9 +5627,9 @@
       }
     },
     "pino": {
-      "version": "5.13.6",
-      "resolved": "https://registry.npmjs.org/pino/-/pino-5.13.6.tgz",
-      "integrity": "sha512-cYhvK4lopLnW/OtU3uRqBNsPiBt0Cdv97LGJg4yPGbGajlOjWWaODwD66q3Ynqnj6bSNw8HSofYcEOKZqh4Gcg==",
+      "version": "5.16.0",
+      "resolved": "https://registry.npmjs.org/pino/-/pino-5.16.0.tgz",
+      "integrity": "sha512-k9cDzHd9S/oYSQ9B9g9+7RXkfsZX78sQXERC8x4p2XArECZXULx9nqNwZvJHsLj779wPCt+ybN+dG8jFR70p6Q==",
       "requires": {
         "fast-redact": "^2.0.0",
         "fast-safe-stringify": "^2.0.7",
@@ -6250,10 +6276,11 @@
       }
     },
     "sonic-boom": {
-      "version": "0.7.6",
-      "resolved": "https://registry.npmjs.org/sonic-boom/-/sonic-boom-0.7.6.tgz",
-      "integrity": "sha512-k9E2QQ4zxuVRLDW+ZW6ISzJs3wlEorVdmM7ApDgor7wsGKSDG5YGHsGmgLY4XYh4DMlr/2ap2BWAE7yTFJtWnQ==",
+      "version": "0.7.7",
+      "resolved": "https://registry.npmjs.org/sonic-boom/-/sonic-boom-0.7.7.tgz",
+      "integrity": "sha512-Ei5YOo5J64GKClHIL/5evJPgASXFVpfVYbJV9PILZQytTK6/LCwHvsZJW2Ig4p9FMC2OrBrMnXKgRN/OEoAWfg==",
       "requires": {
+        "atomic-sleep": "^1.0.0",
         "flatstr": "^1.0.12"
       }
     },
diff --git a/package.json b/package.json
index 9b49f51..5eb77fa 100644
--- a/package.json
+++ b/package.json
@@ -31,7 +31,7 @@
   "dependencies": {
     "@azure/identity": "^1.0.2",
     "@azure/keyvault-secrets": "^4.0.2",
-    "aws-sdk": "^2.575.0",
+    "aws-sdk": "^2.628.0",
     "express": "^4.17.1",
     "js-yaml": "^3.13.1",
     "json-stream": "^1.0.0",
@@ -40,7 +40,7 @@
     "lodash.merge": "^4.6.2",
     "make-promises-safe": "^5.1.0",
     "node-vault": "^0.9.8",
-    "pino": "^5.13.6",
+    "pino": "^5.16.0",
     "prom-client": "^11.5.3"
   },
   "devDependencies": {


Test patch:
diff --git a/lib/poller.test.js b/lib/poller.test.js
index ce747eb..47df018 100644
--- a/lib/poller.test.js
+++ b/lib/poller.test.js
@@ -405,6 +405,15 @@ describe('Poller', () => {
       expect(error).to.not.equal(undefined)
       expect(error.message).equals('Not Found')
     })
+
+    it('handles odd whitespace and newlines in status', async () => {
+      const poller = pollerFactory()
+
+      await poller._updateStatus('\n\n\nLots      of spaces\n\n\n').then(() => {
+        const statusMessage = externalSecretsApiMock.status.put.getCall(0).args[0].body.status.status
+        expect(statusMessage).to.equal('Lots of spaces')
+      })
+    })
   })
 
   describe('_scheduleNextPoll', () => {
@@ -802,13 +811,78 @@ describe('Poller', () => {
             ]
           },
           permitted: false
+        },
+        {
+          // test regex
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
+          descriptor: {
+            dataFrom: [
+              'dev/team-b/secret'
+            ]
+          },
+          permitted: false
+        },
+        {
+          // empty annotation
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: '' } } },
+          descriptor: {
+            dataFrom: ['test']
+          },
+          permitted: true
+        },
+        {
+          // test regex
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
+          descriptor: {
+            dataFrom: [
+              'dev/team-a/secret'
+            ]
+          },
+          permitted: true
+        },
+        {
+          // test regex
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: '.*' } } },
+          descriptor: {
+            data: [
+              { key: 'whatever', name: 'somethingelse' }
+            ],
+            dataFrom: ['something']
+          },
+          permitted: true
+        },
+        {
+          // test regex data bad, dataFrom OK
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
+          descriptor: {
+            data: [
+              { key: 'dev/team-b/secret', name: 'somethingelse' }
+            ],
+            dataFrom: [
+              'dev/team-a/ok-secret'
+            ]
+          },
+          permitted: false
+        },
+        {
+          // test regex data OK, dataFrom bad
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
+          descriptor: {
+            data: [
+              { key: 'dev/team-a/ok-secret', name: 'somethingelse' }
+            ],
+            dataFrom: [
+              'dev/team-b/bad-secret'
+            ]
+          },
+          permitted: false
         }
       ]
 
       for (let i = 0; i < testcases.length; i++) {
         const testcase = testcases[i]
         const verdict = poller._isPermitted(testcase.ns, testcase.descriptor)
-        expect(verdict.allowed).to.equal(testcase.permitted)
+        expect(verdict.allowed, `test case ${i + 1}`).to.equal(testcase.permitted)
       }
     })
   })