feat: add general support for isBinary for all backends (#585)

* feat: add general support for isBinary for all backends

- Add support for `isBinary` in `KVBackend` + tests
- Remove specific implementation of `isBinary` from Azure Key Vault & GCP Secrets Manager backends
- Update description for `isBinary` field in the CRD to remove Azure-specific details
- Update docs

* chore: add test for isBinary explicitly set to false

Author: ninadpage

README.md

@@ -443,23 +443,6 @@ spec:
       name: password
 ```
 
-Due to the way Azure handles binary files, you need to explicitly let the ExternalSecret know that the secret is binary.
-You can do that with the `isBinary` field on the key. This is necessary for certificates and other secret binary files.
-
-```yml
-apiVersion: kubernetes-client.io/v1
-kind: ExternalSecret
-metadata:
-  name: hello-keyvault-service
-spec:
-  backendType: azureKeyVault
-  keyVaultName: hello-world
-  data:
-    - key: hello-service/credentials
-      name: password
-      isBinary: true
-```
-
 ### Alibaba Cloud KMS Secret Manager
 
 kubernetes-external-secrets supports fetching secrets from [Alibaba Cloud KMS Secret Manager](https://www.alibabacloud.com/help/doc-detail/152001.htm)
@@ -623,6 +606,33 @@ To retrieve an individual secret's content, use the following where "mysecret" i
 
 The secrets will persist even if the helm installation is removed, although they will no longer sync to Google Secret Manager.
 
+## Binary Secrets
+Most backends do not treat binary secrets any differently than text secrets. Since you typically store a binary secret as a base64-encoded string in the backend, you need to explicitly let the ExternalSecret know that the secret is binary, otherwise it will be encoded in base64 again.
+You can do that with the `isBinary` field on the key. This is necessary for certificates and other secret binary files.
+
+```yml
+apiVersion: kubernetes-client.io/v1
+kind: ExternalSecret
+metadata:
+  name: hello-service
+spec:
+  backendType: anySupportedBackend
+  # ...
+  data:
+    - key: hello-service/archives/secrets_zip
+      name: secrets.zip
+      isBinary: true  # Default: false
+    # also works with `property`
+    - key: hello-service/certificates
+      name: cert.p12
+      property: cert.p12
+      isBinary: true
+```
+
+AWS Secrets Manager is a notable exception to this. If you create/update a secret using [SecretBinary](https://docs.aws.amazon.com/secretsmanager/latest/apireference/API_CreateSecret.html#API_CreateSecret_RequestSyntax) parameter of the API, then AWS API will return the secret data as `SecretBinary` in the response and ExternalSecret will handle it accordingly. In that case, you do not need to use the `isBinary` field.
+
+Note that `SecretBinary` parameter is not available when using the AWS Secrets Manager console. For any binary secrets (represented by a base64-encoded strings) created/updated via the AWS console, or stored in key-value pairs instead of text strings, you can just use the `isBinary` field explicitly as above.
+
 ## Metrics
 
 kubernetes-external-secrets exposes the following metrics over a prometheus endpoint:

charts/kubernetes-external-secrets/crds/kubernetes-client.io_externalsecrets_crd.yaml

@@ -83,9 +83,10 @@ spec:
                     description: Property to extract if secret in backend is a JSON object
                   isBinary:
                     description: >-
-                      You must set this to true if configuring an item for a binary file stored in Azure KeyVault.
-                      Azure automatically base64 encodes binary files and setting this to true ensures External Secrets
-                      does not base64 encode the base64 encoded binary files.
+                      Whether the backend secret shall be treated as binary data
+                      represented by a base64-encoded string. You must set this to true
+                      for any base64-encoded binary data in the backend - to ensure it
+                      is not encoded in base64 again. Default is false.
                     type: boolean
                 required:
                   - name

examples/hello-service-external-secret-gcp.yml

@@ -15,5 +15,5 @@ spec:
       property: value
       # Version of the secret (default: 'latest')
       version: 1
-      # If the secret is encoded in base64 then decodes it (default: false)
+      # If the secret is already encoded in base64, then sends it unchanged (default: false)
       isBinary: false

examples/hello-service-external-secret-vault.yml

@@ -11,3 +11,7 @@ spec:
     - name: password
       key: secret/data/hello-service/password
       property: password
+    - name: cert.p12
+      key: secret/data/hello-service/certificates
+      property: cert.p12
+      isBinary: true # defaults to false

lib/backends/azure-keyvault-backend.js

@@ -26,18 +26,13 @@ class AzureKeyVaultBackend extends KVBackend {
    * Get secret property value from Azure Key Vault.
    * @param {string} key - Key used to store secret property value in Azure Key Vault.
    * @param {string} specOptions.keyVaultName - Name of the azure key vault
-   * @param {string} keyOptions.isBinary - Does the secret contain a binary? Set to "true" to handle as binary. Does not work with "property"
    * @returns {Promise} Promise object representing secret property value.
    */
 
-  async _get ({ key, keyOptions, specOptions: { keyVaultName } }) {
+  async _get ({ key, specOptions: { keyVaultName } }) {
     const client = this._keyvaultClient({ keyVaultName })
     this._logger.info(`fetching secret ${key} from Azure KeyVault ${keyVaultName}`)
     const secret = await client.getSecret(key)
-    // Handle binary files, since the Azure client does not
-    if (keyOptions && keyOptions.isBinary) {
-      return Buffer.from(secret.value, 'base64')
-    }
     return secret.value
   }
 }

lib/backends/gcp-secrets-manager-backend.js

@@ -44,13 +44,7 @@ class GCPSecretsManagerBackend extends KVBackend {
     const [version] = await this._client.accessSecretVersion({
       name: 'projects/' + projectId + '/secrets/' + key + '/versions/' + secretVersion
     })
-    const secret = version.payload.data.toString('utf8')
-    // Handle binary files - this is useful when you've stored a base64 encoded string
-    if (keyOptions && keyOptions.isBinary) {
-      return Buffer.from(secret, 'base64')
-    }
-
-    return secret
+    return version.payload.data.toString('utf8')
   }
 }
 

lib/backends/kv-backend.js

@@ -20,6 +20,8 @@ class KVBackend extends AbstractBackend {
    * @param {string} data[].name - Kubernetes Secret property name.
    * @param {string} data[].property - If the backend secret is an
    *   object, this is the property name of the value to use.
+   * @param {string} data[].isBinary - If the backend secret shall be treated
+   *   as binary data represented by a base64-encoded string. Defaults to false.
    * @param {Object} specOptions - Options set on spec level.
    * @returns {Promise} Promise object representing secret property values.
    */
@@ -28,6 +30,7 @@ class KVBackend extends AbstractBackend {
       const { name, property = null, key, ...keyOptions } = dataItem
       const plainOrObjValue = await this._get({ key, keyOptions, specOptions })
       const shouldParseValue = 'property' in dataItem
+      const isBinary = 'isBinary' in dataItem && dataItem.isBinary === true
 
       let value = plainOrObjValue
       if (shouldParseValue) {
@@ -48,6 +51,16 @@ class KVBackend extends AbstractBackend {
         value = parsedValue[property]
       }
 
+      if (isBinary) {
+        // value in the backend is binary data which is already encoded in base64.
+        if (typeof value === 'string') {
+          // Skip this step if the value from the backend is not a string (e.g., AWS
+          // SecretsManager will already return a `Buffer` with base64 encoding if the
+          // secret contains `SecretBinary` instead of `SecretString`).
+          value = Buffer.from(value, 'base64')
+        }
+      }
+
       return { [name]: value }
     }))
   }

lib/backends/kv-backend.test.js

@@ -103,6 +103,42 @@ describe('kv-backend', () => {
       expect(secretPropertyValues).deep.equals([{ fakePropertyName1: 'fakePropertyValue1' }, { fakePropertyName2: 'fakePropertyValue2' }])
     })
 
+    it('handles binary values', async () => {
+      kvBackend._get.onCall(0).resolves('YmluYXJ5Cg==') // base64 of binary
+      kvBackend._get.onCall(1).resolves('stringPropertyValue1')
+      kvBackend._get.onCall(2).resolves('{"stringPropertyKey2": "stringPropertyValue2", "binaryPropertyKey2": "YmluYXJ5Cg=="}')
+      kvBackend._get.onCall(3).resolves('{"stringPropertyKey2": "stringPropertyValue2", "binaryPropertyKey2": "YmluYXJ5Cg=="}')
+
+      const secretPropertyValues = await kvBackend._fetchDataValues({
+        data: [{
+          key: 'binaryPropertyKey1',
+          name: 'binaryPropertyName1',
+          isBinary: true
+        }, {
+          key: 'stringPropertyKey1',
+          name: 'stringPropertyName1'
+          // isBinary: false
+        }, {
+          key: 'jsonProperties',
+          name: 'stringPropertyName2',
+          // isBinary: false,
+          property: 'stringPropertyKey2'
+        }, {
+          key: 'jsonProperties',
+          name: 'binaryPropertyName2',
+          isBinary: true,
+          property: 'binaryPropertyKey2'
+        }]
+      })
+
+      expect(secretPropertyValues).deep.equals(
+        [{ binaryPropertyName1: Buffer.from('YmluYXJ5Cg==', 'base64') }, // base64 of binary (unchanged)
+          { stringPropertyName1: 'stringPropertyValue1' },
+          { stringPropertyName2: 'stringPropertyValue2' },
+          { binaryPropertyName2: Buffer.from('YmluYXJ5Cg==', 'base64') } // base64 of binary (unchanged)
+        ])
+    })
+
     it('fetches secret property values using the specified role', async () => {
       kvBackend._get.onFirstCall().resolves('fakePropertyValue1')
       kvBackend._get.onSecondCall().resolves('fakePropertyValue2')
@@ -422,8 +458,11 @@ describe('kv-backend', () => {
   })
 
   describe('base64 encoding', () => {
-    it('handles json objects', async () => {
+    beforeEach(() => {
       kvBackend._get = sinon.stub()
+    })
+
+    it('handles json objects', async () => {
       kvBackend._get
         .resolves(JSON.stringify({
           textProperty: 'text',
@@ -446,5 +485,44 @@ describe('kv-backend', () => {
         jsonProperty: 'eyJzb21lS2V5Ijp7Im15VGV4dCI6InRleHQifX0=' // base 64 value of: {"someKey":{"myText":"text"}}
       })
     })
+
+    it('handles different types of binary data returned by backends', async () => {
+      kvBackend._get.onCall(0).resolves(Buffer.from('YmluYXJ5Cg==', 'base64')) // base64 of binary as a Buffer
+      kvBackend._get.onCall(1).resolves(Buffer.from('YmluYXJ5Cg==', 'base64')) // base64 of binary as a Buffer
+      kvBackend._get.onCall(2).resolves('YmluYXJ5Cg==') // base64 of binary as String
+      kvBackend._get.onCall(3).resolves('test')
+      // e.g. AWS Secrets Manager will return `SecretBinary` as a Buffer and `SecretString` as a String
+
+      const manifestData = await kvBackend.getSecretManifestData({
+        spec: {
+          data: [{
+            key: 'binaryPropertyKey1',
+            name: 'binaryPropertyName1',
+            isBinary: true
+          }, {
+            key: 'binaryPropertyKey2',
+            name: 'binaryPropertyName2'
+            // isBinary: false, but will have no impact if the backend returns a Buffer
+          }, {
+            key: 'stringPropertyKey3',
+            name: 'stringPropertyName3'
+            // isBinary: false,
+            // must be set to true to ensure base64-encoded string in the backend
+            // is not encoded in base64 again
+          }, {
+            key: 'stringPropertyKey4',
+            name: 'stringPropertyName4',
+            isBinary: false // explicitly set false
+          }]
+        }
+      })
+
+      expect(manifestData).deep.equals({
+        binaryPropertyName1: 'YmluYXJ5Cg==', // base64 of binary (unchanged)
+        binaryPropertyName2: 'YmluYXJ5Cg==', // base64 of binary (unchanged)
+        stringPropertyName3: 'WW1sdVlYSjVDZz09', // base64 of base64 of binary
+        stringPropertyName4: 'dGVzdA==' // base64 of test
+      })
+    })
   })
 })