Skip to content

fix: generate AuthBridge routes in separate ConfigMap with correct format #517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Alan-Cha merged 9 commits into from
Aug 28, 2026
Merged

fix: generate AuthBridge routes in separate ConfigMap with correct format #517

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ba17e4a
fix(webhook): generate routes in separate ConfigMap, reference via file
Alan-Cha Aug 19, 2026
dbaacb5
fix(webhook): generate routes in AuthBridge's routing.Route format
Alan-Cha Aug 26, 2026
6c460f1
test: update test calls for ensurePerAgentConfigMap's third return value
Alan-Cha Aug 26, 2026
3ed7f87
test: update auth routes test for separate ConfigMap and flat route f…
Alan-Cha Aug 26, 2026
d4a121a
refactor: extract AuthProxy path constants and add multiple-audience …
Alan-Cha Aug 27, 2026
66f13f1
chore: remove backup files from sed operations
Alan-Cha Aug 27, 2026
2bf20d8
fix(webhook): remove shadowed err variable and add routes volume over…
Alan-Cha Aug 27, 2026
b639e85
fix(lint): replace hardcoded 'authproxy-routes' strings with Authprox…
Alan-Cha Aug 27, 2026
c9f1708
fix(webhook): warn when hostRegex is used (expects glob, not regex sy…
Alan-Cha Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 79 additions & 29 deletions operator/internal/webhook/injector/pod_mutator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp
listenerOverrides["reverse_proxy_addr"] = fmt.Sprintf(":%d", originalAgentPort)
listenerOverrides["reverse_proxy_backend"] = fmt.Sprintf("http://127.0.0.1:%d", newAgentPort)
}
perAgentCMName, err := m.ensurePerAgentConfigMap(ctx, namespace, crName,
perAgentCMName, routesCMName, err := m.ensurePerAgentConfigMap(ctx, namespace, crName,
ModeProxySidecar, nsConfig.AuthBridgeRuntimeYAML, nsConfig,
listenerOverrides,
mtlsMode, tlsBridgeMode, spireEnabled, agentRuntime)
Expand Down Expand Up @@ -875,6 +875,12 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp
// requiredVolumes is always set above (resolved or legacy path) before
// the mode switch, so it is never nil here.
proxyVolumes := overrideAuthBridgeConfigMapInVolumes(requiredVolumes, perAgentCMName)

// Override authproxy-routes volume if routes ConfigMap was created
if routesCMName != "" {
proxyVolumes = overrideRoutesConfigMapInVolumes(proxyVolumes, routesCMName)
}

for i := range proxyVolumes {
if !volumeExists(podSpec.Volumes, proxyVolumes[i].Name) {
podSpec.Volumes = append(podSpec.Volumes, proxyVolumes[i])
Expand Down Expand Up @@ -945,13 +951,18 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp
// data plane terminates the actual TLS — DownstreamTlsContext on the
// inbound listener (gated on MTLSEnabled) and UpstreamTlsContext on
// original_destination_tls (strict only).
perAgentCMName, err := m.ensurePerAgentConfigMap(ctx, namespace, crName,
perAgentCMName, routesCMName, err := m.ensurePerAgentConfigMap(ctx, namespace, crName,
ModeEnvoySidecar, nsConfig.AuthBridgeRuntimeYAML, nsConfig, nil, mtlsMode, "", spireEnabled, agentRuntime) // bridge never runs under envoy-sidecar
if err != nil {
return false, fmt.Errorf("envoy-sidecar per-agent ConfigMap: %w", err)
}
requiredVolumes = overrideAuthBridgeConfigMapInVolumes(requiredVolumes, perAgentCMName)

// Override authproxy-routes volume if routes ConfigMap was created

@cwiklik cwiklik Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit / question — this envoy-sidecar path also generates authbridge-routes-<crName> and overrides the volume, but routes drive the proxy-sidecar token-exchange plugin, and the comment above notes the bridge "never runs under envoy-sidecar." Is the routes ConfigMap intended in this mode, or is it dead work? If it's never consumed here, gating routes generation on ModeProxySidecar would avoid creating an orphan ConfigMap.

if routesCMName != "" {
requiredVolumes = overrideRoutesConfigMapInVolumes(requiredVolumes, routesCMName)
}

resolvedForEnvoy := ResolveConfig(currentConfig, nsConfig)
resolvedForEnvoy.MTLSMode = mtlsMode
envoyCMName, err := m.ensurePerAgentEnvoyConfigMap(ctx, namespace, crName, resolvedForEnvoy)
Expand Down Expand Up @@ -1174,7 +1185,7 @@ func (m *PodMutator) ensurePerAgentConfigMap(
tlsBridgeMode string,
spireEnabled bool,
agentRuntime *agentv1alpha1.AgentRuntime,
) (string, error) {
) (configCMName string, routesCMName string, err error) {
cmName := perAgentConfigMapName(crName)

// Parse the base YAML into a generic map
Expand Down Expand Up @@ -1263,10 +1274,14 @@ func (m *PodMutator) ensurePerAgentConfigMap(
// Routes tell AuthBridge which audiences to request when calling specific
// destinations. Routes are only effective when the namespace is configured
// with SPIFFE authentication (CLIENT_AUTH_TYPE=federated-jwt).
//
// Routes are written to a separate ConfigMap and mounted at /etc/authproxy/routes.yaml.
// The config.yaml references the file path rather than containing routes inline.
var routesData []byte
if agentRuntime != nil && agentRuntime.Spec.Auth != nil &&
len(agentRuntime.Spec.Auth.Outbound) > 0 {

// Navigate to pipeline.outbound.plugins[token-exchange].config
// Configure token-exchange plugin to read routes from file
pipeline, _ := cfg["pipeline"].(map[string]interface{})
if pipeline == nil {
mutatorLog.Info("WARN: no pipeline block found, cannot inject routes",
Expand All @@ -1282,7 +1297,7 @@ func (m *PodMutator) ensurePerAgentConfigMap(
mutatorLog.Info("WARN: no outbound plugins found, cannot inject routes",
"namespace", namespace, "crName", crName)
} else {
// Find the token-exchange plugin
// Find the token-exchange plugin and configure it to read routes from file
for i := range plugins {
plugin, _ := plugins[i].(map[string]interface{})
if plugin == nil {
Expand All @@ -1296,41 +1311,56 @@ func (m *PodMutator) ensurePerAgentConfigMap(
plugin["config"] = pluginConfig
}

// Generate routes from spec.auth.outbound
routes := make([]interface{}, 0, len(agentRuntime.Spec.Auth.Outbound))
for _, outboundRoute := range agentRuntime.Spec.Auth.Outbound {
route := map[string]interface{}{
"audiences": outboundRoute.Audiences,
}

// Add destination match (host or hostRegex)
destination := make(map[string]interface{})
if outboundRoute.Destination.Host != "" {
destination["host"] = outboundRoute.Destination.Host
}
if outboundRoute.Destination.HostRegex != "" {
destination["hostRegex"] = outboundRoute.Destination.HostRegex
}
route["destination"] = destination

routes = append(routes, route)
// Set routes to reference external file
pluginConfig["routes"] = map[string]interface{}{
"file": "/etc/authproxy/routes.yaml",
}

pluginConfig["routes"] = routes
mutatorLog.Info("injected token-exchange routes from AgentRuntime spec.auth",
"namespace", namespace, "crName", crName, "routeCount", len(routes))
mutatorLog.Info("configured token-exchange to read routes from file",
"namespace", namespace, "crName", crName, "routeCount", len(agentRuntime.Spec.Auth.Outbound))
break
}
}
}
}
}

// Generate routes.yaml content in AuthBridge's routing.Route format:
// - host: "hostname" (flat, not nested under destination)
// - target_audience: "audience" (single string, not audiences array)
routes := make([]interface{}, 0, len(agentRuntime.Spec.Auth.Outbound))
for _, outboundRoute := range agentRuntime.Spec.Auth.Outbound {
route := make(map[string]interface{})

// Host or HostRegex (flat fields, not nested)
if outboundRoute.Destination.Host != "" {
route["host"] = outboundRoute.Destination.Host
}
if outboundRoute.Destination.HostRegex != "" {
// AuthBridge router doesn't support hostRegex - use glob pattern in host field
route["host"] = outboundRoute.Destination.HostRegex

@clawgenti clawgenti Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mapping HostRegex directly to the host field is a silent semantic change: a regex pattern (e.g. .*\.team1\.svc\.cluster\.local) will be passed where AuthBridge expects a glob. If AuthBridge doesn't support regex syntax at all, this will silently fail to match at runtime with no observable error at admission time. Consider validating the regex-to-glob conversion or adding a more explicit warning/event.

@clawgenti clawgenti Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: HostRegex is being stored verbatim in the host field, but AuthBridge's router matches host as a glob, not a regex. A regex like .*\.team1\.svc\.cluster\.local won't match as a glob (*.team1.svc.cluster.local would). Consider logging a more actionable warning here (e.g. "HostRegex is not supported; treating value as a glob — ensure it uses glob syntax") or returning an error to surface the mismatch to the operator early.

@cwiklik cwiklik Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion — hostRegex is passed verbatim into AuthBridge's host field, which matches by glob, not regex. So a real regex value (e.g. .*\.team1\.svc.cluster.local) will silently never match at runtime — the only signal is the operator-log warning just above. The warning is a good addition; consider also validating/rejecting obvious regex metacharacters, or documenting the glob requirement on the CRD field (or renaming hostRegex), so users who follow the field name aren't surprised. Note the auth test asserts route2["host"] == .*\.team1\.svc.cluster.local, i.e. it encodes the (warned) passthrough of regex syntax into a glob field.

}

// target_audience is a single string, not array
// Take first audience if multiple specified
if len(outboundRoute.Audiences) > 0 {
route["target_audience"] = outboundRoute.Audiences[0]
Comment thread
Alan-Cha marked this conversation as resolved.

@clawgenti clawgenti Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the first audience is used and the rest are silently dropped — the warning log is good but callers relying on multiple audiences (possible today or via future API extensions) will get no error. Consider returning an error or at minimum documenting this limitation in the field's godoc/API type. Tracking issue #518 is referenced, but that issue should probably be linked in the AgentRuntime CRD validation too.

}

routes = append(routes, route)
}

var err error
Comment thread
Alan-Cha marked this conversation as resolved.
Outdated
routesData, err = yaml.Marshal(routes)
if err != nil {
return "", "", fmt.Errorf("failed to marshal routes for %s/%s: %w", namespace, crName, err)
}
}

// Marshal back to YAML
data, err := yaml.Marshal(cfg)
if err != nil {
return "", fmt.Errorf("failed to marshal per-agent config for %s/%s: %w", namespace, crName, err)
return "", "", fmt.Errorf("failed to marshal per-agent config for %s/%s: %w", namespace, crName, err)
}

// Server-side apply: atomic create-or-update in a single API call.
Expand All @@ -1346,12 +1376,32 @@ func (m *PodMutator) ensurePerAgentConfigMap(
}

if err := m.Client.Apply(ctx, cmApply, client.FieldOwner("rossoctl-webhook"), client.ForceOwnership); err != nil {
return "", fmt.Errorf("failed to apply per-agent ConfigMap %s/%s: %w", namespace, cmName, err)
return "", "", fmt.Errorf("failed to apply per-agent ConfigMap %s/%s: %w", namespace, cmName, err)
}
mutatorLog.Info("Applied per-agent ConfigMap",
"namespace", namespace, "name", cmName, "mode", mode, "mtlsMode", mtlsMode)

return cmName, nil
// Create separate routes ConfigMap if routes are present
if len(routesData) > 0 {
routesCMName := "authbridge-routes-" + crName
routesCMApply := applyconfigscorev1.ConfigMap(routesCMName, namespace).
WithLabels(map[string]string{managedByLabel: managedByValue}).
WithData(map[string]string{"routes.yaml": string(routesData)})

// Set same OwnerReference for garbage collection
if ownerRef := m.buildOwnerReference(ctx, namespace, crName); ownerRef != nil {
routesCMApply = routesCMApply.WithOwnerReferences(ownerRef)
}

if err := m.Client.Apply(ctx, routesCMApply, client.FieldOwner("rossoctl-webhook"), client.ForceOwnership); err != nil {
return "", "", fmt.Errorf("failed to apply routes ConfigMap %s/%s: %w", namespace, routesCMName, err)
}
mutatorLog.Info("Applied routes ConfigMap",
"namespace", namespace, "name", routesCMName, "routeCount", len(agentRuntime.Spec.Auth.Outbound))
return cmName, routesCMName, nil
}

return cmName, "", nil
}

// ensurePerAgentEnvoyConfigMap renders an envoy.yaml from the
Expand Down
56 changes: 38 additions & 18 deletions operator/internal/webhook/injector/pod_mutator_auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pipeline:
}

// Call ensurePerAgentConfigMap with the AgentRuntime
cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "weather-agent",
cmName, routesCMName, err := m.ensurePerAgentConfigMap(ctx, "team1", "weather-agent",
ModeProxySidecar, baseYAML, nsConfig, nil, "", "", true, agentRuntime)
if err != nil {
t.Fatalf("unexpected error: %v", err)
Expand Down Expand Up @@ -181,36 +181,56 @@ pipeline:
t.Fatal("token-exchange plugin not found")
}

routes, ok := tokenExchangeConfig["routes"].([]interface{})
// Verify routes is a file reference, not an inline array
routesRef, ok := tokenExchangeConfig["routes"].(map[string]interface{})
if !ok {
t.Fatal("routes not found or not an array")
t.Fatal("routes not found or not a map")
}
if routesRef["file"] != "/etc/authproxy/routes.yaml" {
t.Errorf("routes file mismatch: got %v, want /etc/authproxy/routes.yaml", routesRef["file"])
}

// Fetch the routes ConfigMap
if routesCMName == "" {
t.Fatal("routesCMName is empty")
}
routesCM := &corev1.ConfigMap{}
if err := fakeClient.Get(ctx, client.ObjectKey{Namespace: "team1", Name: routesCMName}, routesCM); err != nil {
t.Fatalf("failed to get routes ConfigMap: %v", err)
}

// Parse routes.yaml
routesYAML, ok := routesCM.Data["routes.yaml"]
if !ok {
t.Fatal("routes ConfigMap missing routes.yaml key")
}

var routes []interface{}
if err := yaml.Unmarshal([]byte(routesYAML), &routes); err != nil {
t.Fatalf("failed to parse routes.yaml: %v", err)
}

// Verify we have 2 routes
if len(routes) != 2 {
t.Fatalf("expected 2 routes, got %d", len(routes))
}

// Verify first route (exact host match)
// Verify first route (exact host match) - now in flat format
route1, _ := routes[0].(map[string]interface{})
dest1, _ := route1["destination"].(map[string]interface{})
if dest1["host"] != "weather-tool-mcp.team1.svc.cluster.local" {
t.Errorf("route 1 host mismatch: got %v", dest1["host"])
if route1["host"] != "weather-tool-mcp.team1.svc.cluster.local" {
t.Errorf("route 1 host mismatch: got %v", route1["host"])
}
audiences1, _ := route1["audiences"].([]interface{})
if len(audiences1) != 1 || audiences1[0] != "spiffe://localtest.me/ns/team1/sa/weather-tool" {
t.Errorf("route 1 audiences mismatch: got %v", audiences1)
if route1["target_audience"] != "spiffe://localtest.me/ns/team1/sa/weather-tool" {
t.Errorf("route 1 target_audience mismatch: got %v", route1["target_audience"])
}

// Verify second route (regex match)
// Verify second route (regex match) - now in flat format
route2, _ := routes[1].(map[string]interface{})
dest2, _ := route2["destination"].(map[string]interface{})
if dest2["hostRegex"] != `.*\.team1\.svc\.cluster\.local` {
t.Errorf("route 2 hostRegex mismatch: got %v", dest2["hostRegex"])
if route2["host"] != `.*\.team1\.svc\.cluster\.local` {
t.Errorf("route 2 host mismatch: got %v", route2["host"])
}
audiences2, _ := route2["audiences"].([]interface{})
if len(audiences2) != 1 || audiences2[0] != "spiffe://localtest.me/ns/team1/sa/default" {
t.Errorf("route 2 audiences mismatch: got %v", audiences2)
if route2["target_audience"] != "spiffe://localtest.me/ns/team1/sa/default" {
t.Errorf("route 2 target_audience mismatch: got %v", route2["target_audience"])
}
}

Expand Down Expand Up @@ -250,7 +270,7 @@ pipeline:
nsConfig := &NamespaceConfig{}

// Call with nil agentRuntime
cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent",
cmName, _, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent",
ModeProxySidecar, baseYAML, nsConfig, nil, "", "", false, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
Expand Down
Loading
Loading