feat: Add vaultRead() and vaultKV() template functions to read secrets from HashiCorp Vault (GH-473)#848
feat: Add vaultRead() and vaultKV() template functions to read secrets from HashiCorp Vault (GH-473)#848DeekshithaTimmareddy wants to merge 7 commits into
Conversation
- Add vaultRead() and vaultKV() template functions - Automatic KV v1/v2 detection and data unwrapping - Integration with CLI commands (run, plan, render) - Configuration via VAULT_ADDR and VAULT_TOKEN env vars - Complete documentation with usage examples - Move vault/api to direct dependency
DeekshithaTimmareddy
changed the title
jrasell
left a comment
jrasell
left a comment
There was a problem hiding this comment.
Hi @TIMMAREDDYDEEKSHITHA-ship-it and thanks for this. I like the idea of having the different functionality to get the unwrapped KV data or the raw object!
In #850 we are adding CLI flags as well as env var support to configure the Consul client and I think having that within this functionality will be useful too. It'll also be nice to have the UX match each integration as much as possible. The Vault config object has ReadEnvironment and ConfigureTLS functions which I think we should look at using to provide env var configuration and TLS support.
It would also be great to have some minimal tests to cover the Vault API functionality. I think there are a few possibilities here:
- have a basic test HTTP server that can be used to mock the Vault API and return data
- run Vault in dev mode via helper scripts, populate with data in the test
- check if Vault has a similar HTTP test agent that can be used in tests
jrasell
left a comment
jrasell
left a comment
There was a problem hiding this comment.
Hi @TIMMAREDDYDEEKSHITHA-ship-it and thanks for the continued work on this.
One meta comment and item that might be worth discussing is whether it would ever be useful to have multiple variable sources? In the current implementation, it looks like it can only be vault or consul, not potentially Vault and Consul.
It would also be nice to have some tests covering the Vault API calls here as that is the bulk of the functionality.
| Variable precedence is: | ||
|
|
||
| 1. external variable source | ||
| 2. environment variables | ||
| 3. variable files | ||
| 4. CLI --var | ||
| This means CLI --var values override Vault-sourced values. |
There was a problem hiding this comment.
I think this list order needs to be flipped as the numbered list implies external sources have the highest priority (listed as #1), whereas the closing sentence says "CLI --var values override Vault-sourced values", which means Vault is actually lowest priority.
| // vaultKV reads from Vault KV and returns raw data without unwrapping. | ||
| // useful when you need access to metadata or want to handle KV versions yourself. |
There was a problem hiding this comment.
The comment suggests we get the complete response including metadata, but the function is returning secret.Data which is the same as the vaultRead function without the v1/2 unwrapping. Do we need to update the comment or the function?
| if !strings.Contains(out, "vault variable source requires a non-empty path") && !strings.Contains(out, "a Vault-backed variable source was requested, but no Vault client is configured") { | ||
| t.Fatalf("expected missing-path or missing-client error, got:\n%s", out) | ||
| } |
There was a problem hiding this comment.
We could use must.StrContains here and it's probably worth having an assertion for each string check, otherwise the test feels a little fragile and can pass if either error message appears.
| // For KV v2, it automatically unwraps nested data structure. | ||
| func vaultRead(client *vault.Client) func(string) (map[string]interface{}, error) { | ||
| return func(path string) (map[string]interface{}, error) { | ||
| secret, err := client.Logical().Read(path) |
There was a problem hiding this comment.
The Vault API supports ReadWithContext in order to provide timeout or context cancellation. If Vault is slow or unreachable, template rendering and variable parsing will block indefinitely with no way to cancel.
| // useful when you need access to metadata or want to handle KV versions yourself. | ||
| func vaultKV(client *vault.Client) func(string) (map[string]interface{}, error) { | ||
| return func(path string) (map[string]interface{}, error) { | ||
| secret, err := client.Logical().Read(path) |
There was a problem hiding this comment.
The Vault API supports ReadWithContext in order to provide timeout or context cancellation. If Vault is slow or unreachable, template rendering and variable parsing will block indefinitely with no way to cancel.
2 participants
Description
Implements Vault KV integration for nomad-pack templates, allowing packs to read secrets from HashiCorp Vault.
Testing Steps
Testing Environment Setup
Started Vault Dev Server
Here vault is running at : 'http://127.0.0.1:8200'
and the root token is : 'hvs.W3kXXdCLhHibE3qMTQpYRhuS'
Testing Data Setup
Added test secrets to Vault KV v2:
verified secrets were stored:
here all secrets confirmed present in Vault
Test Pack Creation:
Added Vault function calls at the top:
[[ $apiKey := vaultRead "secret/data/api-key" ]]
[[ $dbData := vaultRead "secret/data/database" ]]
Added environment variables in task section:
env {
API_KEY = [[ index $apiKey "api_key" ]]
DB_URL = [[ index $dbData "url" ]]
DB_PASSWORD = [[ index $dbData "password" ]]
}
Test Execution
Built nomad-pack and rendered template with Vault integration:
Here output showed successful Vault secret retrieval
Testing Backward Compatibility
Pack rendered successfully without Vault functions without errors
Changes
vaultRead()andvaultKV()template functionsVAULT_ADDR,VAULT_TOKEN)Reminders
CHANGELOG.mdentryChanges to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.