-
Notifications
You must be signed in to change notification settings - Fork 402
Conversation
@@ -55,10 +57,16 @@ class VaultBackend extends KVBackend { | |||
const secretResponse = await this._client.read(key) | |||
|
|||
if (kvVersion === 1) { | |||
if (keyOptions && keyOptions.isBinary) { | |||
secretResponse.data[property] = Buffer.from(secretResponse.data[property], 'base64').toString('utf8') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels kind of scary 😄 I guess its fine to turn it into a string but feels like we have an underlying problem in the code forcing us to write weird code :)
Main issue is probably that the KV backend expects us to return a string in _get
forcing us to serialise here when we already have an object.
Exactly what i am looking for .. Is there any way to store a base64 encoded value in vault and use external secrets to retrieve it ? |
@@ -55,10 +57,16 @@ class VaultBackend extends KVBackend { | |||
const secretResponse = await this._client.read(key) | |||
|
|||
if (kvVersion === 1) { | |||
if (keyOptions && keyOptions.isBinary) { | |||
secretResponse.data[property] = Buffer.from(secretResponse.data[property], 'base64').toString('utf8') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be enough to not do the toString('utf8')
here, just converting the base64 string to a buffer. later on this buffer will be base64 encoded again as the base class won't mess with buffers. This should put the original value from vault into the secret without it ending up double base64 encoded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs rebase and comments to be addressed. Should try returning a buffer from vault-backends get instead of a binary string.
Closing this in favor of #585 |
No description provided.