-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add support for azure keyvault backend #87
Conversation
backend/azure_kv.go
Outdated
// There below line was added to access the azure data plane | ||
// Which is required to access secrets in keyvault | ||
|
||
clientCredentialConfig.Resource = "https://vault.azure.net" |
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.
should we make vault.azure.net
a constant, kind of:
azureKVEndpoint = "vault.azure.net"
backend/azure_kv.go
Outdated
|
||
func (c *azureKVClient) ReadSecret(path string, key string) (string, error) { | ||
data := "" | ||
uri := fmt.Sprintf("https://%s.vault.azure.net", c.keyvaultName) |
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.
see my comment above about the constant and lmk what do you think
errors/errors.go
Outdated
@@ -7,6 +7,7 @@ const ( | |||
UnknownErrorType = "UnknownError" | |||
BackendNotImplementedErrorType = "BackendNotImplementedError" | |||
BackendSecretNotFoundErrorType = "BackendSecretNotFoundError" | |||
BackendSecretForbiddenErrorType = "BackendSecretForbiddenError" | |||
K8sSecretNotFoundErrorType = "K8sSecretNotFoundError" | |||
InvalidConfigmapNameErrorType = "InvalidConfigmapNameError" |
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 think this might be a leftover from the migration from configmap to CRDs?
backend/azure_kv.go
Outdated
uri := fmt.Sprintf("https://%s.%s", c.keyvaultName, azureKVEndpoint) | ||
|
||
// TODO: Add support for secret version? | ||
result, err := c.client.GetSecret(context.Background(), uri, path, "") |
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.
let's inject the context that we create in main ?
backend/backend.go
Outdated
const vaultBackendName = "vault" | ||
const azureKVBackendName = "azure-kv" |
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.
const (
vaultBackendName = "vault"
azureKVBackendName = "azure-kv"
)
…ironment variables (native handler)
…ith how code is written
Codecov Report
@@ Coverage Diff @@
## master #87 +/- ##
==========================================
- Coverage 85.26% 81.19% -4.08%
==========================================
Files 9 11 +2
Lines 482 553 +71
==========================================
+ Hits 411 449 +38
- Misses 53 86 +33
Partials 18 18
Continue to review full report at Codecov.
|
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.
Cool stuff 👍 !
LGTM
Status
READY/
Migrations
NO
Description
Add support Azure KeyVault secrets backend.
List of fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
Checklist: