Skip to content

fix: sweep post-#411 stale references (dead labels, dead images, dead docs)#414

Merged
huang195 merged 9 commits into
rossoctl:mainfrom
huang195:fix/weather-demo-drop-client-registration-inject
May 16, 2026
Merged

fix: sweep post-#411 stale references (dead labels, dead images, dead docs)#414
huang195 merged 9 commits into
rossoctl:mainfrom
huang195:fix/weather-demo-drop-client-registration-inject

Conversation

@huang195

@huang195 huang195 commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Sweep through kagenti-extensions for stale references and architectural drift after #411 (combined-sidecar split) and the related operator-managed registration switch in kagenti-operator#361. Eight commits, three distinct treatments.

Treatment A: original CI-blocker fix (commit 1)

weather-tool-advanced.yaml and weather-service-advanced.yaml carried kagenti.io/client-registration-inject: "true", which after #411 silently disables registration: the operator's ClientRegistrationReconciler honors the label as a "skip me" signal (SkipReason in kagenti-operator/internal/clientreg/names.go:58), and the legacy in-pod sidecar that the label deferred to was removed in #411. Workload registers nowhere, setup_keycloak_weather_advanced.py --wait-tool-client times out forever. This is what was blocking kagenti#1584's Deploy & Test lane.

Treatment B: full repo sweep for other post-#411 stale references (commits 2–6)

  • Commit 2 (a0b6ee7): broad sweep — local-build-and-test.sh Dockerfile paths, authbridge/CLAUDE.md prose, top-level README.md image inventory, authbridge/README.md mode tables, authbridge/demos/webhook/README.md, authbridge/demos/weather-agent/demo-ui.md, LOCAL_TESTING_GUIDE.md, and YAML banners on the soon-to-be-deleted standalone demos.
  • Commit 3 (fa18e33): review nits — set -euo pipefail in local-build-and-test.sh, accessibility tweaks on demos/README.md broken-row markers, "all four feature gates removed" lead-in.
  • Commit 4 (0626dce): refresh weather-agent + webhook for combined sidecar shape — diagrams and pod-verification blocks rewritten for 2/2 containers and the per-mode container name (authbridge-proxy / envoy-proxy).
  • Commit 5 (b65c087): consolidate single-target/ + multi-target/ into a new token-exchange-routes/ configuration reference. Both old demos used the pre-refactor(cmd): split unified authbridge binary into 3 mode-specific binaries #411 multi-sidecar pattern with images that no longer publish. The new doc covers both single- and multi-route patterns from a config-only angle and pairs with the working deployment demos. Deletes ~3,500 LOC; adds ~210 LOC.
  • Commit 6 (c3444c8): rewrite the github-issue demo (1,209 + 1,156 + 83 lines across demo-ui.md, demo-manual.md, demo.md, plus the deployment YAML). Architecture diagrams, all kubectl exec/logs -c <sidecar> blocks (mode-aware via $SIDECAR shell auto-detect), pod-ready counts, "Sidecar Modes" prose all brought current.

Treatment C: drop webhook demo + collapse authproxy/ → proxy-init/ (commits 7–8)

  • Commit 7 (0416172, ~−1,800 LOC): the webhook demo's stated purpose was "the webhook injects sidecars" — but post-refactor(cmd): split unified authbridge binary into 3 mode-specific binaries #411 webhook injection is the only path; every demo uses it implicitly. The demo's USP is gone and its synthetic auth-proxy + auth-target images aren't published anymore. Same coverage exists in weather-agent (advanced) and github-issue (real workloads, still-published images). Drop the whole authbridge/demos/webhook/ subtree. With the demo gone, the auth-proxy + demo-app source it depended on (authbridge/authproxy/main.go, Dockerfile, quickstart/) becomes dead code; cascade-delete. What's left in authbridge/authproxy/ is exactly the iptables init container — init-iptables.sh, Dockerfile.init, a one-target Makefile. The authproxy/ name no longer matches the contents, so git mv to proxy-init/. Trim the Makefile, rewrite the README to focus on the init container's role. Mechanical path-rename sweep across 14 supporting files (CI workflows, dependabot config, pre-commit config, root Makefile, root and subdirectory READMEs, CLAUDE.md files, dependent demo YAML comments). Drop the orphaned go-fmt-authproxy / go-vet-authproxy pre-commit hooks (no Go files in the dir anymore). Drop the dedicated go-ci job in ci.yaml (no module to test). Update authbridge/go.work to remove the deleted ./authproxy member.
  • Commit 8 (55bd738): post-rename review nits — three docs still invoked the renamed make build-images target; updated to build-proxy-init (root) and docker-build-init (per-component). The proxy-init Makefile hardcoded podman; added CONTAINER_RUNTIME ?= $(shell command -v podman …) auto-detect plus an ifeq guard so the podman-specific ctr images tag retag step inside the kind node only runs when we're actually on podman. The previous 2>/dev/null || true swallowed "podman not found" errors on docker-only machines, producing a successful-looking make load-image followed by ImagePullBackOff; the conditional removes that footgun.

End-state directory layout

authbridge/
├── authlib/                    # shared library (Go module)
├── cmd/
│   ├── authbridge-proxy/         # proxy-sidecar binary
│   ├── authbridge-envoy/         # envoy-sidecar binary
│   ├── authbridge-lite/          # lite binary
│   └── abctl/                    # TUI
├── proxy-init/                 # iptables init container (envoy-sidecar mode only)
├── demos/
│   ├── weather-agent/            # beginner + advanced + abctl walkthrough
│   ├── github-issue/             # full token-exchange flow + scope-based access
│   ├── token-exchange-routes/    # routes config reference (single + multi)
│   └── mcp-parser/               # parser plugin reference
└── docs/                       # framework + plugin docs (4 files)

Every directory's name matches what's inside; no dead-image references; no orphaned hooks; no broken cross-references.

Test plan

  • helm template on the kagenti chart still renders cleanly with extensions images pinned at v0.6.0-alpha.1
  • bash -n on every modified shell script
  • python3 -c "import ast; ast.parse(...)" on every modified Python file
  • kubectl apply --dry-run=client on the surviving demo YAMLs
  • make -n on the new proxy-init/Makefile for both CONTAINER_RUNTIME=podman and CONTAINER_RUNTIME=docker
  • Repo-wide grep for authbridge/authproxy, make build-images, client-registration-inject, combinedSidecar, authbridge-unified, authbridge-light, envoy-with-processor returns zero hits in tracked content (matches in deprecation banners only)
  • Cross-repo consistency verified against kagenti-operator (image names, container names, modes, ConfigMap names, plugin names) and kagenti chart (image pins, plugin references) — companion fix for kagenti's CI script + doc links filed as kagenti#1592
  • Re-run kagenti's Deploy & Test once kagenti#1592 lands (covers the cross-repo path-rename break)

Closes / unblocks

The original CI failure on kagenti#1584 (now merged). Companion path-rename fix at kagenti#1592.

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

@huang195
fix(weather-demo): drop dead client-registration-inject label
f864060 
The advanced weather demo's two pod templates carried
`kagenti.io/client-registration-inject: "true"`, which after PR rossoctl#411
silently disables registration:

* The operator's ClientRegistrationReconciler treats this label as a
  signal to skip — `SkipReason` in
  kagenti-operator/internal/clientreg/names.go:58 returns "label is
  true (legacy webhook client-registration sidecar; operator-managed
  registration disabled for this workload)".
* The legacy in-pod kagenti-client-registration sidecar that the
  label was supposed to defer to was removed in rossoctl#411.

Net effect: setting this label today produces a workload with NO
Keycloak client registered at all, and `setup_keycloak_weather_advanced.py
--wait-tool-client` correctly times out waiting for a SPIFFE-shaped
client (`spiffe://localtest.me/ns/<ns>/sa/<sa>`) that nobody will ever
create. This was hiding behind the kagenti repo's MLflow e2e
failures upstream — once those were fixed, the AuthBridge weather
e2e exposed the latent bug.

Drop the label from both pod templates:
* k8s/weather-tool-advanced.yaml
* k8s/weather-service-advanced.yaml

Plus refresh the two doc references that were directing readers to
set the label:
* demos/weather-agent/demo-ui-advanced.md — replace the bullet that
  recommended the label with one explaining operator-managed
  registration is now the only path.
* demos/webhook/README.md — rewrite the "client registration" bullet
  to call out the label as removed and explain why setting it today
  silently breaks registration.

Verified: `kubectl apply --dry-run=client -f` on both YAMLs renders
clean. Repo grep for the label only matches the two doc references
above (both now phrased as "don't set it").

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195 huang195 mentioned this pull request May 15, 2026
Merged
4 tasks
@huang195
chore: sweep remaining post-rossoctl#411 stale references
a0b6ee7 
Follow-up to the dead-label fix in the previous commit. Repo-wide
sweep for other stale references to the pre-rossoctl#411 multi-sidecar
architecture, with two distinct treatments:

Fixed in place (still-supported paths drifted to dead names):

* local-build-and-test.sh — Dockerfile paths
  cmd/authbridge/Dockerfile.proxy and cmd/authbridge/Dockerfile.envoy
  no longer exist. Update to the post-rossoctl#411 cmd/authbridge-{proxy,envoy,
  lite}/Dockerfile paths and add a build step for the third image
  (authbridge-lite). Inventory list at the end of the script
  expanded accordingly.
* authbridge/CLAUDE.md — fix the prose at line 132 that incorrectly
  told future readers the kagenti.io/client-registration-inject:
  "true" label still works (it doesn't — operator's SkipReason
  honors it and the legacy sidecar that the label deferred to is
  gone). Fix the runtime-resources table to reflect operator-managed
  registration and the bundled spiffe-helper.
* README.md (top-level) — image inventory had authbridge-unified +
  separate client-registration / spiffe-helper images. Replace with
  the post-rossoctl#411 set: authbridge / authbridge-envoy / authbridge-lite
  / proxy-init, with a note explaining the bundling and that older
  release tags still publish the legacy shape.
* authbridge/README.md — image / mode tables fully rewritten to the
  post-rossoctl#411 set. Default mode is now proxy-sidecar (was envoy-sidecar
  in the old text).
* authbridge/demos/webhook/README.md — overview prose, prerequisites
  image list, "Enabling Combined Sidecar Mode" section (the feature
  gate is gone), debugging-logs section, image-pull-error
  troubleshooting block. The `combinedSidecar`, `envoyProxy: false`,
  `spiffeHelper: false` per-sidecar feature gates are all retired;
  rewrite the section to document the post-rossoctl#411 mode-resolution
  chain (CR → namespace ConfigMap → annotation → cluster default).
* authbridge/demos/weather-agent/demo-ui.md — "Key differences" table
  flipped: proxy-sidecar is now the default; both rows now name the
  combined images instead of the dead authbridge-light/envoy split.
* authbridge/demos/README.md — index page note about
  authbridge-unified replaced with the post-rossoctl#411 image set; flag the
  Single-Target / Multi-Target rows as currently broken with an
  inline ⚠️ warning paragraph.

Marked broken-needs-migration (no in-place fix without testing):

* authbridge/authproxy/k8s/auth-proxy-deployment.yaml,
  authbridge/demos/{single,multi}-target/k8s/authbridge-deployment.yaml,
  authbridge/demos/{single,multi}-target/k8s/authbridge-deployment-no-spiffe.yaml
  use the pre-rossoctl#411 multi-sidecar pattern (envoy-proxy +
  authbridge-light + client-registration + standalone spiffe-helper
  containers) with images that no longer publish. Rewriting them to
  the combined-sidecar shape is real work that needs each demo
  manually verified — out of scope for a stale-references cleanup.
  Add an ⚠️ broken-after-rossoctl#411 header to each YAML naming the dead
  images so any apply attempt produces a useful explanation
  alongside the inevitable ImagePullBackOff. Mirror the warning at
  the top of demos/{single,multi}-target/demo.md.

* LOCAL_TESTING_GUIDE.md (760 lines, deeply baked into the legacy
  shape) — top-of-file deprecation banner pointing at the working
  demos. The `local-build-and-test.sh` reference in §1 still works
  because we updated the script; the rest needs re-author.

Verified: bash syntax on local-build-and-test.sh; python3 yaml
parse on each touched K8s manifest; repo-wide grep for the dead
strings now matches only inside warning blocks ("no longer", "BROKEN",
"don't set it", etc.).

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195 huang195 changed the title fix(weather-demo): drop dead client-registration-inject label fix: sweep post-#411 stale references (dead labels, dead images, dead docs) May 15, 2026
huang195 added 4 commits May 15, 2026 17:06
@huang195
chore: address PR review nits
fa18e33 
Four small follow-ups from the review:

* local-build-and-test.sh: bump `set -e` to `set -euo pipefail` to
  match the kagenti shell convention (CLAUDE.md). One unguarded
  variable surfaced under `-u` — `KIND_EXPERIMENTAL_PROVIDER` at the
  podman-detect branch — fixed with `${KIND_EXPERIMENTAL_PROVIDER:-}`.
  All other env-var reads were already either `${VAR:-default}` or
  guaranteed-set by upstream defaults. Verified: bash -n + a smoke
  run against a missing KAGENTI_DIR exits at the expected error
  message (no -u trip).

* local-build-and-test.sh: quote the `${image_name}` argument to
  `echo` inside the tar-file path expansion, in case an image name
  ever contains whitespace. Cosmetic given current names but free.

* authbridge/demos/webhook/README.md: lead the feature-gate-removal
  paragraph with a single-line conclusion ("All four feature gates
  ... are removed.") so a skimmer sees the bottom line before the
  explanatory clause.

* authbridge/demos/README.md: strikethrough on the broken
  Single-Target / Multi-Target rows is invisible to assistive tech.
  Add an explicit "(broken — see below)" text suffix on each row so
  screen readers and non-rendering viewers also get the cue.

Skipped from the review:

* PR title rename suggestion. Reviewer agreed current title is
  defensible ("not blocking"); commit 1 is a real bug fix on a
  user-facing CI path, so leading with `fix:` matches the most
  important half. Keeping it.

Verified: bash -n on the script and a runtime invocation through
the early exit; the README markdown changes are pure prose.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
docs(demos): refresh weather-agent + webhook for combined sidecar shape
0626dce 
Surgical updates to working demos so their prose matches what the
operator actually injects post-rossoctl#411 / kagenti-operator#361 (one
combined AuthBridge sidecar, container name depends on resolved mode).

Changes:

* weather-agent/demo-ui.md
  - Architecture diagram: collapse 4-box agent-pod (agent +
    spiffe-helper + client-registration + envoy-proxy) into 2-box
    (agent + AuthBridge sidecar). Note that container name is
    mode-dependent (authbridge-proxy / envoy-proxy), spiffe-helper
    is bundled, and registration is operator-managed out of band.
  - "Verify pod" section: 4/4 → 2/2; expected `kubectl exec` output
    `agent kagenti-client-registration envoy-proxy spiffe-helper`
    becomes either `agent authbridge-proxy` (default) or
    `agent envoy-proxy` (envoy-sidecar).
  - "Check client registration" section: replace the
    `-c kagenti-client-registration` log inspection with a
    Secret-name-based check + a pointer at the kagenti-operator's
    controller-manager logs for the actual reconciler activity.
  - "Agent Pod Not Starting" troubleshooting: drop the
    `kagenti-client-registration` / `spiffe-helper` -c branches;
    add a kagenti-controller-manager fallback for stuck registration.
  - "Proxy-Sidecar Mode" section: rewrite as "Switching modes" —
    cluster default is now proxy-sidecar (was envoy-sidecar in the
    old prose); document the four-layer resolution chain
    (CR → namespace ConfigMap → annotation → cluster default) and
    show the canonical `kubectl patch agentruntime ... authBridgeMode`
    surface. Verification block shows expected container names per
    mode. Switching back drops the override on the AgentRuntime CR.
  - Inbound-troubleshooting reference: generalize "check envoy-proxy
    logs" to mention both authbridge-proxy and envoy-proxy.

* weather-agent/demo-ui-advanced.md
  - Drop two `combinedSidecar` references (the feature gate is gone
    after rossoctl#411); replace `4/4 (or combined sidecar variant)` with
    a clean `2/2`.
  - Hard-coded `-c envoy-proxy` in two log-inspection blocks: the
    advanced demo runs in envoy-sidecar mode by design, so the
    primary commands stay on `-c authbridge-proxy` for
    proxy-sidecar parity, with an inline note that envoy-sidecar
    uses `envoy-proxy`.

* weather-agent/k8s/configmaps-advanced.yaml
  - One stale comment referenced an `envoy-with-processor` build
    that no longer ships. Generalize to "AuthBridge ext_proc /
    forward-proxy" since the same observation applies to both
    modes after rossoctl#411.

* webhook/README.md
  - Architecture diagram redrawn for the combined-sidecar shape:
    one box for the AuthBridge sidecar with explicit per-mode
    container names. Out-of-band note added about
    operator-managed Keycloak client registration and the
    `kagenti-keycloak-client-credentials-<hash>` Secret.

* webhook/k8s/agent-deployment-webhook.yaml +
  webhook/k8s/agent-deployment-webhook-no-spiffe.yaml
  - Top-of-file "containers injected" lists were the old multi-
    sidecar shape (proxy-init + spiffe-helper +
    kagenti-client-registration + envoy-proxy). Rewrite to the
    post-rossoctl#411 shape (one combined sidecar, container name
    depends on mode, spiffe-helper bundled, registration
    operator-managed). The no-spiffe variant explicitly notes
    that the bundled spiffe-helper stays inactive when
    SPIRE_ENABLED=false.

No structural document changes; the demo flows themselves still
work end-to-end. Container-name updates throughout match the
operator's `AuthBridgeProxyContainerName = "authbridge-proxy"` /
`EnvoyProxyContainerName = "envoy-proxy"` constants.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
docs(demos): consolidate single-target + multi-target into routes guide
b65c087 
Replace the two broken-after-rossoctl#411 standalone demos
(`single-target/`, `multi-target/`) with a single configuration-only
reference at `demos/token-exchange-routes/`. Both old demos used the
pre-rossoctl#411 multi-sidecar pattern (envoy-proxy + authbridge-light +
client-registration + standalone spiffe-helper) with images that no
longer publish; rewriting their YAMLs to the combined-sidecar shape
is real per-demo work that needs runtime testing. The route-config
content is the part that's still useful and isn't covered well by
other demos, so extract that.

New `demos/token-exchange-routes/README.md` covers:
- How AuthBridge's outbound `token-exchange` plugin matches the
  request `Host` header against `authproxy-routes` entries and
  performs RFC 8693 exchange.
- ConfigMap shape: per-route `host` / `target_audience` /
  `token_scopes` fields, with `kubectl create configmap` example.
- Single-target example (one route entry, weather-tool style).
- Multi-target example (three routes; the existing
  `routes.yaml` is moved into this directory unchanged as a
  worked example).
- Mixing exchange and passthrough (default policy);
  tightening to `default_policy: exchange` for tool-only
  namespaces.
- Glob pattern semantics — `filepath.Match` quirks around `**`,
  short-name matching, etc.
- Troubleshooting: `Host` mismatches, missing scopes on the
  agent's own Keycloak client, audience errors at the target,
  routes-not-reloaded.
- Cross-references to `weather-agent/demo-ui-advanced.md` and
  `webhook/README.md` for the deployment side.

Removed:
- `demos/single-target/` (entire directory) — covered by webhook +
  weather-agent demos plus the new routes reference; manual
  multi-sidecar deployment YAMLs were the broken bit.
- `demos/multi-target/{demo.md,run-demo-commands.sh,teardown-demo.sh,k8s/}`
  — the script-driven flow tied to the dead `agent` Deployment in
  the `authbridge` namespace; not worth migrating.

Renamed:
- `demos/multi-target/` → `demos/token-exchange-routes/`
  (`git mv` preserves history on `routes.yaml`).

Cross-references updated everywhere:
- `demos/README.md` table: drop the strikethrough rows + warning
  paragraph; add Token-Exchange Routes / MCP Parser / abctl
  Walkthrough as Reference-tier entries; the recommended-path
  step 3 points at the new routes guide.
- `demos/github-issue/{demo.md,demo-ui.md,demo-manual.md}`,
  `demos/weather-agent/demo-ui.md` — "Multi-Target Demo" links
  retargeted to `../token-exchange-routes/README.md` with updated
  link labels.
- `authbridge/README.md` overview link bullet + the demos list at
  the bottom collapsed to one Token-Exchange Routes entry.
- `authbridge/CLAUDE.md` — directory tree + demo descriptions +
  Keycloak setup-script table updated; gotcha rossoctl#6 (hardcoded SPIFFE
  ID in `single-target/setup_keycloak.py`) removed since the file
  is gone, remaining gotchas renumbered.
- `CLAUDE.md` (top-level) — directory listing line + manual-
  deployment learning-path bullet retargeted at the new routes
  guide.
- `demos/webhook/setup_keycloak.py` — comment that pointed at
  `single-target/k8s/authbridge-deployment.yaml` for the
  `envoy-config` ConfigMap retargeted at the kagenti-system
  release namespace.

