Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: make HSM pin an optional argument to allow setting it via the BANK_VAULTS_HSM_PIN env var #252

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

mark64
Copy link
Contributor

@mark64 mark64 commented Oct 30, 2023

Overview

Previously, the pin had to be provided by the user in their Vault yaml. If you want to keep the pin secret in
source code, then this isn't desirable. Viper defaults to the command line version of the pin, so even if you
set a dummy pin in the yaml and override it via envsConfig, bank-vaults won't use the right pin.

Notes for reviewer

This is my first PR here so please check if I missed things that need to be updated.

@mark64 mark64 requested a review from a team as a code owner October 30, 2023 17:44
@mark64 mark64 requested review from ramizpolic and removed request for a team October 30, 2023 17:44
@ramizpolic
Copy link
Member

Hi @mark64, thanks for raising this! I will look into this one once I am back from PTO, apologies for taking a bit longer. I just need to verify how this specific config flag is passed downstream.

@ramizpolic ramizpolic self-assigned this Dec 22, 2023
…ULTS_HSM_PIN env var

Signed-off-by: Mark Hill <markleehill@gmail.com>
@github-actions github-actions bot added the size/S Denotes a PR that changes 10-99 lines label Jan 27, 2024
@mark64 mark64 changed the title Make HSM pin an optional argument to allow setting it via the BANK_VAULTS_HSM_PIN env var fix: make HSM pin an optional argument to allow setting it via the BANK_VAULTS_HSM_PIN env var Jan 27, 2024
Copy link
Member

@ramizpolic ramizpolic left a comment

Choose a reason for hiding this comment

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

Thanks @mark64!

@ramizpolic ramizpolic merged commit 6396c8d into bank-vaults:main Feb 9, 2024
33 of 34 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
size/S Denotes a PR that changes 10-99 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants