-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: add e2e test for docker auth with _json_key #132
Conversation
Signed-off-by: Devin Christensen <[email protected]>
182a1bf
to
ba2ddbe
Compare
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.
Thanks for adding this, the reason why the test fails, is because you have to add: secrets-webhook.security.bank-vaults.io/provider: vault
to the annotations.
- added correct annotations to the secret - pulled duplicated types out to shared context Signed-off-by: Devin Christensen <[email protected]>
8ac8c92
to
d247832
Compare
@csatib02, test is still failing because the unwrapped value from vault is not being base64 encoded: --- Expected
+++ Actual
@@ -1 +1,5 @@
-X2pzb25fa2V5OiB7CiAgInR5cGUiOiAic2VydmljZV9hY2NvdW50IiwKICAicHJvamVjdF9pZCI6ICJ0ZXN0Igp9Cg==
+_json_key: {
+ "type": "service_account",
+ "project_id": "test"
+}
+ i was looking in the docker source code to try to confirm that they're always treating the value of auth as a base64 encoded string, but didn't find anything definitive. |
Visiting the code I noticed a mistake in the If you change the two functions to the following, then the tests will pass. // assembleCredentialData assembles the credential data that will be retrieved from Vault
func AssembleCredentialData(authCreds map[string]string) (map[string]string, error) {
if username, ok := authCreds["username"]; ok {
if password, ok := authCreds["password"]; ok {
return map[string]string{
"username": username,
"password": password,
}, nil
}
}
if auth, ok := authCreds["auth"]; ok {
return map[string]string{
"auth": auth,
}, nil
}
return nil, fmt.Errorf("no valid credentials found")
}
// assembleDockerAuthConfig assembles the DockerAuthConfig from the retrieved data from Vault
func AssembleDockerAuthConfig(dcCreds map[string]string) DockerAuthConfig {
if username, ok := dcCreds["username"]; ok {
if password, ok := dcCreds["password"]; ok {
return DockerAuthConfig{
Username: username,
Password: password,
Auth: base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", username, password))),
}
}
}
if auth, ok := dcCreds["auth"]; ok {
return DockerAuthConfig{
Auth: base64.StdEncoding.EncodeToString([]byte(auth)),
}
}
return DockerAuthConfig{}
} |
Signed-off-by: Devin Christensen <[email protected]>
61c85ed
to
17e4aef
Compare
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.
LGTM, thank you for working on this! 🚀
Overview
@csatib02, i've add the e2e tests from bank-vaults/vault-secrets-webhook#494. however, they're currently failing:
or
So it's not currently unwrapping the value. I can look into this more later, but figured you might know immediately what the issue is.