Conversation
This is to prepare for Kubernetes support. To authenticate with a Kubernetes service account, we need to make one additional request that then returns the X-Vault-Token. My plan is to check for AuthKubernetes before we start retrieving secrets, and to switch it back to AuthVaultToken once the token has been obtained, so most of the logic does not need to change. For now, this only adds the CLI options for this, but trying to use it would crash.
It makes more sense to me to first verify that we can even parse the secrets file, before we start making requests to Vault.
This reads the Kubernetes jwt from /var/run, and uses Vault's Kubernetes authentication to obtain a short-lived token.
This is a bit of a trade-off between safety and debuggability. If the response could not be parsed, but still contained a secret, this might log secrets to stdout, and they might end up in places where secrets are not supposed to go. But on the other hand, the error message that some json failed to parse, is not very useful if you can't see what the json what that we tried to parse, and if it fails, it is likely a programming or configuration error, and the response will not contain secrets.
It works now! \o/
kubectl logs vaultenv-test-pod
Host: host.minikube.internal
Port: 8200
Addr: http://host.minikube.internal:8200
Authentication method: Kubernetes service account, role: vaultenv-test-role
Secret file: /lib/test.secrets
Command: /bin/env
Arguments: []
Use TLS: False
Validate certs: True
Inherit env: True
Inherit env blacklist: []
Base delay: 40
Retry attempts: 9
Log-level: Info
Use PATH: True
Concurrent requests: 8
DATA_TESTAPP_CONFIG_USERNAME=vaultenvtestuser
DATA_TESTAPP_CONFIG_PASSWORD=hunter2
Finally it can test what it was supposed to test!
ReinierMaas
left a comment
There was a problem hiding this comment.
LGTM!
I have a single comment on the consistency of the code itself. Other pointers are about the testing framework. If those are resolved we can merge the code.
|
|
||
| service_account_jwt_b64 = kubectl( | ||
| "get", "secret", vault_service_account_name, "--output", "go-template={{ .data.token }}", | ||
| capture_output=True, |
There was a problem hiding this comment.
We can replace capture_output=True with stdout=subprocess.PIPE we only use stdout. If there is noise on stderr that we want to ignore we can use stderr=subprocess.DEVNULL.
This will keep the stderr stream output directed to the console allowing to debug the possible failure.
There are more places where we capture_output=True and only use a single output stream.
| # We need to kill and restart the server or the kernel will accept the TCP | ||
| # connections while the Vault server is paused. | ||
| vault_server.terminate() | ||
| wait_seconds = 5 | ||
| vault_server.wait(timeout=wait_seconds) |
There was a problem hiding this comment.
We can try to kill the process when terminate doesn't work to reduce the chance that a running vault server process remains after running the tests:
| # We need to kill and restart the server or the kernel will accept the TCP | |
| # connections while the Vault server is paused. | |
| vault_server.terminate() | |
| wait_seconds = 5 | |
| vault_server.wait(timeout=wait_seconds) | |
| # We need to kill and restart the server or the kernel will accept the TCP | |
| # connections while the Vault server is paused. | |
| vault_server.terminate() | |
| wait_seconds = 5 | |
| try: | |
| vault_server.wait(timeout=wait_seconds) | |
| except subprocess.TimeoutExpired: | |
| vault_server.kill() | |
| raise |
|
I've taken some time to get the Kubernetes tests working, but I haven't succeeded. I have enabled most integration tests in CI, except for the retry tests, which need the SIGSTOP signal to actually stop vaultenv, which does not happen in our CI apparently. |
|
@OpsBotPrime merge |
|
Pull request approved for merge by @Riscky, rebasing now. |
Approved-by: Riscky Auto-deploy: false
|
Rebased as 5e84b58, waiting for CI … |
322936f to
5e84b58
Compare
closes #121
This PR adds two new authentication options to Vaultenv, both of which do not require a root token.
This PR is based of #116, and most of the work is done by @ruuda (thanks!).
Next to the work in that PR, which never got merged, this PR adds a new option
--github-token, which is an alternative to--vault-tokenor--kubernetes-role. If the--github-tokenis supplied, Vaultenv will use this token to get a Vault token that is used for subsequent requests. The provided token should be a Github Personal Access Token with theorg:readscope on the relevant organization. See also https://www.vaultproject.io/docs/auth/githubThis feature has been tested locally, and works nicely.
I did not yet add automated tests, because this requires a setup involving a Github account.