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

Use api.ReadBaoVariable everywhere #157

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

cipherboy
Copy link
Member

This is a follow-up to #138; will be rebased when that one merges.


This cleans up the logic of api.ReadBaoVariable a touch, and then replaces a bunch of instances of os.GetEnv with api.ReadBaoVariable instead.

I'm thinking perhaps we want to have an option (BAO_IGNORE_VAULT_VARIABLES) might be useful to globally ignore VAULT_ prefixed environment variables? That way Vault and Bao could coexist on the same system with different environments and not have a partial environment from one leak into the other?

Thoughts?

This also enables usage with non-Bao prefixed variables and prevents
weird reads if one is accidentally used. Further, by using
os.LookupEnv(...), we can return an empty BAO_ prefixed variable
value if it is explicitly set, over returning a populated VAULT_
variable instead.

Similarly, we introduce LookupBaoVariable to handle places where
we need to check if the variable was set as well.

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
This addresses more cases where os.LookupEnv was used instead of
os.GetEnv, so we prefer the Bao-aware api.LookupBaoVariable in these
cases instead.

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
@cipherboy cipherboy force-pushed the use-read-bao-variable-everywhere branch from a7e5e2a to d87417b Compare February 27, 2024 13:49
@naphelps naphelps self-requested a review February 27, 2024 18:06
@naphelps naphelps merged commit aed19f5 into openbao:main Feb 27, 2024
3 of 9 checks passed
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.

None yet

2 participants