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

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

Conversation

ninadpage
Copy link
Contributor

Changes

  • 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

Background

@ninadpage ninadpage force-pushed the feature/is-binary-general-implementation branch from e51951a to 5024f21 Compare December 22, 2020 00:55
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.

Looks great! Thanks for this contribution! 👍
I made a small comment. Since there's some logic on determining if isBinary is true or false, it would be nice to have a test which explicitly sets it as false.

lib/backends/kv-backend.test.js Show resolved Hide resolved
@Flydiverny
Copy link
Member

Flydiverny commented Dec 22, 2020

Looks like e2e tests are failing due to using old repo URLs I'll see if I can amend that. (fixed by #586)

@ninadpage ninadpage force-pushed the feature/is-binary-general-implementation branch from 5024f21 to 40e7583 Compare December 22, 2020 01:45
- 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
@ninadpage ninadpage force-pushed the feature/is-binary-general-implementation branch from 40e7583 to fccf502 Compare December 22, 2020 01:50
@ninadpage ninadpage force-pushed the feature/is-binary-general-implementation branch from 9b2511b to 297fa4f Compare December 22, 2020 02:02
@Flydiverny Flydiverny merged commit e138a28 into external-secrets:master Dec 22, 2020
@Flydiverny Flydiverny mentioned this pull request Dec 22, 2020
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