Existing single/multi-target sweep banners from earlier in this PR
(broken-after-rossoctl#411 YAML headers) are deleted with the YAMLs.

Net diff: -1,500 lines (the two broken demos), +210 lines (the new
consolidated reference + cross-ref updates). Repo-wide grep for
`single-target/` and `multi-target/` now matches only on prose
that uses those words descriptively (e.g. "single-target route
pattern"), not as path components.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
docs(github-issue): rewrite for combined sidecar shape
c3444c8 
The github-issue demo (the recommended-path "intermediate" demo) was
written for the pre-rossoctl#411 multi-sidecar pattern: four containers per
agent pod (`agent` + `spiffe-helper` + `kagenti-client-registration`
+ `envoy-proxy`), all `kubectl exec/logs -c <sidecar>` blocks
targeting names that no longer exist, and a `combinedSidecar`
feature gate that was removed. Anyone following the demo today
hits container-not-found errors at every verification step.

Updates across all four files:

* `demo.md` (index) — architecture component list rewritten:
  two-container pod (agent + AuthBridge sidecar; container name
  depends on resolved mode), spiffe-helper bundled inside, client
  registration operator-managed.

* `k8s/git-issue-agent-deployment.yaml` — top-of-file injection
  comment rewritten to describe the post-rossoctl#411 shape: one combined
  sidecar with mode-dependent container name; proxy-init only in
  envoy-sidecar mode; operator-managed registration via the
  `kagenti-keycloak-client-credentials-<hash>` Secret mounted at
  `/shared/`.

* `demo-manual.md` — full rewrite of the architecture diagram and
  every verification block:
  - Agent pod ASCII diagram: 3+1 boxes collapsed into 1+1, with
    explicit per-mode container names and operator-managed
    registration callout.
  - "Verify injected containers" step: expected output flipped
    from `agent spiffe-helper kagenti-client-registration envoy-proxy`
    to `agent authbridge-proxy` (or `agent envoy-proxy` for
    envoy-sidecar). Pod ready count `4/4` → `2/2`.
  - "Check client registration" step: replaced the `-c
    kagenti-client-registration` log inspection with a
    Secret-name-based mount check + a kagenti-controller-manager
    log filter for the actual reconciler activity.
  - All `kubectl exec/logs -c envoy-proxy` blocks: rewritten to
    auto-detect the sidecar container name via a `$SIDECAR` shell
    variable that greps for `authbridge-proxy|envoy-proxy`.
  - "Agent Pod Not Starting (4/4 containers)" troubleshooting:
    rewritten for 1/2 / 0/2 with the operator log fallback added
    for stuck-on-shared-secret cases.

* `demo-ui.md` — same treatment as demo-manual.md, plus:
  - Architecture diagram parallel update.
  - "Sidecar Modes" section that previously documented two
    branches (legacy `combinedSidecar: false` vs combined
    `combinedSidecar: true`) collapsed into a single description
    of the post-rossoctl#411 cluster default. The link to the
    `combinedSidecar` feature-gate doc removed (the gate is gone).
  - "Agent card not available" troubleshooting block: log
    inspection rewritten to use the `$SIDECAR` auto-detect.

Net effect: every command in the demo now works against either
proxy-sidecar (cluster default) or envoy-sidecar mode without
manual fixup, and every architecture description matches what the
operator actually injects post-rossoctl#411.

Verified: `grep -n` for `kagenti-client-registration` returns zero
hits across all four files; remaining `envoy-proxy` matches are
all in the per-mode comments and shell auto-detect blocks (i.e.,
correctly describing envoy-sidecar mode), not stale references.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
cwiklik
cwiklik approved these changes May 15, 2026
View reviewed changes

@cwiklik cwiklik left a comment

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.

Clean, thorough sweep of post-#411 stale references across 34 files. Deletes the legacy single-target/ and multi-target/ demos that depended on the retired multi-sidecar shape, replaces them with a focused token-exchange-routes/ configuration reference, and updates all surviving demo docs to reflect the combined sidecar architecture. Shell script improvements (set -euo pipefail, proper quoting, updated Dockerfile paths, authbridge-lite build) are correct. No broken links — mcp-parser/README.md and demo-with-abctl.md both exist upstream. All 6 commits signed-off, CI passing.

One suggestion below — otherwise LGTM.

Comment thread authbridge/authproxy/k8s/auth-proxy-deployment.yaml Outdated
# (authbridge / authbridge-envoy / authbridge-lite). Applying this
# YAML today will produce ImagePullBackOff. The standalone authproxy
# quickstart needs migration to the combined sidecar shape; use the
# webhook or weather-agent demos in the meantime.

@cwiklik cwiklik May 15, 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: The BROKEN warning comment is correct and helpful, but this YAML still references localhost/authbridge-unified:latest which will cause ImagePullBackOff on apply. Consider deleting the file entirely (this standalone authproxy quickstart isn't used by any demo path now) or renaming to .yaml.broken to prevent accidental kubectl apply -f k8s/.

@huang195
refactor: drop webhook demo + collapse authproxy → proxy-init
0416172 
Combined cleanup that eliminates the last remnants of the pre-rossoctl#411
multi-sidecar architecture from the repo. Three coordinated moves:

1. Delete the webhook demo

The webhook demo was a synthetic-workload demonstration of "the
webhook injects sidecars" — but post-rossoctl#411, webhook injection is the
only path; every demo uses it implicitly. Its USP is gone, and its
images (auth-proxy:latest, demo-app:latest) were dropped from CI long
ago — anyone running it today hits ImagePullBackOff unless they
rebuild locally. The same coverage (inbound validation + outbound
token exchange) is provided by the weather-agent advanced and
github-issue demos, which use real workloads. Drop:

- authbridge/demos/webhook/ (entire dir): README.md, k8s/, setup_keycloak.py

2. Cascade the auth-proxy + demo-app source

With the webhook demo gone, nothing else references the auth-proxy
or demo-app images, so the source can go too:

- authbridge/authproxy/main.go + Dockerfile (auth-proxy app source)
- authbridge/authproxy/k8s/auth-proxy-deployment.yaml (already broken
  standalone deployment)
- authbridge/authproxy/quickstart/ (entire subtree — README.md,
  setup_keycloak.py, requirements.txt, k8s/demo-app-deployment.yaml,
  demo-app/main.go + Dockerfile)
- authbridge/authproxy/go.mod + go.sum (no Go code remains)
- authbridge/authproxy/Makefile build/deploy/test targets that
  reference any of the above

3. Rename authproxy/ → proxy-init/

What's left in authbridge/authproxy/ after the cascade is exactly
the iptables init container and nothing else (init-iptables.sh +
Dockerfile.init + a trimmed Makefile + a rewritten README focused
on just the init container's role). The directory name authproxy
no longer matches what's inside, and the pre-commit hooks
go-fmt-authproxy / go-vet-authproxy were already orphaned with no
.go files to scan. Rename to proxy-init/ — matches the image name
and the container name the operator injects.

Cross-reference sweep across 14 supporting files:

* `local-build-and-test.sh` — path updated.
* `Makefile` (root) — fmt now iterates over the live Go modules;
  build-images target replaced with build-proxy-init.
* `.pre-commit-config.yaml` — drop the orphaned
  go-fmt-authproxy / go-vet-authproxy hooks.
* `.github/workflows/ci.yaml` — drop the dedicated `go-ci` job
  (no Go module to test); update the pre-commit job's
  go-version-file to reference authlib/go.mod; rewrite the
  GOWORK="off" comment that named the deleted authproxy module.
* `.github/workflows/build.yaml` — proxy-init context path bumped.
* `.github/workflows/security-scans.yaml` — drop the
  authbridge/proxy-init/quickstart skip-dir entry (path is gone).
* `.github/dependabot.yml` — rewrite. Drop entries for the deleted
  authproxy/quickstart/demo-app docker image, the now-Go-less
  authproxy go.mod, the never-existed authproxy/client-registration
  pip + docker entries. Add the four authlib + cmd/* go.mod
  entries that were missing entirely.
* `authbridge/go.work` — drop ./authproxy from the workspace.
* `authbridge/CLAUDE.md` — directory tree, demo descriptions,
  Keycloak setup-script table, build/deploy section, and the
  envoy-config + required-ConfigMaps sections all rewritten to
  reference the kagenti Helm chart's templates instead of the
  deleted demos/webhook/k8s/configmaps-webhook.yaml. Gotcha rossoctl#5
  (virtualenv in deleted quickstart) removed; remaining gotchas
  renumbered.
* `CLAUDE.md` (root) — directory tree updated; ci.yaml description
  no longer mentions authproxy as a separate Go target;
  manual-deployment learning-path bullet retargeted at the
  routes-config doc.
* `authbridge/README.md` — the "AuthProxy" link bullets that
  described an Envoy + ext_proc binary (which doesn't exist
  anymore — that's authbridge-envoy now) replaced with bullets
  describing each cmd/* binary plus proxy-init.
* `README.md` (root) — same overview rewrite; broken
  ./authbridge/client-registration/ link removed.
* `LOCAL_TESTING_GUIDE.md` — Webhook Demo bullet retargeted at the
  token-exchange-routes guide; deprecation banner alternatives
  list updated to point at github-issue instead of the deleted
  webhook demo.
* `authbridge/demos/README.md` — Webhook row + description block
  removed from the demos table.
* `authbridge/demos/token-exchange-routes/README.md` — two
  broken `../webhook/` links retargeted at github-issue.
* `authbridge/demos/weather-agent/k8s/weather-tool-advanced.yaml`
  — the comment line that pointed at the deleted webhook README
  for the type-label rule retargeted at the operator's webhook
  source.

Net diff: 23 files modified, 14 deleted, 1 dir renamed (1 file moved
via git mv). About -1,800 LOC of unmaintained, broken-image-dependent
content; +120 LOC of corrected prose and the trimmed proxy-init
Makefile/README. End-state directory layout: each subdirectory under
authbridge/ has a name that matches what's inside, no orphaned hooks,
no broken cross-references, no dead-image references.

Verified: `bash -n local-build-and-test.sh`; `kubectl apply
--dry-run=client` on the surviving demo YAMLs; `helm template` on
the kagenti chart still renders (confirms our path changes don't
break anyone consuming the chart); repo-wide grep for
`authbridge/authproxy` returns zero hits in tracked content;
broken-link grep on demos/webhook/ matches only inside the
LOCAL_TESTING_GUIDE.md deprecation banner that was already flagged
as stale earlier in this PR.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
huang195 force-pushed the fix/weather-demo-drop-client-registration-inject branch from 302623a to 0416172 Compare May 16, 2026 00:09
@huang195 huang195 mentioned this pull request May 16, 2026
Merged
3 tasks
huang195 added 2 commits May 15, 2026 20:20
@huang195
chore: address PR rossoctl#414 review nits
55bd738 
Three suggestions from the reviewer, all applied:

* `make build-images` rename — root Makefile's `build-images`
  target was renamed to `build-proxy-init` in commit 0416172 but
  four docs (README.md, CONTRIBUTING.md, CLAUDE.md, authbridge/
  CLAUDE.md) still invoked the old name. Update each:
  - README.md: bare `make build-images` → `make build-proxy-init`
    (the root target now builds only proxy-init, not "all images";
    naming matches reality).
  - CONTRIBUTING.md, CLAUDE.md: `cd authbridge/proxy-init && make
    build-images` → `make docker-build-init` (the proxy-init/
    Makefile only has `docker-build-init` and `load-image` targets;
    there is no `build-images` target there).
  - authbridge/CLAUDE.md: the rebuild test snippet pointed at
    `make build-images && make load-images`; replace with the
    `cd authbridge && podman build -f cmd/...` recipe that
    actually rebuilds the affected combined image — the touched
    code under `authlib/exchange/` ends up in every cmd/* binary,
    not just proxy-init. Also add a top-level "Building Everything
    Locally" section that points at the repo-root
    local-build-and-test.sh as the orchestrated path, with
    per-image podman commands as the manual fallback.

* proxy-init/Makefile container-runtime hardcoded to podman —
  local-build-and-test.sh in this same PR auto-detects docker vs
  podman; the Makefile didn't. Add
    CONTAINER_RUNTIME ?= $(shell command -v podman >/dev/null 2>&1 \
                                 && echo podman || echo docker)
  so docker users get docker by default. The
    $(CONTAINER_RUNTIME) build -f Dockerfile.init …
  line then works for either. Override with
    make CONTAINER_RUNTIME=docker docker-build-init
  on systems with both installed.

* proxy-init/Makefile silent-error swallow on the podman ctr retag
  — the `2>/dev/null || true` masked "podman not found" on
  docker-only systems, producing a successful-looking `make
  load-image` followed by ImagePullBackOff at runtime. Replace
  the unconditional retag with an `ifeq ($(CONTAINER_RUNTIME),
  podman) … endif` block that only runs when we know we're using
  podman, and drop the `2>/dev/null || true` so a real failure
  surfaces. Docker doesn't need the retag (kind resolves docker
  images by their full name natively), so this is a clean
  per-runtime split.

Verified: `make -n` renders the right commands for both
CONTAINER_RUNTIME=podman (default on machines with podman) and
CONTAINER_RUNTIME=docker (explicit override). Repo-wide grep for
`make build-images` returns zero hits after the doc updates.

The PR description point from the same review is addressed
separately by editing the PR body — no code change needed.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
docs: address PR rossoctl#414 review nits — finish the authbridge/REA…
334042d 
…DME sweep

Four small followups missed in the earlier sweep. All in scope of
the post-rossoctl#411 cleanup; they survived because previous edits to the
same files only touched the deployment-modes table + demos list,
not the prose / diagrams / component-doc list.

* `authbridge/README.md` line 3 — opening paragraph linked to two
  deleted/renamed paths: `[client registration](./client-registration/)`
  (no such directory in the repo) and `[token exchange](./authproxy/)`
  (renamed to `proxy-init/` in commit 0416172, and `proxy-init/`
  is just iptables — not a "token exchange" component anyway).
  Rewrite the paragraph to point at `authlib/` + `cmd/` for the
  actual code, with an explicit note that registration is now
  operator-managed.

* `authbridge/README.md` ASCII architecture diagram — the box
  labeled `client-registration (registers Workload with Keycloak)`
  is gone (operator-managed), so drop that box. The "AuthProxy
  Sidecar" header was rewritten to "AuthBridge Sidecar (combined
  image)" with the per-mode container-name disambiguation
  (`authbridge-proxy` / `envoy-proxy`) we already established
  elsewhere. Spiffe-helper is also no longer a separate box —
  bundled inside the combined image, gated by `SPIRE_ENABLED`,
  noted inline. Add an out-of-band footer naming the operator's
  ClientRegistrationReconciler + the
  `kagenti-keycloak-client-credentials-<hash>` Secret it produces.

* `authbridge/README.md` Mermaid architecture diagram — same
  treatment: collapse SpiffeHelper + ClientReg + Sidecar (which
  was three sub-nodes — auth-proxy + envoy-proxy + ext-proc) into
  one Sidecar node with the per-mode name. Move ClientRegistration
  into a new `Operator (kagenti-system)` subgraph so the diagram
  shows it living outside the workload pod. Renumber the
  request-flow arrows from 1-11 to 1-9 reflecting the simplified
  shape.

* `authbridge/README.md` Workload Pod table — drop the
  `client-registration` row, and split the single `AuthProxy
  Sidecar` row into two per-mode rows (`authbridge-proxy` and
  `envoy-proxy`) with a `Mode` column. Add a brief lead-in
  paragraph explaining the post-rossoctl#411 shape.

* `authbridge/README.md` Component Documentation links — both
  `[AuthProxy](authproxy/README.md)` and
  `[Client Registration](client-registration/README.md)` 404 now.
  Replace with one bullet per mode-specific cmd/* binary plus
  proxy-init plus authlib plus a pointer at the `docs/` directory
  that holds the framework / plugin author references. Add a
  trailing note that registration is operator-managed (the
  `Client Registration` link's purpose).

* `CLAUDE.md` lines 265-266 — the "Routes config reference" line
  appeared twice (once bare, once with the parenthetical). Drop
  the bare duplicate; keep the more informative one.

Verified: every link in authbridge/README.md resolves; repo-wide
grep confirms the only remaining `client-registration` mentions
in this file are descriptive prose about the post-rossoctl#411 migration
("standalone client-registration / spiffe-helper sidecars are
gone"), not paths or component references.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
huang195 merged commit e3254d4 into rossoctl:main May 16, 2026
18 checks passed
@huang195
huang195 deleted the fix/weather-demo-drop-client-registration-inject branch May 16, 2026 00:39
@clawgenti clawgenti mentioned this pull request May 18, 2026
Closed
@mrsabath mrsabath mentioned this pull request Jun 2, 2026
Closed
9 tasks
@huang195 huang195 mentioned this pull request Jun 24, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cwiklik cwiklik cwiklik approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Kagenti Issue Prioritization
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@huang195 @cwiklik @rubambiza