Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vault secret backend #310

Merged
merged 28 commits into from
Sep 27, 2019
Merged

Vault secret backend #310

merged 28 commits into from
Sep 27, 2019

Conversation

vnzongzna
Copy link
Contributor

@vnzongzna vnzongzna commented Jun 13, 2019

Fixes issue #105

Proposed Changes

  • Added Vault as a secret backend for kv/kv-v2 secret engine
  • Add Tests

@uberspot uberspot requested review from ramaro and adrianchifor June 13, 2019 12:36
@vnzongzna
Copy link
Contributor Author

@adrianchifor @ramaro review required

@vnzongzna vnzongzna marked this pull request as ready for review July 5, 2019 01:51
@ramaro
Copy link
Member

ramaro commented Jul 30, 2019

@vaibhavk any plan on fixing the items we've reviewed?

@prakashrj
Copy link

@ramaro. Can you please review these new updates?

@uberspot
Copy link
Contributor

uberspot commented Sep 25, 2019

I tested this PR locally btw and no regression popped up.
Two "nice to have"s remain:

  • Use target_name from the inventory in the vault class instead of changing the RefBackend constructor (@ramaro might have a tip for that)
  • Test vaultkv properly in the CI/CD since now the vaultkv: secret isn't in the examples/ so tests don't run on it in Travis.

Other than that the PR looks good.
I'll approve but ideally would like a second approval from @ramaro or @adrianchifor before merging. :)

Copy link
Member

@ramaro ramaro left a comment

Choose a reason for hiding this comment

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

just 2 nitpicks and I think we're good to merge!
Let's also add some docs (other than the kap) after.

@vnzongzna
Copy link
Contributor Author

I'll add the vaultkv information on docs by making a new PR for gh-pages branch

@uberspot
Copy link
Contributor

I'll add the vaultkv information on docs by making a new PR for gh-pages branch

That won't work unfortunately because of permissions. Ideally if you update the docs under docs/ on the master branch I can easily update the gh-pages part. :)

@ramaro ramaro merged commit c207227 into kapicorp:master Sep 27, 2019
@vnzongzna vnzongzna deleted the vault-secret-backend branch September 28, 2019 01:55
@vnzongzna vnzongzna restored the vault-secret-backend branch September 28, 2019 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants