feat: use go-spiffe SDK directly instead of spiffe-helper sidecar - #522
feat: use go-spiffe SDK directly instead of spiffe-helper sidecar#522Alan-Cha wants to merge 7 commits into
Conversation
Remove spiffe-helper sidecar from operator pod and use go-spiffe SDK directly to fetch JWT-SVIDs from the SPIRE Workload API. Changes: - operator/cmd/main.go: Remove --jwt-svid-path flag, pass SpiffeSocket instead of JWTSVIDPath to controller - operator/internal/controller/clientregistration_controller.go: Replace JWTSVIDPath field with SpiffeSocket, add fetchJWTSVID() helper that uses workloadapi.Client.FetchJWTSVID() to get JWT-SVID - charts/operator/templates/manager/manager.yaml: Remove spiffe-helper sidecar container, jwt-svid volume, and --jwt-svid-path CLI arg - charts/operator/templates/manager/configmap-spiffe-helper.yaml: Delete (no longer needed) This matches the architecture already used by authbridge-proxy and eliminates the need for maintaining a separate spiffe-helper sidecar. Closes: #478 Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Address errcheck linter: defer client.Close() now properly checks and logs any close errors. Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Add unit tests for fetchJWTSVID error handling: - Test invalid socket path - Test empty socket path - Document limitations of unit tests vs integration/E2E tests Add comprehensive testing guide documenting: - Unit test scope and limitations - Integration test requirements (future work) - E2E test procedures with SPIRE - Manual verification steps for token exchange - Common issues and troubleshooting Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
JWT-SVID audience must match the token endpoint URL for Keycloak
JWT-SPIFFE authentication, not just the realm name. Keycloak validates
the audience claim and rejects tokens with incorrect audience.
Changes fetchJWTSVID() call to construct the full token endpoint URL:
${KEYCLOAK_URL}/realms/${REALM}/protocol/openid-connect/token
Fixes "Invalid token audience" error during operator authentication.
Assisted-By: Claude Code
Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Update: Fixed JWT-SVID Audience IssueFound and fixed a critical bug during E2E testing: Problem: JWT-SVID was being fetched with Fix: Changed tokenEndpoint := strings.TrimSuffix(ab.KeycloakURL, "/") + "/realms/" + ab.KeycloakRealm + "/protocol/openid-connect/token"
jwtSVID, err := r.fetchJWTSVID(ctx, tokenEndpoint)Verification in progress:
Committed in 952a53d |
JWT-SVID audience must match Keycloak's realm issuer URL exactly per RFC 7523. The issuer is typically a public URL (e.g., keycloak.localtest.me) while the authbridge-config contains the in-cluster service address. Changes: - Add getKeycloakIssuer() to query /.well-known/openid-configuration - Use issuer URL as JWT-SVID audience instead of service URL - Add proper error handling and logging for issuer lookup Fixes "Invalid token audience" error during operator authentication. Ref: docs/users-guides/authentication.md lines 72-76 Assisted-By: Claude Code Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Critical Fix: JWT-SVID Audience Must Match Keycloak Issuer URLFound root cause of "Invalid token audience" error: Problem: JWT-SVID audience was using in-cluster service URL, but Keycloak validates against its public issuer URL. From authentication.md:
Solution: Query Keycloak's OIDC discovery endpoint to get authoritative issuer URL: func (r *ClientRegistrationReconciler) getKeycloakIssuer(ctx context.Context, keycloakURL, realm string) (string, error) {
discoveryURL := keycloakURL + "/realms/" + realm + "/.well-known/openid-configuration"
// Query and extract issuer field
return config.Issuer, nil // e.g., "http://keycloak.localtest.me:8080/realms/rossoctl"
}Testing with correct issuer URL now... Committed in afce861 |
Fixes errcheck linter error: Error return value of `resp.Body.Close` is not checked Assisted-By: Claude Code Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
clawgenti
left a comment
There was a problem hiding this comment.
Clean refactor that eliminates the spiffe-helper sidecar in favor of the go-spiffe SDK — good alignment with the authbridge-proxy pattern. A few items below.
Reviewed by clawgenti using the github-pr-review skill
| - name: jwt-svid | ||
| mountPath: /opt | ||
| readOnly: true | ||
| {{- end }} |
There was a problem hiding this comment.
nit: This {{- if and .Values.spiffe ... }}{{- end }} block is empty after removing the jwt-svid volume mount from inside it — it can be deleted.
| // Per RFC 7523 and Keycloak SPIFFE authentication: the JWT audience must match | ||
| // Keycloak's realm issuer URL exactly. Query the OIDC discovery endpoint to get | ||
| // the authoritative issuer value, since it may differ from the in-cluster service URL. | ||
| realmIssuer, err := r.getKeycloakIssuer(ctx, ab.KeycloakURL, ab.KeycloakRealm) |
There was a problem hiding this comment.
suggestion: getKeycloakIssuer is called on every reconcile when UseSpiffeAuth=true, creating a new http.Client and issuing an OIDC discovery request each time. The issuer URL is stable — consider caching it on the reconciler (e.g. a keycloakIssuer string field populated once on first successful fetch) to avoid per-reconcile HTTP overhead.
| } | ||
|
|
||
| jwtSVID, err := os.ReadFile(cleanPath) | ||
| jwtSVID, err := r.fetchJWTSVID(ctx, realmIssuer) |
There was a problem hiding this comment.
suggestion: fetchJWTSVID opens a new gRPC connection to the SPIRE Workload API on every call. Consider reusing a long-lived workloadapi.Client (stored on the reconciler and lazily initialized) to reduce connection churn on frequent reconciles, similar to how authbridge-proxy manages its SPIFFE client.
Summary
Remove spiffe-helper sidecar from operator pod and use go-spiffe SDK directly to fetch JWT-SVIDs from the SPIRE Workload API.
Changes
--jwt-svid-pathflag, pass SpiffeSocket to controller--jwt-svid-pathCLI argMotivation
This matches the architecture already used by authbridge-proxy and eliminates the need for maintaining a separate spiffe-helper sidecar. The go-spiffe SDK provides direct Workload API access, making the file-based JWT-SVID exchange unnecessary.
Testing
Closes
Replaces #478 with a clean implementation.
Assisted-By: Claude Code