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

refactor: Rename vault to backends, start using AuthType interface #59

Merged
merged 5 commits into from
Feb 13, 2021

Conversation

werne2j
Copy link
Member

@werne2j werne2j commented Feb 12, 2021

Description

  • Refactor the code to have a simpler structure as well as reuse code by taking advantage of more interfaces.
  • Remove the vault client and use the vault api client directly
  • Pull config and types into own package

Checklist

Please make sure that your PR fulfills the following requirements:

  • Reviewed the guidelines for contributing to this repository
  • The commit message follows the Conventional Commits Guidelines.
  • Tests for the changes have been updated
  • Docs have been added / updated

Type of Change

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • New tests
  • Build/CI related changes
  • Documentation content changes
  • Other (please describe)

@werne2j werne2j self-assigned this Feb 12, 2021
@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #59 (e95d7b4) into main (5fe0e85) will increase coverage by 1.15%.
The diff coverage is 43.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
+ Coverage   44.73%   45.89%   +1.15%     
==========================================
  Files          13       14       +1     
  Lines         427      438      +11     
==========================================
+ Hits          191      201      +10     
+ Misses        213      209       -4     
- Partials       23       28       +5     
Impacted Files Coverage Δ
cmd/generate.go 35.55% <0.00%> (ø)
cmd/root.go 0.00% <0.00%> (ø)
pkg/auth/ibmsecretmanager/iam.go 0.00% <0.00%> (ø)
pkg/auth/vault/github.go 0.00% <0.00%> (ø)
pkg/kube/template.go 48.33% <0.00%> (ø)
pkg/backends/ibmsecretmanager.go 56.25% <56.25%> (ø)
pkg/auth/vault/approle.go 71.42% <71.42%> (ø)
pkg/backends/vault.go 76.00% <76.00%> (ø)
pkg/config/config.go 86.36% <100.00%> (ø)
pkg/utils/util.go 55.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d59a2de...797da89. Read the comment docs.

Copy link
Member

@jkayani jkayani left a comment

Choose a reason for hiding this comment

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

A Vault configuration has a Backend which can use one of several AuthType's depending on which specific backend it is. Each Backend encapsulates the logic for getting the data into the map[string]interface{} from it's Vault (plain Vault or SecretManager). The AuthType's handle logging in and setting the Vault token in the client.

Great change I think. I didn't read the details too closely so I will rely on your tests and the CI to make sure it's good

@werne2j werne2j marked this pull request as ready for review February 12, 2021 17:48
@werne2j werne2j merged commit 4e8ddfb into main Feb 13, 2021
@werne2j werne2j deleted the vaultRefactor branch February 13, 2021 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants