-
Notifications
You must be signed in to change notification settings - Fork 54
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
VEP-1493: Vault Integration For Secrets Storage #1756
Conversation
This VEP proposes integration with Hashicorp Vault for secrets storage. Signed-off-by: Dako Dakov <ddakov@vmware.com>
for more information, see https://pre-commit.ci
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.
I find the proposal well-structured and comprehensive. However, I do have a few suggestions to improve it.
specs/vep-1493-vault-integration/diagrams/high_level_design.puml
Outdated
Show resolved
Hide resolved
specs/vep-1493-vault-integration/diagrams/high_level_design.puml
Outdated
Show resolved
Hide resolved
specs/vep-1493-vault-integration/diagrams/high_level_design.puml
Outdated
Show resolved
Hide resolved
specs/vep-1493-vault-integration/diagrams/high_level_design.puml
Outdated
Show resolved
Hide resolved
Address review feedback. Clarify API/CLI changes. Signed-off-by: Dako Dakov <ddakov@vmware.com>
for more information, see https://pre-commit.ci
Update Component diagram. Signed-off-by: Dako Dakov <ddakov@vmware.com>
Update diagram picture. Signed-off-by: Dako Dakov <ddakov@vmware.com>
…com/vmware/versatile-data-kit into person/ddakov/vep-vault-integration
more updates on the component diagram. Signed-off-by: Dako Dakov <ddakov@vmware.com>
Add alternatives section of the VEP Signed-off-by: Dako Dakov <ddakov@vmware.com>
for more information, see https://pre-commit.ci
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.
Looks good to me. I made some very small (formatting) comments.
My main 3 comments (and those can be addressed in separate PR):
- ) Would vdk secrets CLI be the same as vdk properties (with only difference not allowing values on command line) ? Right ?
- I'd like us to think (and hence describe) a bit about performance, availability and security.
-
Availability. If Vault is down or slow ,
As vault is external service I really recommend implementing read-through cache with fallback (read from storage and on error from cache) . Because during a job run we only will read secrets. And writes would be outside of data job run. That would help keep the SLA of job runs , so in case Vault is unavailable we will rely on the server cache to make sure we still get return the values. As values would tend to be changed fairly rarely - having even hours exipry time (before failing) would be enough . (Spring CacheManager should made this easy). Though the cache would need to be encrypted and should be able to disable it. -
Performance - the secrets will be loaded and cached at teh start of the SDK run job. (the same is currently for properties ), right?
- The SDK API changes are missing . We will add new interface ISecrets in job_input (https://github.com/vmware/versatile-data-kit/blob/main/projects/vdk-core/src/vdk/api/job_input.py) , and then we can re-use basically the whole properties builtin plugin - https://github.com/vmware/versatile-data-kit/tree/main/projects/vdk-core/src/vdk/internal/builtin_plugins/job_properties with difference possibly having SecretsRouter. That's teh direction my thoughts go but other options are ok.
Co-authored-by: Antoni Ivanov <aivanov@vmware.com>
Co-authored-by: Antoni Ivanov <aivanov@vmware.com>
Co-authored-by: Antoni Ivanov <aivanov@vmware.com>
Co-authored-by: Antoni Ivanov <aivanov@vmware.com>
Co-authored-by: Antoni Ivanov <aivanov@vmware.com>
Co-authored-by: Antoni Ivanov <aivanov@vmware.com>
Address review feedback. Clarify API/CLI changes. Signed-off-by: Dako Dakov <ddakov@vmware.com>
for more information, see https://pre-commit.ci
Clarify SDK changes. Signed-off-by: Dako Dakov <ddakov@vmware.com>
…com/vmware/versatile-data-kit into person/ddakov/vep-vault-integration
|
Address review feedback. Signed-off-by: Dako Dakov <ddakov@vmware.com>
for more information, see https://pre-commit.ci
Fix typo. Signed-off-by: Dako Dakov <ddakov@vmware.com>
The final version looks great. |
Add clarification about vault entries. Signed-off-by: Dako Dakov <ddakov@vmware.com>
This VEP proposes integration with Hashicorp Vault for secrets storage.