feat(images): Unify combined sidecar images, drop client-registration#409
Merged
Merged
Conversation
huang195
force-pushed
the
feat/unified-sidecar-images
branch
from
May 14, 2026 15:30
fa6db56 to
001ad05
Compare
Consolidates the authbridge container images down to two combined
variants, each bundling spiffe-helper that starts conditionally on
SPIRE_ENABLED. Removes the in-pod client-registration sidecar from
this repo (operator now owns Keycloak client registration).
## Image layout (after)
| Name | Built from | Contents |
|---|---|---|
| `authbridge` (proxy-sidecar combined, default mode) | cmd/authbridge/Dockerfile.proxy | authbridge-proxy + spiffe-helper |
| `authbridge-envoy` (envoy-sidecar combined) | cmd/authbridge/Dockerfile.envoy | Envoy + authbridge (ext_proc) + spiffe-helper |
| `proxy-init` | authproxy/Dockerfile.init | iptables init for envoy-sidecar mode |
Both combined images use the same supervisor pattern:
* If SPIRE_ENABLED=true: start spiffe-helper in background, append
its PID to CRITICAL_PIDS.
* Start authbridge-proxy / Envoy + authbridge in background, append.
* `wait -n $CRITICAL_PIDS` blocks until any critical process exits.
* On SIGTERM/SIGINT: graceful shutdown, exit 0.
* On any critical-process death: kill the others, exit 1, kubelet
restarts the pod. No silent process death.
## Removals
* `authproxy/Dockerfile.authbridge` (old envoy+everything combined)
* `authproxy/entrypoint-authbridge.sh`
* `authproxy/authbridge-combined.yaml` (was the bake-in YAML for
the old combined image; operator-supplied per-agent ConfigMap
superseded it)
* `client-registration/` (entire directory: Dockerfile,
Python script, requirements, example manifests, images).
Operator now creates Keycloak clients via its
ClientRegistration controller and mounts the resulting Secret
at /shared/client-id.txt + /shared/client-secret.txt — same
paths the in-pod sidecar wrote to. Workloads opting back into
the legacy in-pod path use `kagenti.io/client-registration-inject:
"true"`; that path now sources the image from kagenti-operator.
* `spiffe-helper/Dockerfile` (standalone spiffe-helper image —
bundled into both combined images now).
* `tests/test_client_registration.py` (covered the removed Python).
* `TestAuthbridgeCombinedYAML_Loads` (asserted the deleted YAML).
## Renames
* `cmd/authbridge/Dockerfile` → `cmd/authbridge/Dockerfile.envoy`
* `cmd/authbridge/Dockerfile.light` → `cmd/authbridge/Dockerfile.proxy`
* `cmd/authbridge/entrypoint.sh` → `cmd/authbridge/entrypoint-envoy.sh`
The proxy variant uses `authbridge-proxy` (lite binary from PR rossoctl#407,
no gRPC, no envoy types) and switches base from distroless/static to
alpine. Distroless/static has no shell, which the bash supervisor
needs; alpine is +3 MB but has bash readily and matches the envoy
variant's pattern. Image size ends up around ~48 MB (vs ~12 MB for
the no-spiffe-helper variant) — spiffe-helper bundle is the bulk;
size optimization is deferred to a follow-up.
The envoy variant adds spiffe-helper to the existing UBI9-micro base
(already has bash + glibc, both required). Image size ~165 MB (vs
~141 MB without spiffe-helper).
## CI matrix
Drops 6 entries (client-registration, auth-proxy, spiffe-helper,
authbridge-unified, authbridge-light, demo-app), retains 3:
* `proxy-init` (still needed for envoy-sidecar mode iptables)
* `authbridge-envoy`
* `authbridge`
The `auth-proxy` and `demo-app` source files stay in-tree for the
standalone quickstart; just not built/published by CI.
## Out of scope (Phases B + C)
Operator + chart coordination:
* kagenti-operator must flip default mode from envoy-sidecar to
proxy-sidecar.
* Operator's image-name slot for proxy-sidecar mode (`Images.AuthBridgeLight`)
should be renamed and pointed at the new `authbridge` image.
The new image bundles spiffe-helper, so the operator should stop
injecting spiffe-helper as a separate sidecar in proxy-sidecar
mode (envoy-sidecar mode unchanged).
* `combinedSidecar` feature gate cleanup.
* kagenti chart values pin update.
These changes don't break anything in this PR because operator/chart
pin to existing image tags published by older releases of this repo.
A new release should not be cut from this branch until B + C are
ready.
## Verification
* `go build ./...` clean across authlib, cmd/authbridge,
cmd/authbridge-proxy, cmd/authbridge-envoy.
* `go test -race ./...` passes in authlib + cmd/authbridge.
* `podman build` succeeds for both Dockerfiles.
* Built image sizes (linux/amd64):
authbridge:test 47.8 MB
authbridge-envoy:test 165 MB
## Follow-ups (not blocking)
* Demo manifests in `authbridge/demos/**/k8s/` reference deleted
image names — update after Phase B + C land. Most likely require
rewriting the deployment shape (proxy-sidecar default), so
delaying until operator semantics settle is correct.
* Doc references in `authbridge/README.md`, `authbridge/docs/*`,
`authbridge/cmd/authbridge/README.md`, `LOCAL_TESTING_GUIDE.md`,
`local-build-and-test.sh` — find/replace pass.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
huang195
force-pushed
the
feat/unified-sidecar-images
branch
from
May 14, 2026 15:34
001ad05 to
76d2cb3
Compare
Three files referenced the now-deleted authbridge/client-registration/
or stale image names. Reviewer caught the bisectability hole; this is
the broader sweep.
* local-build-and-test.sh — line 73 unconditionally did
"cd authbridge/client-registration && podman build". Would fail
"no such directory" the moment anyone ran the script after
merge. Also: line 84 referenced authproxy/Dockerfile.envoy which
was deleted earlier (script was already partially broken
pre-PR). Rewrote the matrix to build the three published images:
authbridge (proxy-sidecar combined), authbridge-envoy
(envoy-sidecar combined), proxy-init.
* .github/workflows/security-scans.yaml — Bandit loop iterated
over "authbridge/client-registration authbridge tests" guarded
by [ -d "$dir" ]. Doesn't crash (graceful skip), but the dead
dir name is misleading — dropped from the loop.
* authbridge/authproxy/Makefile — comment mentioning
"authbridge-unified" build path. Stale text only, no breakage.
Updated to point at Dockerfile.proxy and Dockerfile.envoy.
Verified clean (no breakage from this PR):
* tests/test_keycloak_sync.py — imports authbridge/keycloak_sync.py
which has no client-reg deps
* .pre-commit-config.yaml — clean
* All other workflows (ci.yaml, project, scorecard, spellcheck,
stale, self-assign, pr-verifier) — no client-reg / image refs
* Spellcheck dictionary — clean
* cmd/authbridge + authlib — Go build clean
Demo manifests in authbridge/demos/**/k8s/ still reference deleted
image tags. They continue to work because ghcr.io retains the old
:latest tags (just no longer refreshed). The deployment shape they
encode is going away in Phase B anyway, so deferring rewrite to
that PR is correct.
Cross-repo references (kagenti-operator + kagenti chart) are Phase B
+ C territory, called out in the PR body.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
esnible
approved these changes
May 14, 2026
esnible
left a comment
Member
There was a problem hiding this comment.
Summary
Careful, well-documented image consolidation. The PR description is unusually thorough (test plan, sizes, follow-ups). Verified end-to-end:
- Shell supervisors are correctly written (
set -eu, signal trap, intentional# shellcheck disable=SC2086comments where word-splitting on$CRITICAL_PIDSis desired). - All
FROMlines pin a digest where appropriate; both runtime stages setUSER(1001 / 1337); both bases ship bash 5.x sowait -nworks. - Go changes are pure comment updates — no behavior change.
- Cross-repo claim verified:
kagenti-operator/internal/webhook/injector/keycloak_client_credentials.go:104-105mounts at exactly/shared/client-id.txtand/shared/client-secret.txt— the same paths the deletedclient-registrationsidecar wrote to. The handoff is correct. - Removed
TestAuthbridgeCombinedYAML_Loadsis justified (the YAML it tested is gone).
Areas reviewed: Shell, Dockerfiles, Go (comments), GitHub Actions, cross-repo (operator)
Commits: 2, both signed-off
CI status: 17/17 passing
Assisted-By: Claude Code
| CRITICAL_PIDS="$CRITICAL_PIDS $!" | ||
|
|
||
| # Give authbridge a moment to bind the gRPC listener before Envoy connects | ||
| sleep 2 |
Member
There was a problem hiding this comment.
suggestion: The hardcoded sleep 2 between authbridge and Envoy is a race-window in two directions:
- If authbridge is slow to bind on a busy node (e.g. cold container, cgroup throttling), Envoy will fail its first ext_proc connection attempt and may crash before the listener is ready —
wait -nthen exits the container and kubelet restarts the pod, masking the issue as a slow startup loop. - If authbridge binds in ~100ms (the common case), you've added ~1.9 s of cold-start latency to every pod for no benefit.
A listener probe is more robust and usually faster, e.g.:
for i in $(seq 1 50); do
if (echo > /dev/tcp/127.0.0.1/<grpc-port>) 2>/dev/null; then break; fi
sleep 0.1
done(bash has /dev/tcp built in, no nc needed.) Or accept the simplicity tradeoff and document why sleep 2 is sufficient for the deployment shapes you target.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates the authbridge container images down to two combined
sidecar variants, each bundling
spiffe-helperthat startsconditionally based on
SPIRE_ENABLED. Removes the in-podclient-registrationsidecar from this repo entirely (operator nowowns Keycloak client registration end-to-end).
Sets up the kagenti-extensions side of the larger work to flip the
default deployment mode from
envoy-sidecartoproxy-sidecar.Operator + chart updates land in follow-up PRs (Phase B + C).
Image layout
authbridge(proxy-sidecar combined, default mode)cmd/authbridge/Dockerfile.proxyauthbridge-envoy(envoy-sidecar combined)cmd/authbridge/Dockerfile.envoyproxy-init(unchanged)authproxy/Dockerfile.initSizes are linux/amd64 (
podman build, no extra strip). Sizeoptimization deferred to follow-ups.
Process supervision pattern
Both combined images use the same bash supervisor:
Verified behavior:
restarts. No silent process death.
SPIRE_ENABLEDis unset/false.Removals
authproxy/Dockerfile.authbridge(old Envoy + everything combined)authproxy/entrypoint-authbridge.shauthproxy/authbridge-combined.yaml(operator-supplied per-agentConfigMap supersedes the bake-in YAML)
client-registration/— entire directory. Operator'sClientRegistrationcontroller creates Keycloak clients and thewebhook mounts the Secret at
/shared/client-id.txt+/shared/client-secret.txt(same paths the in-pod sidecarwrote to).
spiffe-helper/Dockerfile(standalone — bundled into both combinedimages now).
tests/test_client_registration.py(covered removed Python).TestAuthbridgeCombinedYAML_Loads(asserted deleted YAML).Renames
cmd/authbridge/Dockerfile→Dockerfile.envoycmd/authbridge/Dockerfile.light→Dockerfile.proxycmd/authbridge/entrypoint.sh→entrypoint-envoy.shChanges to
Dockerfile.envoy(renamedDockerfile)Adds spiffe-helper bundling stage + conditional start in entrypoint.
UBI9-micro base unchanged (has bash + glibc, both needed).
Changes to
Dockerfile.proxy(renamedDockerfile.light)cmd/authbridge(full binary, ~17 MB) tocmd/authbridge-proxy(lite binary from PR feat(cmd): Add authbridge-proxy and authbridge-envoy lite binaries #407, no gRPC, ~9 MB).distroless/statictoalpine:3.20. Distrolesshas no shell — bash supervisor needs one. Alpine adds ~3 MB but
matches the envoy variant's pattern.
CI matrix
.github/workflows/build.yamlcut from 9 entries to 3:proxy-init(still needed for envoy-sidecar iptables)authbridge-envoyauthbridgeRemoved:
client-registration,auth-proxy,spiffe-helper,authbridge-unified,authbridge-light,demo-app. The auth-proxyand demo-app source stays in-tree for the standalone quickstart; just
not published by CI.
Backward compatibility
Existing release tags in ghcr.io continue to publish the old image
set unchanged. New tags cut from this branch only publish the 3
images above. Don't tag a new release until kagenti-operator
(Phase B) and the kagenti chart (Phase C) are updated to reference
the new image set — otherwise pulls break.
Test plan
go test -race ./...inauthbridge/authlib— passesgo test -race ./...inauthbridge/cmd/authbridge— passesgo build ./...in all four cmd modules — passespodman buildforDockerfile.proxy— 47.8 MB imagepodman buildforDockerfile.envoy— 165 MB image(operator must reference new image names)
Follow-ups (not in this PR)
Phase B (kagenti-operator):
Images.AuthBridgeLight→ rename + repoint atnew
authbridgeimagemode (it's bundled now)
combinedSidecarfeature gatePhase C (kagenti chart):
values.yamldefaults updatecombinedSidecarHelm flagDocumentation cleanup (separate PR):
authbridge/demos/**/k8s/reference deletedimage names
authbridge/README.md,authbridge/docs/*,cmd/authbridge/README.md,LOCAL_TESTING_GUIDE.md,local-build-and-test.shdefault changes the deployment shape)
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com