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

Add support for all Auth Methods via annotations #213

Merged
merged 6 commits into from
Feb 25, 2021

Conversation

pbar1
Copy link
Contributor

@pbar1 pbar1 commented Jan 17, 2021

The Vault Agent Injector currently only supports the Kubernetes Auth Method. This PR adds the ability to use any Auth Method that the Vault Agent supports via configuration parameters passed by annotations like: vault.hashicorp.com/auth-config-PARAM-NAME.

For example, the following annotations,

vault.hashicorp.com/agent-inject: "true"
vault.hashicorp.com/auth-type: cert
vault.hashicorp.com/auth-path: auth/cert
vault.hashicorp.com/auth-config-name: pki-test
vault.hashicorp.com/ca-cert: /certs/ca.pem
vault.hashicorp.com/client-cert: /certs/client.pem
vault.hashicorp.com/client-key: /certs/client-key.pem

Results in the injector adding a VAULT_CONFIG that looks like this:

{
  "auto_auth": {
    "method": {
      "type": "cert",
      "mount_path": "auth/cert",
      "config": {
        "name": "pki-test"
      }
    },
    "sink": [
      {
        "type": "file",
        "config": {
          "path": "/home/vault/.vault-token"
        }
      }
    ]
  },
  "exit_after_auth": false,
  "pid_file": "/home/vault/.pid",
  "vault": {
    "address": "https://vault.vault.svc.cluster.local:8200",
    "ca_cert": "/certs/ca.pem",
    "client_cert": "/certs/client.pem",
    "client_key": "/certs/client-key.pem",
  },
  "template": <omitted for brevity>
}

Also, the changes are general enough to allow passing config for not only the Cert method, but any Auth Method. It does this using the same pattern as vault.hashicorp.com/agent-inject-{secret,template,command}. I've seen a couple other PRs where folks have been adding support for a specific Auth Method here and there; this should support them all in one go.

The reasoning behind this feature: We inject all pods with X.509 certs via SPIFFE at runtime that end up in emptyDir volumes. We'd like to use the Vault Agent Auto-Auth Cert Method with these certs; our Vaults already require mTLS and use the TLS Certificates Auth Method against this PKI, so having the Vault Agent Injector generating the proper config based on annotation alone (as it does for the K8s method) would be a big win in terms of ease of use. We'd be able to deploy the Injector to any cluster in our fleet and have teams able to talk to Vault using the preexisting PKI infra.

I've also got a separate, complementary PR open (#212, for copying another container's volume mounts). We're currently successfully running a build of the injector with these features in our dev environment.

Added some tests, everything is passing. Let me know how I can help to get this merged. Thanks! ☄️

No fail if Role not specified unless K8s auth

Add tests

Signed-off-by: Pierce Bartine <piercebartine@gmail.com>
@jasonodonnell
Copy link
Contributor

Hey @pbar1, thanks for the contribution! This is a really nice feature and definitely something we'd love to work with. Currently we're a bit stretched thin but we'll be reviewing this PR in detail later this week.

At a glance I did have one question that wasn't obvious to me: how are the secrets required for authentication (certs, secret id for approve, etc) getting into the pod? Are you expecting users use #212 and have those already contain the secrets needed?

@pbar1
Copy link
Contributor Author

pbar1 commented Jan 20, 2021

Of course - thanks for the response!

As for getting the certs, #212 would be one way of doing it, and it would work with any type of volume. Others may want to use the annotation vault.hashicorp.com/agent-extra-secret to explicitly mount a K8s secret without copying another container's mounts, for example. I broke the PRs up by feature since circumstances could be different amongst users.

@jasonodonnell
Copy link
Contributor

Thanks @pbar1, I forgot we had agent-extra-secret 😄. That will suffice as a means to get the secrets required in or use #212.

@pbar1
Copy link
Contributor Author

pbar1 commented Feb 12, 2021

Just checking in - any updates, anything I can do to help?

Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

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

I tested this with app-role and it worked great using these annotations:

vault.hashicorp.com/agent-inject: "true"
vault.hashicorp.com/agent-inject-status: "update"
vault.hashicorp.com/agent-run-as-same-user: "true"
vault.hashicorp.com/agent-extra-secret: "approle-test"
vault.hashicorp.com/auth-type: "approle"
vault.hashicorp.com/auth-path: "auth/approle"
vault.hashicorp.com/auth-config-role-id-file-path: "/vault/custom/role-id"
vault.hashicorp.com/auth-config-secret-id-file-path: "/vault/custom/secret-id"
# DATABASE SECRET
vault.hashicorp.com/agent-inject-secret-db-creds: "database/creds/db-app"
vault.hashicorp.com/agent-inject-template-db-creds: |
  {{- with secret "database/creds/db-app" -}}
  postgres://{{ .Data.username }}:{{ .Data.password }}@postgres.postgres.svc:5432/wizard?sslmode=disable
  {{- end }}
vault.hashicorp.com/agent-inject-command-db-creds: "sh -c 'kill -HUP $(pidof app)'"
# VAULT SETTINGS
vault.hashicorp.com/role: "my-role"
vault.hashicorp.com/tls-secret: "tls-test-client"
vault.hashicorp.com/ca-cert: "/vault/tls/ca.crt"

I ran into a small issue due to the agent-extra-secret mounting the volume as read-only:

2021-02-19T15:49:04.595Z [ERROR] auth.approle: error removing secret ID file after reading: error="remove /vault/custom/secret-id: read-only file system"

I don't think we'll want this writable, though, so we'll have to live with the error for now.

Thanks for the contribution and your patience.

@jasonodonnell
Copy link
Contributor

@tvoran You mind giving this a glance to ensure I didn't miss anything?

Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just a couple suggestions.

It looks like config.AutoAuth.Method.Type should be checked here similar to MountPath:

if config.AutoAuth.Method.MountPath != annotations[AnnotationVaultAuthPath] {
t.Errorf("auto_auth mount path: expected path to be %s, got %s", annotations[AnnotationVaultAuthPath], config.AutoAuth.Method.MountPath)
}

agent-inject/agent/agent.go Show resolved Hide resolved
@pbar1
Copy link
Contributor Author

pbar1 commented Feb 23, 2021

@tvoran Implemented both of your suggestions

@jasonodonnell jasonodonnell merged commit 0dfc71e into hashicorp:master Feb 25, 2021
@pbar1 pbar1 deleted the generalize-autoauth branch February 26, 2021 21:26
RemcoBuddelmeijer pushed a commit to RemcoBuddelmeijer/vault-k8s that referenced this pull request Feb 22, 2022
* Generalize Agent Auto Auth to allow all methods

No fail if Role not specified unless K8s auth

Add tests

Signed-off-by: Pierce Bartine <piercebartine@gmail.com>

* Fix merge conflict

* Tweak test conditions

* Default Auto-Auth type and validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants