Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Vault: stringify JSON response for compatibility with KV backend #214

Merged
merged 1 commit into from
Nov 14, 2019

Conversation

Pluies
Copy link
Contributor

@Pluies Pluies commented Nov 13, 2019

Hello, it's me again 👋

Due to changes in #206,
we now need to stringify the response from the backend which currently comes as
an object rather than a JSON string.

Fixes issue such as:

{"level":30,"time":1573653228258,"pid":19,"hostname":"external-secrets-8zq4bq","msg":"running poll on the secret example/example","v":1}
{"level":40,"time":1573653228468,"pid":19,"hostname":"external-secrets-8zq4bq","msg":"Failed to JSON.parse value for 'secrets/data/example/credentials', please verify that your secret value is correctly formatted as JSON. To use plain text secret remove the 'property: apiToken'","v":1}

@Pluies
Copy link
Contributor Author

Pluies commented Nov 13, 2019

cc @Flydiverny

Copy link
Member

@Flydiverny Flydiverny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, doh! can you update the test so the build passes ?

Due to changes in external-secrets#206,
we now need to stringify the response from the backend which currently comes as
an object rather than a JSON string.

Fixes issue such as:

```
{"level":30,"time":1573653228258,"pid":19,"hostname":"external-secrets-8zq4bq","msg":"running poll on the secret example/example","v":1}
{"level":40,"time":1573653228468,"pid":19,"hostname":"external-secrets-8zq4bq","msg":"Failed to JSON.parse value for 'secrets/data/example/credentials', please verify that your secret value is correctly formatted as JSON. To use plain text secret remove the 'property: apiToken'","v":1}
```
@Pluies
Copy link
Contributor Author

Pluies commented Nov 14, 2019

can you update the test so the build passes ?

Eh! My turn to "doh" :)

Done on this PR; and I'm working on tests that will run vaultBackend.getSecretManifestData to test more than the (fairly straightforward) inner loop of _get. I also want to include tests with broken data or failed responses from Vault to test edge cases and ensure they're handled correctly. I'll open a separate PR once this is ready 👍

@Flydiverny Flydiverny merged commit 5527530 into external-secrets:master Nov 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants