From 76d4f7c061bd5e44c47332efe6f8fcf601dd1ab5 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 01:50:23 -0400 Subject: [PATCH 1/8] feat: use go-spiffe SDK directly instead of spiffe-helper sidecar 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 --- .../manager/configmap-spiffe-helper.yaml | 36 ----------- .../operator/templates/manager/manager.yaml | 43 ------------- operator/cmd/main.go | 5 +- .../clientregistration_controller.go | 60 ++++++++++++------- 4 files changed, 38 insertions(+), 106 deletions(-) delete mode 100644 charts/operator/templates/manager/configmap-spiffe-helper.yaml diff --git a/charts/operator/templates/manager/configmap-spiffe-helper.yaml b/charts/operator/templates/manager/configmap-spiffe-helper.yaml deleted file mode 100644 index 075b3b16..00000000 --- a/charts/operator/templates/manager/configmap-spiffe-helper.yaml +++ /dev/null @@ -1,36 +0,0 @@ -{{- if and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled }} ---- -apiVersion: v1 -kind: ConfigMap -metadata: - name: operator-spiffe-helper-config - namespace: {{ .Release.Namespace }} - labels: - {{- include "chart.labels" . | nindent 4 }} -data: - config.hcl: | - agent_address = "/spiffe-workload-api/spire-agent.sock" - # cmd/cmd_args fire on X.509 SVID renewals only, not JWT SVIDs. - # File permissions are handled by running spiffe-helper as the same UID - # as the manager container (65532) so it can read the 600-mode file. - renew_signal = "" - cert_dir = "" - svid_file_name = "" - svid_bundle_file_name = "" - {{- /* - jwt_audience must equal the Keycloak realm's issuer URL — the string Keycloak - advertises in its /.well-known/openid-configuration. It is always derived from - keycloak.publicUrl (the external URL Keycloak was configured with). The internal - k8s service address must NOT be used because Keycloak's FederatedJWTClientValidator - checks the aud claim via string equality against its own issuer, which is always - the external URL. - */ -}} - {{- $audience := printf "%s/realms/%s" .Values.keycloak.publicUrl .Values.keycloak.realm -}} - {{- if not (hasPrefix "http" $audience) -}} - {{- fail "keycloak.publicUrl must be set to the external Keycloak URL (e.g. http://keycloak.example.com) when spiffe.operatorAuth.enabled=true" -}} - {{- end -}} - jwt_svids = [{ - jwt_audience = "{{ $audience }}" - jwt_svid_file_name = "/opt/jwt_svid.token" - }] -{{- end }} diff --git a/charts/operator/templates/manager/manager.yaml b/charts/operator/templates/manager/manager.yaml index ae832633..5d8779a2 100644 --- a/charts/operator/templates/manager/manager.yaml +++ b/charts/operator/templates/manager/manager.yaml @@ -118,7 +118,6 @@ spec: {{- end }} {{- if and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled }} - "--use-spiffe-auth=true" - - "--jwt-svid-path={{ .Values.spiffe.operatorAuth.jwtSVIDPath | default "/opt/jwt_svid.token" }}" - "--operator-client-id=spiffe://{{ .Values.signatureVerification.spireTrustDomain | default "localtest.me" }}/ns/{{ .Release.Namespace }}/sa/{{ .Values.controllerManager.serviceAccountName }}" {{- end }} command: @@ -187,41 +186,7 @@ spec: readOnly: true {{- end }} {{- if and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled }} - - name: jwt-svid - mountPath: /opt - readOnly: true {{- end }} - {{- if and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled }} - - name: spiffe-helper - image: ghcr.io/spiffe/spiffe-helper:0.11.0 - imagePullPolicy: IfNotPresent - args: - - "-config" - - "/etc/spiffe-helper/config.hcl" - volumeMounts: - - name: spiffe-workload-api - mountPath: /spiffe-workload-api - readOnly: true - - name: spiffe-helper-config - mountPath: /etc/spiffe-helper - readOnly: true - - name: jwt-svid - mountPath: /opt - securityContext: - allowPrivilegeEscalation: false - runAsNonRoot: true - runAsUser: 65532 - capabilities: - drop: - - ALL - resources: - requests: - cpu: 10m - memory: 32Mi - limits: - cpu: 100m - memory: 64Mi - {{- end }} securityContext: {{- toYaml .Values.controllerManager.securityContext | nindent 8 }} serviceAccountName: {{ .Values.controllerManager.serviceAccountName }} @@ -251,11 +216,3 @@ spec: driver: "csi.spiffe.io" readOnly: true {{- end }} - {{- if and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled }} - - name: spiffe-helper-config - configMap: - name: operator-spiffe-helper-config - - name: jwt-svid - emptyDir: - medium: Memory - {{- end }} diff --git a/operator/cmd/main.go b/operator/cmd/main.go index ffa8629f..518f1087 100644 --- a/operator/cmd/main.go +++ b/operator/cmd/main.go @@ -135,7 +135,6 @@ func main() { var credentialWaitTimeout string var enableAuthbridgeConfig bool var useSpiffeAuth bool - var jwtSVIDPath string var operatorClientID string flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ @@ -212,8 +211,6 @@ func main() { "Reconcile authbridge-config ConfigMap in namespaces labeled rossoctl-enabled=true") flag.BoolVar(&useSpiffeAuth, "use-spiffe-auth", false, "Use JWT-SVID authentication for Keycloak client registration instead of admin credentials") - flag.StringVar(&jwtSVIDPath, "jwt-svid-path", "/opt/jwt_svid.token", - "Path to JWT-SVID file written by spiffe-helper sidecar (used when --use-spiffe-auth=true)") flag.StringVar(&operatorClientID, "operator-client-id", "", "Operator SPIFFE ID (e.g. spiffe:///ns//sa/), used when --use-spiffe-auth=true") @@ -711,7 +708,7 @@ func main() { SpireTrustDomain: spireTrustDomain, KeycloakAdminTokenCache: &keycloak.CachedAdminTokenProvider{}, UseSpiffeAuth: useSpiffeAuth, - JWTSVIDPath: jwtSVIDPath, + SpiffeSocket: verifiedFetchSpiffeSocket, OperatorClientID: operatorClientID, Recorder: mgr.GetEventRecorderFor("clientregistration"), //nolint:staticcheck }).SetupWithManager(mgr); err != nil { diff --git a/operator/internal/controller/clientregistration_controller.go b/operator/internal/controller/clientregistration_controller.go index e0dcec9a..1f2550a7 100644 --- a/operator/internal/controller/clientregistration_controller.go +++ b/operator/internal/controller/clientregistration_controller.go @@ -10,12 +10,13 @@ package controller import ( "context" "fmt" - "os" - "path/filepath" "sort" "strings" "time" + "github.com/spiffe/go-spiffe/v2/svid/jwtsvid" + "github.com/spiffe/go-spiffe/v2/workloadapi" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -78,9 +79,9 @@ type ClientRegistrationReconciler struct { // the Admin API with manage-clients role. When false, uses admin credentials. UseSpiffeAuth bool - // JWTSVIDPath is the file path to read the operator's JWT-SVID from. - // Only used when UseSpiffeAuth is true. Default: /opt/jwt_svid.token - JWTSVIDPath string + // SpiffeSocket is the path to the SPIFFE Workload API socket (e.g., unix:///run/spire/sockets/agent.sock). + // Only used when UseSpiffeAuth is true. Used to fetch JWT-SVIDs via go-spiffe SDK. + SpiffeSocket string // OperatorClientID is the operator's SPIFFE ID (e.g., spiffe://localtest.me/ns/rossoctl-operator-system/sa/...). // Only used when UseSpiffeAuth is true. @@ -275,36 +276,30 @@ func (r *ClientRegistrationReconciler) reconcileOne( return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } - jwtSVIDPath := r.JWTSVIDPath - if jwtSVIDPath == "" { - jwtSVIDPath = "/opt/jwt_svid.token" - } - - // Path traversal protection: only allow reading from designated directories - cleanPath := filepath.Clean(jwtSVIDPath) - if !strings.HasPrefix(cleanPath, "/opt/") && !strings.HasPrefix(cleanPath, "/var/run/secrets/") { - err := fmt.Errorf("JWT-SVID path %q outside allowed directories (/opt/, /var/run/secrets/)", jwtSVIDPath) - logger.Error(err, "invalid JWT-SVID path") + if r.SpiffeSocket == "" { + err := fmt.Errorf("SpiffeSocket is required when UseSpiffeAuth=true") + logger.Error(err, "missing SPIFFE socket path") if r.Recorder != nil { - r.Recorder.Eventf(owner, corev1.EventTypeWarning, "InvalidJWTSVIDPath", - "JWT-SVID path %q rejected: must be under /opt/ or /var/run/secrets/", jwtSVIDPath) + r.Recorder.Event(owner, corev1.EventTypeWarning, "SpiffeSocketMissing", + "UseSpiffeAuth=true but SpiffeSocket is empty. Check operator configuration.") } - return ctrl.Result{}, err // fail permanently on config error + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } - jwtSVID, err := os.ReadFile(cleanPath) + // Fetch JWT-SVID from SPIRE via Workload API + jwtSVID, err := r.fetchJWTSVID(ctx, ab.KeycloakRealm) if err != nil { - logger.Error(err, "read JWT-SVID failed", "path", cleanPath) + logger.Error(err, "JWT-SVID fetch failed") if r.Recorder != nil { - r.Recorder.Eventf(owner, corev1.EventTypeWarning, "JWTSVIDReadFailed", - "Failed to read JWT-SVID from %s: %v. Check spiffe-helper sidecar configuration.", cleanPath, err) + r.Recorder.Eventf(owner, corev1.EventTypeWarning, "JWTSVIDFetchFailed", + "Failed to fetch JWT-SVID from SPIRE: %v", err) } return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } // WARNING: JWT-SVID is a bearer token - must never appear in logs or error messages // to prevent token exposure. All code paths must handle jwtSVID as sensitive data. - token, err = kc.JWTSVIDGrantToken(ctx, ab.KeycloakRealm, r.OperatorClientID, string(jwtSVID)) + token, err = kc.JWTSVIDGrantToken(ctx, ab.KeycloakRealm, r.OperatorClientID, jwtSVID) if err != nil { logger.Error(err, "Keycloak JWT-SVID authentication failed") if r.Recorder != nil { @@ -618,3 +613,22 @@ func (r *ClientRegistrationReconciler) SetupWithManager(mgr ctrl.Manager) error return b.Complete(r) } + +// fetchJWTSVID fetches a JWT-SVID from the SPIRE Workload API for the given audience. +// Returns the JWT token as a string or an error if fetching fails. +func (r *ClientRegistrationReconciler) fetchJWTSVID(ctx context.Context, audience string) (string, error) { + client, err := workloadapi.New(ctx, workloadapi.WithAddr(r.SpiffeSocket)) + if err != nil { + return "", fmt.Errorf("failed to create SPIFFE Workload API client: %w", err) + } + defer client.Close() + + svid, err := client.FetchJWTSVID(ctx, jwtsvid.Params{ + Audience: audience, + }) + if err != nil { + return "", fmt.Errorf("failed to fetch JWT-SVID: %w", err) + } + + return svid.Marshal(), nil +} From 3db02299a52bfa22021409a5411a23b2d6306106 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 12:31:59 -0400 Subject: [PATCH 2/8] fix: check error from client.Close() in fetchJWTSVID Address errcheck linter: defer client.Close() now properly checks and logs any close errors. Signed-off-by: Alan Cha --- .../internal/controller/clientregistration_controller.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/operator/internal/controller/clientregistration_controller.go b/operator/internal/controller/clientregistration_controller.go index 1f2550a7..e5ef429e 100644 --- a/operator/internal/controller/clientregistration_controller.go +++ b/operator/internal/controller/clientregistration_controller.go @@ -621,7 +621,12 @@ func (r *ClientRegistrationReconciler) fetchJWTSVID(ctx context.Context, audienc if err != nil { return "", fmt.Errorf("failed to create SPIFFE Workload API client: %w", err) } - defer client.Close() + defer func() { + if closeErr := client.Close(); closeErr != nil { + // Log close error but don't override the function's return error + ctrl.Log.WithName("fetchJWTSVID").Error(closeErr, "failed to close SPIFFE Workload API client") + } + }() svid, err := client.FetchJWTSVID(ctx, jwtsvid.Params{ Audience: audience, From d989096692816162c562005b2ace10a22efae0e5 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 12:46:08 -0400 Subject: [PATCH 3/8] test: add unit tests and documentation for JWT-SVID fetching 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 --- .../docs/testing/token-exchange-testing.md | 198 ++++++++++++++++++ ...gistration_controller_fetchjwtsvid_test.go | 55 +++++ 2 files changed, 253 insertions(+) create mode 100644 operator/docs/testing/token-exchange-testing.md create mode 100644 operator/internal/controller/clientregistration_controller_fetchjwtsvid_test.go diff --git a/operator/docs/testing/token-exchange-testing.md b/operator/docs/testing/token-exchange-testing.md new file mode 100644 index 00000000..245a091c --- /dev/null +++ b/operator/docs/testing/token-exchange-testing.md @@ -0,0 +1,198 @@ +# Token Exchange Testing Guide + +This document describes how to test JWT-SVID fetching and token exchange functionality. + +## Test Levels + +### 1. Unit Tests + +**Location:** `operator/internal/controller/clientregistration_controller_fetchjwtsvid_test.go` + +**What they test:** +- Error handling when SPIFFE socket is invalid +- Error handling when SPIFFE socket path is empty +- Method signature and basic validation + +**Limitations:** +- Cannot test actual SPIRE Workload API interaction without a running SPIRE agent +- Cannot verify JWT-SVID content or token exchange flow + +**Run them:** +```bash +cd operator +go test -v -run TestFetchJWTSVID ./internal/controller/ +``` + +### 2. Integration Tests + +**Not yet implemented** - Would require: +- Mock SPIFFE Workload API server implementation +- Test fixtures for JWT-SVID responses +- Mock Keycloak IDP for token exchange verification + +**Future work:** +- Implement mock Workload API using gRPC server +- Test JWT-SVID marshaling/unmarshaling +- Test Keycloak JWT-SVID grant flow + +### 3. E2E Tests + +**Location:** Main repo `rossoctl/tests/e2e/` + +**What they test:** +- Full operator deployment with SPIRE +- Client registration with SPIFFE auth enabled +- Token exchange in weather agent demo +- AuthBridge routes configuration and matching + +**Prerequisites:** +- Kind cluster with SPIRE deployed +- Keycloak with SPIFFE IDP configured +- Operator deployed with `--use-spiffe-auth=true` + +**Run them:** +```bash +# From rossoctl repo root +./.github/scripts/local-setup/kind-full-test.sh --skip-cluster-destroy + +# Verify operator has SPIFFE auth enabled +kubectl get deployment rossoctl-controller-manager -n rossoctl-system \ + -o jsonpath='{.spec.template.spec.containers[0].args}' | grep use-spiffe-auth + +# Check client registration logs +kubectl logs -n rossoctl-system deployment/rossoctl-controller-manager \ + | grep "authenticated with JWT-SVID" + +# Deploy weather agents and verify token exchange +./.github/scripts/operator/72-deploy-weather-tool.sh +./.github/scripts/operator/74-deploy-weather-agent.sh + +# Check AuthBridge logs for token exchange activity +kubectl logs -n team1 -l app.kubernetes.io/name=weather-service -c authbridge-proxy \ + | grep "token-exchange" +``` + +## Testing Token Exchange Flow + +### Manual Verification + +1. **Verify operator fetches JWT-SVID:** +```bash +# Check operator logs for SPIFFE Workload API client creation +kubectl logs -n rossoctl-system deployment/rossoctl-controller-manager \ + | grep -i "spiffe\|jwt-svid" +``` + +2. **Verify Keycloak client registration:** +```bash +# Check that client secret was created with federated-jwt mode +kubectl get secrets -n team1 | grep keycloak-client-credentials + +# Inspect secret contents (should have client-id.txt, may have client-secret.txt) +kubectl get secret -n team1 -o jsonpath='{.data}' | jq 'keys' +``` + +3. **Verify AuthBridge token exchange:** +```bash +# Get weather-service pod +POD=$(kubectl get pod -n team1 -l app.kubernetes.io/name=weather-service -o jsonpath='{.items[0].metadata.name}') + +# Trigger outbound request +kubectl exec -n team1 $POD -c agent -- python3 -c " +import urllib.request +url = 'http://weather-tool-mcp.team1.svc.cluster.local:8000/mcp' +data = b'{\"method\": \"tools/list\"}' +req = urllib.request.Request(url, data=data, headers={'Content-Type': 'application/json'}) +try: + response = urllib.request.urlopen(req, timeout=10) + print('Success:', response.status) +except Exception as e: + print('Request sent:', str(e)[:100]) +" + +# Check AuthBridge logs for token-exchange activity +kubectl logs -n team1 $POD -c authbridge-proxy --tail=50 | grep token-exchange +``` + +### Automated E2E Test Script + +See `/tmp/rossoctl/e2e/spiffe-sdk-test.sh` for a complete automated test that: +1. Deploys operator with updated image +2. Verifies single-container pod (no spiffe-helper sidecar) +3. Deploys weather agents with SPIFFE identity +4. Triggers token exchange +5. Verifies logs show successful token exchange + +## Adding New Tests + +### Unit Test Template + +```go +func TestFetchJWTSVID_YourCase(t *testing.T) { + r := &ClientRegistrationReconciler{ + SpiffeSocket: "unix:///your/socket/path", + } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + svid, err := r.fetchJWTSVID(ctx, "your-audience") + + // Assert expected behavior + if err != nil { + t.Fatalf("expected success, got error: %v", err) + } + + if svid == "" { + t.Fatal("expected non-empty JWT-SVID") + } +} +``` + +### Integration Test (Future) + +Would require mock Workload API implementation: + +```go +// mockWorkloadAPI implements SPIFFE Workload API gRPC service +type mockWorkloadAPI struct { + // Store test JWT-SVIDs + svids map[string]string +} + +func TestFetchJWTSVID_WithMockWorkloadAPI(t *testing.T) { + // 1. Start mock gRPC server + // 2. Create ClientRegistrationReconciler with mock socket + // 3. Call fetchJWTSVID() + // 4. Verify returned JWT-SVID matches mock response +} +``` + +## Common Issues + +### "failed to create SPIFFE Workload API client" + +- SPIRE agent is not running +- Socket path is incorrect +- Socket permissions are wrong +- Pod doesn't have SPIRE volume mount + +### "failed to fetch JWT-SVID" + +- Workload not attested by SPIRE agent +- Audience doesn't match any SPIRE registration entries +- SPIRE server trust domain mismatch + +### Token exchange fails in AuthBridge + +- Keycloak SPIFFE IDP not configured +- JWT-SVID audience doesn't match Keycloak realm +- Client not registered in Keycloak +- AuthBridge identity.type not set to "spiffe" + +## Related Documentation + +- [SPIFFE Workload API Spec](https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Workload_API.md) +- [go-spiffe Documentation](https://pkg.go.dev/github.com/spiffe/go-spiffe/v2) +- [Rossoctl E2E Testing](../../../rossoctl/tests/e2e/README.md) +- [AuthBridge Token Exchange](../../../rossoctl/auth/authbridge-proxy/docs/token-exchange.md) diff --git a/operator/internal/controller/clientregistration_controller_fetchjwtsvid_test.go b/operator/internal/controller/clientregistration_controller_fetchjwtsvid_test.go new file mode 100644 index 00000000..5b2a22d7 --- /dev/null +++ b/operator/internal/controller/clientregistration_controller_fetchjwtsvid_test.go @@ -0,0 +1,55 @@ +package controller + +import ( + "context" + "testing" + "time" +) + +func TestFetchJWTSVID_InvalidSocketPath(t *testing.T) { + // Test that fetchJWTSVID returns an error when the socket path is invalid + r := &ClientRegistrationReconciler{ + SpiffeSocket: "unix:///nonexistent/socket.sock", + } + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + _, err := r.fetchJWTSVID(ctx, "test-audience") + if err == nil { + t.Fatal("expected error when connecting to nonexistent socket, got nil") + } + + // Error should mention client creation failure + errMsg := err.Error() + if errMsg == "" { + t.Fatal("expected non-empty error message") + } +} + +func TestFetchJWTSVID_EmptySocketPath(t *testing.T) { + // Test that fetchJWTSVID handles empty socket path gracefully + r := &ClientRegistrationReconciler{ + SpiffeSocket: "", + } + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + _, err := r.fetchJWTSVID(ctx, "test-audience") + if err == nil { + t.Fatal("expected error when socket path is empty, got nil") + } +} + +// NOTE: Full integration tests with a real SPIRE agent require: +// 1. Running SPIRE server and agent +// 2. Properly configured workload attestation +// 3. Valid SPIFFE trust domain +// +// These are better suited for E2E tests (e.g., operator/test/e2e/) rather than +// unit tests. The tests above verify error handling for the common failure cases. +// +// For E2E token exchange verification, see: +// - rossoctl/tests/e2e/ (main repo E2E tests) +// - .github/scripts/operator/ (deployment scripts that test token exchange) From 42302de652a3dfb103562c2e4556e2f72e391694 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 12:52:09 -0400 Subject: [PATCH 4/8] test: remove documentation page, keep unit tests only Signed-off-by: Alan Cha --- .../docs/testing/token-exchange-testing.md | 198 ------------------ 1 file changed, 198 deletions(-) delete mode 100644 operator/docs/testing/token-exchange-testing.md diff --git a/operator/docs/testing/token-exchange-testing.md b/operator/docs/testing/token-exchange-testing.md deleted file mode 100644 index 245a091c..00000000 --- a/operator/docs/testing/token-exchange-testing.md +++ /dev/null @@ -1,198 +0,0 @@ -# Token Exchange Testing Guide - -This document describes how to test JWT-SVID fetching and token exchange functionality. - -## Test Levels - -### 1. Unit Tests - -**Location:** `operator/internal/controller/clientregistration_controller_fetchjwtsvid_test.go` - -**What they test:** -- Error handling when SPIFFE socket is invalid -- Error handling when SPIFFE socket path is empty -- Method signature and basic validation - -**Limitations:** -- Cannot test actual SPIRE Workload API interaction without a running SPIRE agent -- Cannot verify JWT-SVID content or token exchange flow - -**Run them:** -```bash -cd operator -go test -v -run TestFetchJWTSVID ./internal/controller/ -``` - -### 2. Integration Tests - -**Not yet implemented** - Would require: -- Mock SPIFFE Workload API server implementation -- Test fixtures for JWT-SVID responses -- Mock Keycloak IDP for token exchange verification - -**Future work:** -- Implement mock Workload API using gRPC server -- Test JWT-SVID marshaling/unmarshaling -- Test Keycloak JWT-SVID grant flow - -### 3. E2E Tests - -**Location:** Main repo `rossoctl/tests/e2e/` - -**What they test:** -- Full operator deployment with SPIRE -- Client registration with SPIFFE auth enabled -- Token exchange in weather agent demo -- AuthBridge routes configuration and matching - -**Prerequisites:** -- Kind cluster with SPIRE deployed -- Keycloak with SPIFFE IDP configured -- Operator deployed with `--use-spiffe-auth=true` - -**Run them:** -```bash -# From rossoctl repo root -./.github/scripts/local-setup/kind-full-test.sh --skip-cluster-destroy - -# Verify operator has SPIFFE auth enabled -kubectl get deployment rossoctl-controller-manager -n rossoctl-system \ - -o jsonpath='{.spec.template.spec.containers[0].args}' | grep use-spiffe-auth - -# Check client registration logs -kubectl logs -n rossoctl-system deployment/rossoctl-controller-manager \ - | grep "authenticated with JWT-SVID" - -# Deploy weather agents and verify token exchange -./.github/scripts/operator/72-deploy-weather-tool.sh -./.github/scripts/operator/74-deploy-weather-agent.sh - -# Check AuthBridge logs for token exchange activity -kubectl logs -n team1 -l app.kubernetes.io/name=weather-service -c authbridge-proxy \ - | grep "token-exchange" -``` - -## Testing Token Exchange Flow - -### Manual Verification - -1. **Verify operator fetches JWT-SVID:** -```bash -# Check operator logs for SPIFFE Workload API client creation -kubectl logs -n rossoctl-system deployment/rossoctl-controller-manager \ - | grep -i "spiffe\|jwt-svid" -``` - -2. **Verify Keycloak client registration:** -```bash -# Check that client secret was created with federated-jwt mode -kubectl get secrets -n team1 | grep keycloak-client-credentials - -# Inspect secret contents (should have client-id.txt, may have client-secret.txt) -kubectl get secret -n team1 -o jsonpath='{.data}' | jq 'keys' -``` - -3. **Verify AuthBridge token exchange:** -```bash -# Get weather-service pod -POD=$(kubectl get pod -n team1 -l app.kubernetes.io/name=weather-service -o jsonpath='{.items[0].metadata.name}') - -# Trigger outbound request -kubectl exec -n team1 $POD -c agent -- python3 -c " -import urllib.request -url = 'http://weather-tool-mcp.team1.svc.cluster.local:8000/mcp' -data = b'{\"method\": \"tools/list\"}' -req = urllib.request.Request(url, data=data, headers={'Content-Type': 'application/json'}) -try: - response = urllib.request.urlopen(req, timeout=10) - print('Success:', response.status) -except Exception as e: - print('Request sent:', str(e)[:100]) -" - -# Check AuthBridge logs for token-exchange activity -kubectl logs -n team1 $POD -c authbridge-proxy --tail=50 | grep token-exchange -``` - -### Automated E2E Test Script - -See `/tmp/rossoctl/e2e/spiffe-sdk-test.sh` for a complete automated test that: -1. Deploys operator with updated image -2. Verifies single-container pod (no spiffe-helper sidecar) -3. Deploys weather agents with SPIFFE identity -4. Triggers token exchange -5. Verifies logs show successful token exchange - -## Adding New Tests - -### Unit Test Template - -```go -func TestFetchJWTSVID_YourCase(t *testing.T) { - r := &ClientRegistrationReconciler{ - SpiffeSocket: "unix:///your/socket/path", - } - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - svid, err := r.fetchJWTSVID(ctx, "your-audience") - - // Assert expected behavior - if err != nil { - t.Fatalf("expected success, got error: %v", err) - } - - if svid == "" { - t.Fatal("expected non-empty JWT-SVID") - } -} -``` - -### Integration Test (Future) - -Would require mock Workload API implementation: - -```go -// mockWorkloadAPI implements SPIFFE Workload API gRPC service -type mockWorkloadAPI struct { - // Store test JWT-SVIDs - svids map[string]string -} - -func TestFetchJWTSVID_WithMockWorkloadAPI(t *testing.T) { - // 1. Start mock gRPC server - // 2. Create ClientRegistrationReconciler with mock socket - // 3. Call fetchJWTSVID() - // 4. Verify returned JWT-SVID matches mock response -} -``` - -## Common Issues - -### "failed to create SPIFFE Workload API client" - -- SPIRE agent is not running -- Socket path is incorrect -- Socket permissions are wrong -- Pod doesn't have SPIRE volume mount - -### "failed to fetch JWT-SVID" - -- Workload not attested by SPIRE agent -- Audience doesn't match any SPIRE registration entries -- SPIRE server trust domain mismatch - -### Token exchange fails in AuthBridge - -- Keycloak SPIFFE IDP not configured -- JWT-SVID audience doesn't match Keycloak realm -- Client not registered in Keycloak -- AuthBridge identity.type not set to "spiffe" - -## Related Documentation - -- [SPIFFE Workload API Spec](https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Workload_API.md) -- [go-spiffe Documentation](https://pkg.go.dev/github.com/spiffe/go-spiffe/v2) -- [Rossoctl E2E Testing](../../../rossoctl/tests/e2e/README.md) -- [AuthBridge Token Exchange](../../../rossoctl/auth/authbridge-proxy/docs/token-exchange.md) From 952a53de6cecba045ebd8a68641a3755785eeee2 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 14:42:38 -0400 Subject: [PATCH 5/8] fix: Use Keycloak token endpoint URL as JWT-SVID audience 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 --- operator/internal/controller/clientregistration_controller.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/operator/internal/controller/clientregistration_controller.go b/operator/internal/controller/clientregistration_controller.go index e5ef429e..36bd22d5 100644 --- a/operator/internal/controller/clientregistration_controller.go +++ b/operator/internal/controller/clientregistration_controller.go @@ -287,7 +287,9 @@ func (r *ClientRegistrationReconciler) reconcileOne( } // Fetch JWT-SVID from SPIRE via Workload API - jwtSVID, err := r.fetchJWTSVID(ctx, ab.KeycloakRealm) + // The audience must be the Keycloak token endpoint URL for JWT-SPIFFE authentication + tokenEndpoint := strings.TrimSuffix(ab.KeycloakURL, "/") + "/realms/" + ab.KeycloakRealm + "/protocol/openid-connect/token" + jwtSVID, err := r.fetchJWTSVID(ctx, tokenEndpoint) if err != nil { logger.Error(err, "JWT-SVID fetch failed") if r.Recorder != nil { From afce8615fa2962e2a4aa40f7543cc7bc7c3a20e2 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 15:15:54 -0400 Subject: [PATCH 6/8] fix: Query Keycloak OIDC discovery to get correct JWT-SVID audience 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 --- .../clientregistration_controller.go | 55 ++++++++++++++++++- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/operator/internal/controller/clientregistration_controller.go b/operator/internal/controller/clientregistration_controller.go index 36bd22d5..40200c4b 100644 --- a/operator/internal/controller/clientregistration_controller.go +++ b/operator/internal/controller/clientregistration_controller.go @@ -9,7 +9,9 @@ package controller import ( "context" + "encoding/json" "fmt" + "net/http" "sort" "strings" "time" @@ -287,9 +289,20 @@ func (r *ClientRegistrationReconciler) reconcileOne( } // Fetch JWT-SVID from SPIRE via Workload API - // The audience must be the Keycloak token endpoint URL for JWT-SPIFFE authentication - tokenEndpoint := strings.TrimSuffix(ab.KeycloakURL, "/") + "/realms/" + ab.KeycloakRealm + "/protocol/openid-connect/token" - jwtSVID, err := r.fetchJWTSVID(ctx, tokenEndpoint) + // 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) + if err != nil { + logger.Error(err, "Failed to get Keycloak issuer URL") + if r.Recorder != nil { + r.Recorder.Eventf(owner, corev1.EventTypeWarning, "IssuerLookupFailed", + "Failed to query Keycloak OIDC discovery: %v", err) + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil + } + + jwtSVID, err := r.fetchJWTSVID(ctx, realmIssuer) if err != nil { logger.Error(err, "JWT-SVID fetch failed") if r.Recorder != nil { @@ -639,3 +652,39 @@ func (r *ClientRegistrationReconciler) fetchJWTSVID(ctx context.Context, audienc return svid.Marshal(), nil } + +// getKeycloakIssuer queries the Keycloak OIDC discovery endpoint to get the authoritative +// issuer URL. This is necessary because the issuer may be a public URL (e.g., keycloak.localtest.me) +// while the KeycloakURL in authbridge-config is the in-cluster service address. +func (r *ClientRegistrationReconciler) getKeycloakIssuer(ctx context.Context, keycloakURL, realm string) (string, error) { + discoveryURL := strings.TrimSuffix(keycloakURL, "/") + "/realms/" + realm + "/.well-known/openid-configuration" + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create OIDC discovery request: %w", err) + } + + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("failed to query OIDC discovery endpoint: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("OIDC discovery returned status %d", resp.StatusCode) + } + + var config struct { + Issuer string `json:"issuer"` + } + if err := json.NewDecoder(resp.Body).Decode(&config); err != nil { + return "", fmt.Errorf("failed to decode OIDC discovery response: %w", err) + } + + if config.Issuer == "" { + return "", fmt.Errorf("OIDC discovery response missing issuer field") + } + + return config.Issuer, nil +} From c0b7d1d1633165147e3db5273eabeaba9e160e9d Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Thu, 27 Aug 2026 16:24:02 -0400 Subject: [PATCH 7/8] fix: check error from resp.Body.Close() in getKeycloakIssuer Fixes errcheck linter error: Error return value of `resp.Body.Close` is not checked Assisted-By: Claude Code Signed-off-by: Alan Cha --- .../internal/controller/clientregistration_controller.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/operator/internal/controller/clientregistration_controller.go b/operator/internal/controller/clientregistration_controller.go index 40200c4b..6194d83c 100644 --- a/operator/internal/controller/clientregistration_controller.go +++ b/operator/internal/controller/clientregistration_controller.go @@ -669,7 +669,11 @@ func (r *ClientRegistrationReconciler) getKeycloakIssuer(ctx context.Context, ke if err != nil { return "", fmt.Errorf("failed to query OIDC discovery endpoint: %w", err) } - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + ctrl.Log.WithName("getKeycloakIssuer").Error(closeErr, "failed to close response body") + } + }() if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("OIDC discovery returned status %d", resp.StatusCode) From d1cf49ac67a0736b7451e658e248484cbaca313a Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Fri, 28 Aug 2026 12:47:37 -0400 Subject: [PATCH 8/8] perf: cache OIDC issuer and reuse workloadapi.Client Add caching to reduce HTTP and gRPC overhead on frequent reconciles: - getKeycloakIssuer(): Cache issuer URL by (keycloakURL, realm) using sync.Map. Issuer is stable per realm, no need to query OIDC discovery endpoint repeatedly. - fetchJWTSVID(): Reuse long-lived workloadapi.Client instead of creating new gRPC connection on every call. Lazily initialize with mutex protection. Addresses suggestions in PR #522 review from cwiklik. Signed-off-by: Alan Cha --- .../operator/templates/manager/manager.yaml | 4 +- charts/operator/values.yaml | 2 - .../clientregistration_controller.go | 42 +++++++++++++++---- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/charts/operator/templates/manager/manager.yaml b/charts/operator/templates/manager/manager.yaml index 5d8779a2..e4641a0f 100644 --- a/charts/operator/templates/manager/manager.yaml +++ b/charts/operator/templates/manager/manager.yaml @@ -180,13 +180,11 @@ spec: mountPath: /tmp/k8s-metrics-server/metrics-certs readOnly: true {{- end }} - {{- if .Values.verifiedFetch.enabled }} + {{- if or .Values.verifiedFetch.enabled (and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled) }} - name: spiffe-workload-api mountPath: /spiffe-workload-api readOnly: true {{- end }} - {{- if and .Values.spiffe .Values.spiffe.enabled .Values.spiffe.operatorAuth .Values.spiffe.operatorAuth.enabled }} - {{- end }} securityContext: {{- toYaml .Values.controllerManager.securityContext | nindent 8 }} serviceAccountName: {{ .Values.controllerManager.serviceAccountName }} diff --git a/charts/operator/values.yaml b/charts/operator/values.yaml index f1337614..5e884799 100644 --- a/charts/operator/values.yaml +++ b/charts/operator/values.yaml @@ -205,8 +205,6 @@ spiffe: enabled: false operatorAuth: enabled: false - # Path to JWT-SVID file written by spiffe-helper sidecar - jwtSVIDPath: "/opt/jwt_svid.token" # Feature gates — highest-priority layer in the injection precedence chain. # Set globalEnabled to false to disable ALL sidecar injection (kill switch). diff --git a/operator/internal/controller/clientregistration_controller.go b/operator/internal/controller/clientregistration_controller.go index 6194d83c..be47b8f8 100644 --- a/operator/internal/controller/clientregistration_controller.go +++ b/operator/internal/controller/clientregistration_controller.go @@ -14,6 +14,7 @@ import ( "net/http" "sort" "strings" + "sync" "time" "github.com/spiffe/go-spiffe/v2/svid/jwtsvid" @@ -89,6 +90,16 @@ type ClientRegistrationReconciler struct { // Only used when UseSpiffeAuth is true. OperatorClientID string + // keycloakIssuerCache caches Keycloak issuer URLs by (keycloakURL, realm) to avoid + // OIDC discovery HTTP requests on every reconcile. The issuer is stable per realm. + keycloakIssuerCache sync.Map // map[string]string + + // workloadAPIClientMu protects workloadAPIClient initialization + workloadAPIClientMu sync.Mutex + // workloadAPIClient is a reused connection to the SPIRE Workload API to avoid + // opening a new gRPC connection on every JWT-SVID fetch. Lazily initialized. + workloadAPIClient *workloadapi.Client + // Recorder emits Kubernetes Events to surface configuration issues visible in kubectl describe. Recorder record.EventRecorder } @@ -631,17 +642,20 @@ func (r *ClientRegistrationReconciler) SetupWithManager(mgr ctrl.Manager) error // fetchJWTSVID fetches a JWT-SVID from the SPIRE Workload API for the given audience. // Returns the JWT token as a string or an error if fetching fails. +// Reuses a long-lived workloadapi.Client to avoid gRPC connection churn on frequent reconciles. func (r *ClientRegistrationReconciler) fetchJWTSVID(ctx context.Context, audience string) (string, error) { - client, err := workloadapi.New(ctx, workloadapi.WithAddr(r.SpiffeSocket)) - if err != nil { - return "", fmt.Errorf("failed to create SPIFFE Workload API client: %w", err) - } - defer func() { - if closeErr := client.Close(); closeErr != nil { - // Log close error but don't override the function's return error - ctrl.Log.WithName("fetchJWTSVID").Error(closeErr, "failed to close SPIFFE Workload API client") + // Lazily initialize the workloadapi.Client + r.workloadAPIClientMu.Lock() + if r.workloadAPIClient == nil { + client, err := workloadapi.New(ctx, workloadapi.WithAddr(r.SpiffeSocket)) + if err != nil { + r.workloadAPIClientMu.Unlock() + return "", fmt.Errorf("failed to create SPIFFE Workload API client: %w", err) } - }() + r.workloadAPIClient = client + } + client := r.workloadAPIClient + r.workloadAPIClientMu.Unlock() svid, err := client.FetchJWTSVID(ctx, jwtsvid.Params{ Audience: audience, @@ -656,7 +670,14 @@ func (r *ClientRegistrationReconciler) fetchJWTSVID(ctx context.Context, audienc // getKeycloakIssuer queries the Keycloak OIDC discovery endpoint to get the authoritative // issuer URL. This is necessary because the issuer may be a public URL (e.g., keycloak.localtest.me) // while the KeycloakURL in authbridge-config is the in-cluster service address. +// Results are cached as the issuer is stable per realm. func (r *ClientRegistrationReconciler) getKeycloakIssuer(ctx context.Context, keycloakURL, realm string) (string, error) { + // Check cache first + cacheKey := keycloakURL + ":" + realm + if cached, ok := r.keycloakIssuerCache.Load(cacheKey); ok { + return cached.(string), nil + } + discoveryURL := strings.TrimSuffix(keycloakURL, "/") + "/realms/" + realm + "/.well-known/openid-configuration" req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil) @@ -690,5 +711,8 @@ func (r *ClientRegistrationReconciler) getKeycloakIssuer(ctx context.Context, ke return "", fmt.Errorf("OIDC discovery response missing issuer field") } + // Cache the issuer for future reconciles + r.keycloakIssuerCache.Store(cacheKey, config.Issuer) + return config.Issuer, nil }