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

Cleaning up connection and v2 secrets #18

Closed

Conversation

john-delivuk
Copy link

@john-delivuk john-delivuk commented Mar 24, 2021

We're currently experimenting with viperx / vault provider. Over the course of working with this the last few days I've noticed a few things I addressed in this PR and I wanted to share. I don't think this is complete at this time, but wanted to open it for feedback. I'd like to move in the same direction as the project rather than fork if possible.

High level changes

  1. Relying on the Vault SDK for the connection values rather than parsing. My opinion is we should allow overriding but I think we should change the interface for AddRemoteProvider method to accept a config struct for the provider, that it will attach to that provider instantiation.
    Whoops, just realized this is part of Viper proper. Thinking about what a better solution would be.
  2. Allowing parsing of both kv2 structs and kv1 as well as all other secrets engines. In all other except kv2, your response is like the following
{
	"data": {
		"serect1": "somethingsecret",
		"serect2": "somethingelsesecret"
	}
}

kv2 looks like

{
	"metadata": {
		"version": 1
	},
	"data": {
		"data": {
			"serect1": "somethingsecret",
			"serect2": "somethingelsesecret"
		}
	}
}

Right now I'm making that choice based on if data is found, but I think it would be best to validate both data and metadata are present before assuming kv2.

Would love your feedback on these, or gauge your interest on moving forward. Happy to help with making this a more powerful plugin.

@john-delivuk
Copy link
Author

due to the inactivity here we're going to fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant