fix: enable Vault JWT auth without Spire for KMS signing#1616
fix: enable Vault JWT auth without Spire for KMS signing#1616infernus01 wants to merge 1 commit into
Conversation
tekton-robot
added
the
size/L
|
/kind bug |
|
Core fix looks good, the priority chain (Spire → static token → K8s SA JWT) preserves backward compatibility. /lgtm |
|
Please add a release note |
When signers.kms.auth.oidc.path and signers.kms.auth.oidc.role are set in the Chains ConfigMap without Spire, the controller now reads the Kubernetes service account token from the pod filesystem and uses it for Vault JWT auth login. Previously, the OIDC token field was only populated by Spire, causing the code to fall through to VAULT_TOKEN lookup and fail with "read .vault-token file: no such file or directory". Signed-off-by: Shubham Bhardwaj <shubbhar@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| if tokenPath == "" { | ||
| tokenPath = defaultOIDCTokenPath | ||
| } | ||
| token, err := os.ReadFile(tokenPath) |
There was a problem hiding this comment.
can we reuse the existing getKMSAuthToken function here instead of calling os.ReadFile directly
There was a problem hiding this comment.
Ah yes that is intentional, since getKMSAuthToken uses TrimSuffix("\n") which only strips a single trailing newline. That's fine for static Vault tokens, but for the OIDC JWT path I used TrimSpace because for example if someone has a token file with just whitespaces like " \n \n", TrimSuffix would leave " \n " (looks non-empty, gets sent to Vault, confusing error), while TrimSpace correctly gives us "" which we then catch with the empty file check.
The empty file check itself is the other difference, since getKMSAuthToken might return "" without error, but here an empty token might cause the code to drop the entire OIDC config and fall back to VAULT_TOKEN, which is the exact bug this PR is fixing.
So same os.ReadFile, but different post-processing needs.
|
/lgtm |
4 participants
Changes
Setting
signers.kms.auth.oidc.pathandsigners.kms.auth.oidc.rolein the Chains ConfigMap without Spire does nothing. The OIDC path and role are parsed correctly but never used — the code falls through toVAULT_TOKEN/~/.vault-tokenand fails:error configuring kms signer with config {hashivault://supply-chain {http://vault.vault:8200/ {jwt tekton-chains} { }}}: read .vault-token file: open /home/nonroot/.vault-token: no such file or directoryNow with this PR, when OIDC path/role are configured and no Spire or static token is present, read the Kubernetes service account token from
/var/run/secrets/kubernetes.io/serviceaccount/tokenand use it for Vault JWT auth. A new config keysigners.kms.auth.oidc.token-pathallows overriding the default path.Priority order: Spire → static token → K8s SA token
fixes: #1479
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes