Skip to content

feat: ConfigMap-driven authbridge mode + flip default to proxy-sidecar - #361

Merged
huang195 merged 8 commits into
rossoctl:mainfrom
huang195:feat/configmap-driven-mode-default-proxy
May 15, 2026
Merged

feat: ConfigMap-driven authbridge mode + flip default to proxy-sidecar#361
huang195 merged 8 commits into
rossoctl:mainfrom
huang195:feat/configmap-driven-mode-default-proxy

Conversation

@huang195

Copy link
Copy Markdown
Member

Summary

Phase B of the authbridge sidecar consolidation (Phase A is kagenti-extensions PR #409). This PR makes proxy-sidecar the default deployment mode and replaces the kagenti.io/authbridge-mode pod annotation with a ConfigMap- and CR-driven resolution chain.

  • Mode selection is now AgentRuntime.Spec.AuthBridgeMode → namespace authbridge-runtime-config.modeproxy-sidecar fallback. No annotations.
  • Default mode is now proxy-sidecar (HTTP_PROXY env + authbridge-proxy container, no Envoy, no iptables).
  • Image set collapses to two combined images plus proxy-init. Spiffe-helper is bundled inside the combined images and gated by the per-workload SPIRE_ENABLED env var. Client-registration is operator-managed (in-pod sidecar path removed).
  • Feature gates drops combinedSidecar, spiffeHelper, clientRegistration.
  • CRD adds Spec.AuthBridgeMode with proxy-sidecar | envoy-sidecar | waypoint enum validation for per-workload overrides.

Net diff: +351 / −1592 lines. Mostly deletion of dead-code paths.

Resolution chain

1. AgentRuntime CR Spec.AuthBridgeMode  (per-workload override)
2. namespace authbridge-runtime-config  (cluster/namespace default)
3. proxy-sidecar                        (cluster-wide fallback)

Image set after this PR

Image Mode Contents
authbridge (default) proxy-sidecar authbridge-proxy + spiffe-helper
authbridge-envoy envoy-sidecar Envoy + ext_proc authbridge + spiffe-helper
proxy-init envoy-sidecar only iptables init container

Removed: authbridge-light, standalone spiffe-helper, standalone client-registration.

Test plan

  • go build ./... — clean
  • go vet ./... — clean
  • Unit tests (./internal/...) — all pass
  • Webhook integration tests (./internal/webhook/v1alpha1/...) — all pass
  • e2e suite (requires Kind cluster — run in CI)
  • Manual: deploy the new operator + new authbridge images to a kind cluster, deploy the weather-agent in team1 namespace, verify proxy-sidecar mode is the default, then create an authbridge-runtime-config ConfigMap with mode: envoy-sidecar in another namespace and verify a workload there gets envoy-sidecar shape.
  • Manual: per-workload override — deploy two AgentRuntime CRs in the same namespace, one with Spec.AuthBridgeMode: envoy-sidecar and one without, confirm both modes coexist.

Follow-ups

  • Phase C: kagenti chart values + UI dropdown for the mode selector (separate PR in kagenti/kagenti).
  • Older release tags continue to publish the dropped authbridge-light, standalone spiffe-helper, and standalone client-registration images; CI no longer builds them on this branch.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

Phase B of the authbridge sidecar consolidation. Replaces the
kagenti.io/authbridge-mode pod annotation with a ConfigMap- and CR-
driven resolution chain, flips the default mode from envoy-sidecar to
proxy-sidecar, and drops the now-redundant SpiffeHelper /
ClientRegistration / CombinedSidecar configuration surface.

Mode resolution chain (first non-empty wins):
  1. AgentRuntime CR Spec.AuthBridgeMode (per-workload override)
  2. Namespace authbridge-runtime-config mode field
  3. proxy-sidecar (cluster-wide fallback)

Image set collapses to two combined images plus proxy-init:
  * authbridge (proxy-sidecar mode, default): authbridge proxy
    + bundled spiffe-helper. No Envoy, no gRPC.
  * authbridge-envoy (envoy-sidecar mode): Envoy + ext_proc
    authbridge + bundled spiffe-helper.
  * proxy-init: iptables init container, envoy-sidecar mode only.

Spiffe-helper is no longer a separate sidecar -- it is bundled in the
combined images and gated per-workload by the SPIRE_ENABLED env var.
Client-registration is operator-managed entirely (the in-pod sidecar
path is gone).

Removed:
  * kagenti.io/authbridge-mode annotation constant + handling
  * combinedSidecar / spiffeHelper / clientRegistration feature gates
  * BuildSpiffeHelperContainer, BuildClientRegistrationContainer*,
    BuildAuthBridgeContainer builders
  * Images.AuthBridgeLight, Images.SpiffeHelper,
    Images.ClientRegistration config fields
  * InjectionDecision.ClientRegistration
  * logClientRegistrationPaths helper

Tests use new newAgentRuntimeWithMode / createAgentRuntimeWithMode
helpers to set mode via the CR field; combined-mode tests are deleted.

Net diff: +351 / -1592 lines.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
huang195 added 4 commits May 14, 2026 15:58
Convert '*' bullets to '-' in the FeatureGates type doc comment so
gofmt accepts the file. golangci-lint flagged this in CI.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Restore manual webhook manifest overrides clobbered by `make manifests`,
add a deprecated annotation fallback in the mode resolution chain, and
clean up minor code-review nits.

Per-comment changes:

rossoctl#1 (must-fix): Restore namespaceSelector / objectSelector / timeoutSeconds
on the inject.kagenti.io webhook in config/webhook/manifests.yaml. PR rossoctl#357
manually re-added these fields after `make manifests` clobbered them; the
prior commit on this branch wiped them again. Without them the webhook
fires on every pod admission cluster-wide which, combined with the new
proxy-sidecar default, is a much wider blast radius than today.

rossoctl#3 (suggestion): Re-introduce AnnotationAuthBridgeMode as a deprecated
fourth layer of the mode resolution chain (between the namespace
ConfigMap and the cluster-wide proxy-sidecar fallback). Workloads that
still set kagenti.io/authbridge-mode get the same mode they had before
this PR, with a `DEPRECATED:` log line per pod admission to push them
toward AgentRuntime.Spec.AuthBridgeMode.

rossoctl#4 (suggestion): Add a TODO on InjectionDecision.SpiffeHelper marking it
for rename to SpireEnabled in a follow-up PR.

rossoctl#5 (suggestion): Replace the `evaluateSidecar("spiffe-helper", true,
...)` magic-true call with a dedicated `evaluateSpiffeHelper(label)`
helper, mirroring the previous evaluateClientRegistration pattern.

rossoctl#6 (nit): Update the proxy-sidecar test error message that still
referenced the old "authbridge-light" image name.

Comment rossoctl#2 (spiffe-helper bundling readiness) is a coordination concern
between this PR and kagenti-extensions PR rossoctl#409. The combined Dockerfiles
on rossoctl#409's branch (76d2cb3) DO bundle spiffe-helper and the
entrypoint-proxy.sh / entrypoint-envoy.sh DO conditionally start it on
SPIRE_ENABLED — so end-state is correct as long as rossoctl#409 lands first.
No code change here; merge ordering is the answer.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
…ad code

Three classes of fixes from a cross-PR audit against kagenti-extensions
PR rossoctl#409 and the future kagenti chart PR:

e2e tests
  * test/e2e/e2e_suite_test.go: drop authbridge-light + standalone
    spiffe-helper images from the Kind preload list (PR rossoctl#409 no longer
    publishes them).
  * test/e2e/e2e_test.go: replace assertions that expect a separate
    spiffe-helper container with assertions that envoy-proxy carries
    SPIRE_ENABLED=true. Spiffe-helper is now bundled inside the
    combined images and gated by SPIRE_ENABLED, so it does not appear
    as its own container.

Resolution-chain unit-test coverage
  * pod_mutator_test.go: add five tests covering layers 2-4 of the
    mode resolution chain that were previously only exercised
    end-to-end:
      - namespace authbridge-runtime-config mode field selects mode
      - CR field beats namespace ConfigMap
      - deprecated kagenti.io/authbridge-mode annotation falls through
      - CR field beats annotation
      - cluster default is proxy-sidecar when nothing is set

Dead-code cleanup
  * Remove PodMutator.EnableClientRegistration field + the
    enableClientRegistration parameter to NewPodMutator. The field was
    set in cmd/main.go to a hardcoded false and never read anywhere
    after the in-pod client-registration path was removed.
  * Remove SidecarDefaults / SidecarDefault types and the
    Sidecars.EnvoyProxy.Enabled value. Defined and logged but never
    read for injection decisions; FeatureGates.EnvoyProxy is the
    actual knob. Drop the matching defaults.sidecars block from
    charts/kagenti-operator/values.yaml.
  * Remove AuthBridgeContainerName ("authbridge"). It was the old
    combined-mode container name, never used to name an actually
    injected container after PR B. Drop the matching skip-list entry
    in authbridge_webhook.isAlreadyInjected and the obsolete
    "combined authbridge container (idempotency)" test.
  * Remove SpiffeHelperContainerName / ClientRegistrationContainerName
    constants and their entries in pod_mutator's port-stealing and
    HTTP_PROXY-injection skip-lists, plus the matching webhook
    isAlreadyInjected check. Neither container is ever injected
    anymore; the filters could never match.

Net diff: +197 / -124. Test pass: full unit + integration suite green.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Two CI failures on the previous commit:

1) e2e: envoy-proxy container failed to start with
   "make mountpoint /shared/client-secret.txt: read-only file system".
   The new authbridge-envoy combined image is built on ubi9-micro,
   which has a read-only rootfs and no /shared directory. With shared-
   data mounted ReadOnly:true on /shared, runc cannot create the
   subPath bind-mount targets for /shared/client-id.txt and
   /shared/client-secret.txt that ApplyKeycloakClientCredentialsSecret
   Volumes adds. The proxy-sidecar BuildProxySidecarContainer mount
   was already RW; only the envoy-proxy mount in
   BuildEnvoyProxyContainerWithSpireOption needed the change.

2) lint: two long-line violations in the SPIRE_ENABLED env-var
   jsonpath assertions added in the previous commit. Split them
   across multiple lines.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well-structured consolidation: 5 sidecar containers → 2 combined images, clean 4-layer mode resolution chain (CR → namespace ConfigMap → deprecated annotation → proxy-sidecar fallback). Net deletion of ~780 lines of dead code with thorough test coverage additions (5 new resolution-chain unit tests, updated e2e assertions).

Areas reviewed: Go, Helm, CRD, Tests, Security
Commits: 5 commits, all signed-off ✅
CI: All 15 checks passing (E2E, unit, integration, lint, helm lint, shellcheck, hadolint, CodeQL, Trivy, DCO)
Webhook selectors: Verified namespaceSelector, objectSelector, timeoutSeconds intact on HEAD

Two minor suggestions for improved observability below — not blocking.

Assisted-By: Claude Code

Mode string `json:"mode"`
}
if err := yaml.Unmarshal([]byte(authbridgeYAML), &top); err != nil {
return ""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: ExtractMode silently returns "" on YAML parse error, which causes the resolution chain to fall through to the next layer. While this is fail-safe behavior, a malformed authbridge-runtime-config ConfigMap would be silently ignored with no signal to the operator.

Consider logging a warning when yaml.Unmarshal fails, so cluster operators can catch misconfigured ConfigMaps.

if err := yaml.Unmarshal([]byte(authbridgeYAML), &top); err != nil {
	// Consider: log.Log.Info("WARN: failed to parse authbridge-runtime-config", "error", err)
	return ""
}

authBridgeMode = ModeProxySidecar
modeSource = "cluster-default"
}
mutatorLog.Info("resolved authbridge mode",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: The ConfigMap and deprecated-annotation layers can return arbitrary strings (unlike the CRD path which has kubebuilder enum validation). An unrecognized mode value (e.g. typo "proxy-sidecart") would flow through to the envoy-sidecar branch without warning.

The failure mode is safe (envoy-sidecar is the more restrictive mode), but adding a validation check after resolution would help operators catch ConfigMap typos:

switch authBridgeMode {
case ModeProxySidecar, ModeEnvoySidecar, ModeWaypoint:
	// valid
default:
	mutatorLog.Info("WARN: unrecognized authBridgeMode, defaulting to proxy-sidecar",
		"mode", authBridgeMode, "source", modeSource)
	authBridgeMode = ModeProxySidecar
	modeSource = "cluster-default-invalid-fallback"
}

huang195 added 3 commits May 14, 2026 20:21
The kagenti-extensions PR that introduced cmd/authbridge-lite (a
size-optimized authbridge build with parsers dropped) ships an
authbridge-lite combined image alongside authbridge and
authbridge-envoy. This change wires it into the operator so workloads
can opt into the lite image via mode=lite.

Lite is structurally a proxy-sidecar deployment — same listeners
(reverse + forward HTTP proxy), same ports, same per-agent
authbridge-runtime-config ConfigMap shape, same SPIRE wiring. The
only difference is the container image: Images.AuthBridgeLite instead
of Images.AuthBridge. The bundled authbridge-lite binary still
declares mode=proxy-sidecar in its YAML config (lite is a build
variant, not a runtime mode), so the per-agent ConfigMap's `mode:`
field stays "proxy-sidecar" regardless.

Changes:

  * api/v1alpha1/agentruntime_types.go: AuthBridgeMode kubebuilder
    enum extended to {proxy-sidecar, envoy-sidecar, lite, waypoint}.
    Doc comment updated.
  * config/types.go + defaults.go + loader.go: new
    Images.AuthBridgeLite field (default
    `ghcr.io/kagenti/kagenti-extensions/authbridge-lite:latest`),
    Validate() requires it non-empty, logger reports it.
  * injector/constants.go: ModeLite = "lite".
  * injector/container_builder.go: BuildProxySidecarContainerWithPorts
    now takes an `image` arg so the same builder serves both
    proxy-sidecar (Images.AuthBridge) and lite (Images.AuthBridgeLite).
    BuildProxySidecarContainer's default-port wrapper passes
    Images.AuthBridge.
  * injector/pod_mutator.go: mode switch's proxy-sidecar branch now
    also matches ModeLite. Picks proxyImage based on mode and threads
    it through the builder + the "injection complete" log.
  * charts/kagenti-operator/values.yaml: defaults.images.authbridgeLite.
  * config/crd/bases/agent.kagenti.dev_agentruntimes.yaml: regenerated.
  * Tests: TestInjectAuthBridge_LiteMode_UsesAuthBridgeLiteImage,
    TestInjectAuthBridge_LiteMode_FromNamespaceConfigMap.

Verified locally: go build / vet / test all green; the lite tests
confirm mode=lite (via CR field or namespace ConfigMap) produces a
proxy-sidecar pod shape that uses Images.AuthBridgeLite.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Adjacent field assignments need a single column alignment; the new
AuthBridgeLite field shifted the ProxyInit / PullPolicy column.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
…e config

Two suggestions from PR rossoctl#361 review (pdettori):

* ExtractMode silently swallowed yaml.Unmarshal failures, so a
  malformed authbridge-runtime-config ConfigMap would be ignored
  with no signal. Now logs a WARN before returning the empty string
  that triggers the resolution chain's fall-through.

* The mode-resolution chain accepted arbitrary strings from the
  ConfigMap and the deprecated annotation (the CRD path is
  enum-validated by the API server, but those two are not). A typo
  like "proxy-sidecart" would flow through to the envoy-sidecar
  branch silently — neither what the operator intended nor a safe
  default. Added a switch over the resolved mode after the chain:
  unrecognized values get logged with a WARN and fall back to
  proxy-sidecar (matches the cluster-default behavior, source tag
  becomes "cluster-default-invalid-fallback" so the ConfigMap typo
  is visible in operator logs).

Includes the lite mode in the recognized set so the new ModeLite
addition isn't tripped by the validator.

New test: TestInjectAuthBridge_ModeResolution_UnrecognizedFallsBackToProxySidecar
verifies the fallback path with a "proxy-sidecart" typo in the
namespace ConfigMap.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants