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

added approle auto-auth functionality #97

Closed
wants to merge 1 commit into from

Conversation

rollerd
Copy link
Contributor

@rollerd rollerd commented Mar 3, 2020

  • Adds the ability to use approle auto-auth method in the Vault agent rather than kubernetes
  • Approle credentials are stored as kube secrets
  • Adds the following annotations:
vault.hashicorp.com/agent-auto-auth-method: "approle"
vault.hashicorp.com/agent-approle-secret-name: "<approle_secret_name>"

@tvoran tvoran added enhancement New feature or request injector Area: mutating webhook service labels Mar 4, 2020
@@ -182,6 +188,8 @@ func New(pod *corev1.Pod, patches []*jsonpatch.JsonPatchOperation) (*Agent, erro
ServiceAccountName: saName,
ServiceAccountPath: saPath,
Status: pod.Annotations[AnnotationAgentStatus],
AutoAuthMethod: pod.Annotations[AnnotationAgentAutoAuthMethod],
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a great idea to add support for other auto auth methods. IMO it's better to have AutoAuthConfig which takes json and you can set the params to not have large number of annotations. Thoughts?

@@ -272,6 +280,14 @@ func ShouldInject(pod *corev1.Pod) (bool, error) {
func (a *Agent) Patch() ([]byte, error) {
var patches []byte

// Add our vault-approle-secrets volume
if a.AutoAuthMethod == "approle" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to abstract this and have an interface that has two methods GetAutoAuthPatches and GetAutoAuthConfig

config_map["role_id_file_path"] = "/vault/approle/roleid"
config_map["secret_id_file_path"] = "/vault/approle/secretid"
config_map["remove_secret_id_file_after_reading"] = false
case "kubernetes":
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be replaced by the GetAutoAuthConfig

Have two structs implement this interface KubernetesAutoAuth and AppRoleAutoAuth.

@rollerd
Copy link
Contributor Author

rollerd commented Mar 9, 2020

@smitthakkar96 - thanks very much for the feedback! I'm hoping to have some time this week to dig in and make the suggested changes.

@rollerd
Copy link
Contributor Author

rollerd commented Apr 2, 2020

Closing per the conversation and better options discussed in issue #115

@rollerd rollerd closed this Apr 2, 2020
@adawalli
Copy link

Really appreciate the work you did on #119 @rollerd - I still wish that we could have also had the option for annotations. The only change I am making is enabling approle, but that means we can't use any of the other fancy annotations for grabbing secrets, etc. Your patch allows approle to work, which is excellent...I just wish for a cleaner deployment (I personally like this original PR better :P )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request injector Area: mutating webhook service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants