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.

Sign up for GitHub

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

Jump to bottom

feat: Add Vault provider #35

Merged
merged 25 commits into from
Jan 25, 2024
Merged

feat: Add Vault provider #35

merged 25 commits into from
Jan 25, 2024

Conversation

csatib02
Copy link
Member

@csatib02 csatib02 commented Dec 15, 2023 •
edited
Loading

Overview

  • Configuration support for the file provider.

  • Added support for the Vault provider.

  • Currently, multi-provider functionality (e.g. simultaneous use of one instance of both the file and Vault providers) is not supported.
    Note: A workaround have been applied temporarily, with plans to address this limitation in a future pull request.

  • The Vault provider implementation has been mostly refactored from Vault-env, with the key difference being that all providers now receive a configuration.

  • Secret-init still requires the PROVIDER environment variable to be set, as previously explained.

Continuation of issue: #1
Closes: #18

Changes

  • Reworked environment prefixes, as in: Vault provider implementation #32
  • Introduced common package for application configuration.
  • Implemented args.go and env_store.go to enhance the clarity and testability of the application.
  • Refactored various sections of main.go for better modularization.
  • The file provider now uses a configuration.
  • Implemented the Vault provider.
  • Included tests for both the Vault and file providers.

@csatib02 csatib02 self-assigned this Dec 15, 2023
@github-actions github-actions bot added the size/M Denotes a PR that changes 100-499 lines label Dec 15, 2023
@github-actions github-actions bot added size/L Denotes a PR that changes 500-999 lines and removed size/M Denotes a PR that changes 100-499 lines labels Dec 15, 2023
@github-actions github-actions bot added size/XL Denotes a PR that changes 1000+ lines and removed size/L Denotes a PR that changes 500-999 lines labels Dec 17, 2023
@csatib02
fix(vault provider): linter
1820fed 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
feat(vault provider): Add sanitization
7256c31 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
feat(vault provider): Implementation finished
27223b3 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
fix(go.mod) update dependency
bed0ba8 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
fix(env.go) single provider workaround
a330483 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
fix(env.go, main.go) vault provider secret loading
fb2688c 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
feat(vault provider config test): Add config test
46216ef 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
feat(vault provider tests): Add more tests, and minor fixes
4f7ae51 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
fix(vault provider tests): Minor fixes
b0f4197 
Signed-off-by: Bence Csati <[email protected]>
@csatib02 csatib02 marked this pull request as ready for review December 21, 2023 13:14
adamtagscherer
adamtagscherer reviewed Dec 28, 2023
View reviewed changes
main.go Outdated Show resolved Hide resolved
sagikazarmark
sagikazarmark reviewed Jan 10, 2024
View reviewed changes
env.go Outdated Show resolved Hide resolved
env.go Outdated Show resolved Hide resolved
env.go Outdated Show resolved Hide resolved
provider/file/config.go Outdated Show resolved Hide resolved
provider/file/config.go Outdated Show resolved Hide resolved
@csatib02
fix: fix remarks
21cff0e 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
chore: simplify and improve code
f002f26 
Signed-off-by: Bence Csati <[email protected]>
@csatib02
feat(pkg: args, config, envstore): add new package, factor out main, …
cb9a1c8 
…aswell as providers, add tests for new package

Signed-off-by: Bence Csati <[email protected]>
ramizpolic
ramizpolic requested changes Jan 22, 2024
View reviewed changes
provider/vault/config_test.go Outdated Show resolved Hide resolved
provider/vault/vault.go Show resolved Hide resolved
provider/vault/vault.go Show resolved Hide resolved
pkg/args/args_test.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
provider/vault/vault.go Outdated Show resolved Hide resolved
provider/vault/config.go Outdated Show resolved Hide resolved
provider/vault/vault.go Outdated Show resolved Hide resolved
provider/vault/vault.go Outdated Show resolved Hide resolved
provider/vault/vault.go Outdated Show resolved Hide resolved
ramizpolic
ramizpolic reviewed Jan 22, 2024
View reviewed changes
main.go Outdated Show resolved Hide resolved
ramizpolic
ramizpolic reviewed Jan 22, 2024
View reviewed changes
main.go Outdated Show resolved Hide resolved
ramizpolic
ramizpolic reviewed Jan 22, 2024
View reviewed changes
main.go Show resolved Hide resolved
ramizpolic
ramizpolic requested changes Jan 23, 2024
View reviewed changes
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.

small notes here, sorry for the drill 😢 the logic looks clean, well done on adding tests!

args.go Outdated Show resolved Hide resolved
env_store.go Outdated Show resolved Hide resolved
env_store_test.go Show resolved Hide resolved
env_store_test.go Show resolved Hide resolved
provider/vault/config.go Outdated Show resolved Hide resolved
provider/vault/vault.go Outdated Show resolved Hide resolved
provider/vault/vault.go Outdated Show resolved Hide resolved
provider/vault/vault_test.go Outdated Show resolved Hide resolved
@csatib02
chore: minor fixes
c4d38a3 
Signed-off-by: Bence Csati <[email protected]>
ramizpolic
ramizpolic approved these changes Jan 24, 2024
View reviewed changes
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.

LGTM, great work @csatib02! Some great changes and improvements, especially adding the tests to track everything. This makes it really simple to check and build on top later on.

@csatib02 csatib02 merged commit dcd3918 into bank-vaults:main Jan 25, 2024
16 checks passed
@csatib02 csatib02 deleted the feat/vault-provider branch January 26, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamtagscherer adamtagscherer adamtagscherer left review comments

@ramizpolic ramizpolic ramizpolic approved these changes

@akijakya akijakya Awaiting requested review from akijakya

@sagikazarmark sagikazarmark Awaiting requested review from sagikazarmark

Assignees

@csatib02 csatib02

Labels
size/XL Denotes a PR that changes 1000+ lines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Backwards compatibility support
4 participants
@csatib02 @sagikazarmark @adamtagscherer @ramizpolic