docs(weather-agent): fix installer secret namespace in demo-ui#439
docs(weather-agent): fix installer secret namespace in demo-ui#439mrsabath wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR updates the weather-agent demo UI docs to (1) list only installer-provided ConfigMaps for ChangesDocumentation Clarification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/demos/weather-agent/demo-ui.md`:
- Around line 110-115: The troubleshooting step currently instructs users to
check the secret `keycloak-admin-secret` in `team1` which contradicts the
earlier note that Operator 0.2+ stores it in `kagenti-system`; update the
command and any related text that references `team1` so it checks
`kagenti-system` instead (e.g., replace `kubectl get secret
keycloak-admin-secret -n team1` with `kubectl get secret keycloak-admin-secret
-n kagenti-system`) and clarify that `NotFound` in `team1` is expected when
Operator 0.2+ is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d1f781da-473d-4ab6-a1c4-d9b5800fa6a1
📒 Files selected for processing (1)
authbridge/demos/weather-agent/demo-ui.md
Clarify keycloak-admin-secret lives in kagenti-system (not team1) on operator 0.2+, and point UI login to kagenti-test-user in keycloak. Fixes kagenti#1679 Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
mrsabath
force-pushed
the
docs/fix-demo-ui-installer-resources
branch
from
8935a78 to
774c94f
Compare
huang195
left a comment
huang195
left a comment
There was a problem hiding this comment.
Small, well-targeted docs fix — the new "Installer-Provided Resources" block is clearer and more accurate. One blocker before merge plus an out-of-scope nit.
must-fix
authbridge/demos/weather-agent/demo-ui.md:569 — this troubleshooting step still checks team1 for keycloak-admin-secret:
kubectl get secret keycloak-admin-secret -n team1That directly contradicts the new note added on line 110 of this same PR. Mariusz, you fixed it up top then walked right past it down here — must be a Monday 😄. Please flip this to -n kagenti-system so the doc is consistent end-to-end. CodeRabbit flagged this one too.
suggestion (out of scope — feel free to skip)
authbridge/demos/weather-agent/demo-ui.md:562-563 — "the client-registration sidecar couldn't register the client" is stale terminology. Per authbridge/CLAUDE.md, the in-pod client-registration sidecar has been removed and the kagenti-operator's ClientRegistrationReconciler handles registration now. Could read something like "the operator's client registration couldn't reach Keycloak" instead. Happy to defer this to a follow-up.
Areas reviewed: Docs (single markdown file, ±25 lines)
Commits: signed-off ✅
CI status: all green ✅
Address review feedback on kagenti#439: - Troubleshooting step now checks keycloak-admin-secret in kagenti-system, matching the note added earlier in the same doc (operator 0.2+). - Update stale "client-registration sidecar" wording to reference the operator's ClientRegistrationReconciler, which handles client registration in operator 0.2+. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
|
@huang195 both items addressed in a11c09a:
Re Monday — guilty as charged 😄 |
The Step 3 verification claimed 2/2 with a single combined AuthBridge sidecar, which is only true for proxy-sidecar mode without SPIRE. Real installs commonly run with `kagenti.io/spire: enabled` and/or envoy-sidecar mode and see 3/3 (`agent envoy-proxy spiffe-helper`). State the expected output explicitly for the cluster-default mode and note that the count depends on AuthBridge mode + SPIRE setting. Link to the deployment guide for the full mode/label reference. Tracking the broader docs gap in kagenti/kagenti#1775. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/demos/weather-agent/demo-ui.md`:
- Around line 230-245: The expected output in Step 3 currently shows a `2/2`
container count which contradicts Step 2's instruction to keep SPIRE identity
enabled by default; update the Step 3 example to reflect the SPIRE-enabled
default (show `3/3` for the weather-service when spiffe-helper is present) or
explicitly state that the snippet assumes SPIRE was manually disabled in Step 2,
and adjust the header/note text so the example and the note about modes
(`proxy-sidecar`/`envoy-sidecar` and `spiffe-helper`) are consistent with the
Step 2 guidance.
- Around line 232-236: The fenced code block showing the pod table ("NAME READY
STATUS RESTARTS AGE ... weather-tool-7f8c9d6b44-yyyyy ...") lacks a language
tag; update that Markdown fenced block to include a language specifier (e.g.,
change "```" to "```text" or "```bash") so it satisfies markdownlint MD040 and
renders correctly (locate the fenced block containing the NAME/READY/STATUS
lines in demo-ui.md and add the language tag).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b8648fe-3b58-418c-b97c-36db06bc2a25
📒 Files selected for processing (1)
authbridge/demos/weather-agent/demo-ui.md
| Expected output (cluster default — `proxy-sidecar` mode, no SPIRE): | ||
|
|
||
| ``` | ||
| NAME READY STATUS RESTARTS AGE | ||
| weather-service-58768bdb67-xxxxx 2/2 Running 0 2m | ||
| weather-tool-7f8c9d6b44-yyyyy 1/1 Running 0 5m | ||
| ``` | ||
|
|
||
| > **Note:** The agent pod shows **2/2** containers — the agent itself plus | ||
| > one combined AuthBridge sidecar (spiffe-helper is bundled inside; client | ||
| > registration is handled by the operator outside the pod). In envoy-sidecar | ||
| > mode you'll also see a `proxy-init` init container that exits after | ||
| > setting up iptables. | ||
| > **Note:** The container count depends on the AuthBridge mode and whether | ||
| > SPIRE identity is enabled — `2/2` is the cluster-default `proxy-sidecar` | ||
| > mode (`agent` + `authbridge-proxy`). With `envoy-sidecar` mode you'll see | ||
| > `agent` + `envoy-proxy` (plus a `proxy-init` init container), and with | ||
| > `kagenti.io/spire: enabled` on the workload you'll also see a | ||
| > `spiffe-helper` sidecar (so `3/3` is normal in that case). See the | ||
| > [AuthBridge deployment guide](https://github.com/kagenti/kagenti/blob/main/docs/authbridge/deployment-guide.md) | ||
| > for the full mode/label reference. |
There was a problem hiding this comment.
Align Step 3 “expected output” with the Step 2 SPIRE default.
Step 2 instructs users to keep SPIRE identity enabled by default, but this expected output header assumes “no SPIRE” (2/2). That can mislead users into thinking their 3/3 result is wrong. Consider either making the SPIRE-enabled output primary here or explicitly saying this example assumes SPIRE was manually disabled in Step 2.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 232-232: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@authbridge/demos/weather-agent/demo-ui.md` around lines 230 - 245, The
expected output in Step 3 currently shows a `2/2` container count which
contradicts Step 2's instruction to keep SPIRE identity enabled by default;
update the Step 3 example to reflect the SPIRE-enabled default (show `3/3` for
the weather-service when spiffe-helper is present) or explicitly state that the
snippet assumes SPIRE was manually disabled in Step 2, and adjust the
header/note text so the example and the note about modes
(`proxy-sidecar`/`envoy-sidecar` and `spiffe-helper`) are consistent with the
Step 2 guidance.
| ``` | ||
| NAME READY STATUS RESTARTS AGE | ||
| weather-service-58768bdb67-xxxxx 2/2 Running 0 2m | ||
| weather-tool-7f8c9d6b44-yyyyy 1/1 Running 0 5m | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced code block (markdownlint MD040).
Please tag this block as text (or bash if intended) to satisfy linting.
Suggested diff
-```
+```text
NAME READY STATUS RESTARTS AGE
weather-service-58768bdb67-xxxxx 2/2 Running 0 2m
weather-tool-7f8c9d6b44-yyyyy 1/1 Running 0 5m</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 232-232: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @authbridge/demos/weather-agent/demo-ui.md around lines 232 - 236, The fenced
code block showing the pod table ("NAME READY STATUS RESTARTS AGE ...
weather-tool-7f8c9d6b44-yyyyy ...") lacks a language tag; update that Markdown
fenced block to include a language specifier (e.g., change "" to "text" or
"```bash") so it satisfies markdownlint MD040 and renders correctly (locate the
fenced block containing the NAME/READY/STATUS lines in demo-ui.md and add the
language tag).
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
3 participants
Summary
Corrects Installer-Provided Resources in the weather beginner demo:
keycloak-admin-secretis inkagenti-system, notteam1(operator 0.2+)kagenti-test-userinkeycloakkeycloak-admin-secretinteam1Net shorter section (−5 lines).
Issue
Part of kagenti/kagenti#1679
Test plan
kubectl get secret keycloak-admin-secret -n kagenti-systemon Kind installkubectl get secret keycloak-admin-secret -n team1→ NotFound (expected)Summary by CodeRabbit