📖 docs: update Placement debug endpoint documentation - #554
Open
haoqing0110 wants to merge 1 commit into
Open
Conversation
✅ Deploy Preview for open-cluster-management ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: haoqing0110 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughUpdated placement controller debug documentation to use a sidecar PlacementDebugServer, change port-forwarding to 9443, switch debug endpoints to /debug/placements/*, revise RBAC guidance, add a POST-based "simulate placement" flow, and update the example debug response structure. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/en/docs/concepts/content-placement/placement.md`:
- Around line 729-731: The fenced bash code block containing the curl example
includes a leading shell prompt `$` which triggers MD014; edit the fenced block
(the ```bash block that currently contains "$ curl -k
https://127.0.0.1:9443/debug/placements/default/placement1") to remove the `$`
so the block contains just the plain command `curl -k
https://127.0.0.1:9443/debug/placements/default/placement1` and close the fence
as ``` to satisfy markdownlint.
- Around line 675-689: Replace the unsafe use of the unauthenticated principal
`system:anonymous` in the ClusterRoleBinding `debugger` and the RoleBinding
`placement-debugger` with a dedicated ServiceAccount: create a ServiceAccount
(e.g., `debugger-sa`) in the intended namespace, bind the `debugger` ClusterRole
and the `placement-debugger` Role to that ServiceAccount instead of
`system:anonymous`, and add a short note documenting how to delete the
ServiceAccount and its bindings after debugging (e.g., `kubectl delete
rolebinding placement-debugger`, `kubectl delete clusterrolebinding debugger`,
`kubectl delete serviceaccount debugger-sa -n <namespace>`).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e465120-705b-4bac-abc4-b4d86618e08c
📒 Files selected for processing (1)
content/en/docs/concepts/content-placement/placement.md
Comment on lines
+675
to
+689
| kubectl create clusterrolebinding debugger \ | ||
| --clusterrole=debugger \ | ||
| --user=system:anonymous | ||
|
|
||
| # Grant permission to create placements in the target namespace | ||
| # Replace <namespace> with your placement namespace | ||
| kubectl create role placement-debugger \ | ||
| --verb=create,get \ | ||
| --resource=placements.cluster.open-cluster-management.io \ | ||
| -n <namespace> | ||
|
|
||
| kubectl create rolebinding placement-debugger \ | ||
| --role=placement-debugger \ | ||
| --user=system:anonymous \ | ||
| -n <namespace> |
There was a problem hiding this comment.
Avoid binding debug privileges to system:anonymous.
Line 675 and Line 686 grant debug and placement-create permissions to an unauthenticated principal. This is risky for shared or production-like hubs; use a dedicated user/service account and document cleanup after debugging.
Suggested doc patch
-kubectl create clusterrolebinding debugger \
- --clusterrole=debugger \
- --user=system:anonymous
+kubectl create clusterrolebinding debugger \
+ --clusterrole=debugger \
+ --user=<debug-user>
@@
-kubectl create rolebinding placement-debugger \
- --role=placement-debugger \
- --user=system:anonymous \
- -n <namespace>
+kubectl create rolebinding placement-debugger \
+ --role=placement-debugger \
+ --user=<debug-user> \
+ -n <namespace>
+
+# (Optional) remove bindings after debugging
+# kubectl delete clusterrolebinding debugger
+# kubectl delete rolebinding placement-debugger -n <namespace>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/en/docs/concepts/content-placement/placement.md` around lines 675 -
689, Replace the unsafe use of the unauthenticated principal `system:anonymous`
in the ClusterRoleBinding `debugger` and the RoleBinding `placement-debugger`
with a dedicated ServiceAccount: create a ServiceAccount (e.g., `debugger-sa`)
in the intended namespace, bind the `debugger` ClusterRole and the
`placement-debugger` Role to that ServiceAccount instead of `system:anonymous`,
and add a short note documenting how to delete the ServiceAccount and its
bindings after debugging (e.g., `kubectl delete rolebinding placement-debugger`,
`kubectl delete clusterrolebinding debugger`, `kubectl delete serviceaccount
debugger-sa -n <namespace>`).
haoqing0110
force-pushed
the
update-placement-debug-docs
branch
from
April 8, 2026 07:21
7dfd8e6 to
7dd4e92
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
content/en/docs/concepts/content-placement/placement.md (2)
729-729:⚠️ Potential issue | 🟡 MinorRemove shell prompt to satisfy markdownlint MD014.
Line 729 should not include
$in this fencedbashcommand block.Suggested doc fix
- $ curl -k https://127.0.0.1:9443/debug/placements/default/placement1 + curl -k https://127.0.0.1:9443/debug/placements/default/placement1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/concepts/content-placement/placement.md` at line 729, The fenced bash code block contains a shell prompt character ("$") before the curl command which violates markdownlint MD014; edit the placement example (the fenced block that currently contains "$ curl -k https://127.0.0.1:9443/debug/placements/default/placement1") and remove the leading "$" so the block contains only the command "curl -k https://127.0.0.1:9443/debug/placements/default/placement1".
675-689:⚠️ Potential issue | 🟠 MajorAvoid documenting
system:anonymousfor debug RBAC bindings.Line 675 and Line 686 still bind powerful debug permissions to an unauthenticated principal. Please switch to a dedicated user or ServiceAccount and add cleanup commands after debugging.
Suggested doc fix
kubectl create clusterrolebinding debugger \ --clusterrole=debugger \ - --user=system:anonymous + --user=<debug-user> @@ kubectl create rolebinding placement-debugger \ --role=placement-debugger \ - --user=system:anonymous \ + --user=<debug-user> \ -n <namespace> + +# Optional cleanup +# kubectl delete clusterrolebinding debugger +# kubectl delete rolebinding placement-debugger -n <namespace>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/concepts/content-placement/placement.md` around lines 675 - 689, Replace the unauthenticated system:anonymous subject in the debug RBAC examples by creating and using a dedicated ServiceAccount (e.g., create a ServiceAccount in the target namespace and bind it with kubectl create clusterrolebinding/debugger and kubectl create rolebinding/placement-debugger via the --serviceaccount=<namespace>:<name> subject), update the kubectl examples that currently use --user=system:anonymous to use --serviceaccount and the correct namespace, and add cleanup commands to delete the ServiceAccount, Role, RoleBinding, ClusterRoleBinding (e.g., kubectl delete rolebinding placement-debugger, role placement-debugger, clusterrolebinding debugger, and kubectl delete serviceaccount <name> -n <namespace>) after debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@content/en/docs/concepts/content-placement/placement.md`:
- Line 729: The fenced bash code block contains a shell prompt character ("$")
before the curl command which violates markdownlint MD014; edit the placement
example (the fenced block that currently contains "$ curl -k
https://127.0.0.1:9443/debug/placements/default/placement1") and remove the
leading "$" so the block contains only the command "curl -k
https://127.0.0.1:9443/debug/placements/default/placement1".
- Around line 675-689: Replace the unauthenticated system:anonymous subject in
the debug RBAC examples by creating and using a dedicated ServiceAccount (e.g.,
create a ServiceAccount in the target namespace and bind it with kubectl create
clusterrolebinding/debugger and kubectl create rolebinding/placement-debugger
via the --serviceaccount=<namespace>:<name> subject), update the kubectl
examples that currently use --user=system:anonymous to use --serviceaccount and
the correct namespace, and add cleanup commands to delete the ServiceAccount,
Role, RoleBinding, ClusterRoleBinding (e.g., kubectl delete rolebinding
placement-debugger, role placement-debugger, clusterrolebinding debugger, and
kubectl delete serviceaccount <name> -n <namespace>) after debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a76c7fb4-bfc8-4bac-b38e-3007588987a9
📒 Files selected for processing (1)
content/en/docs/concepts/content-placement/placement.md
Update the debug section to reflect changes from ocm PR #1461: - Add PlacementDebugServer feature gate enablement steps - Update port from 8443 to 9443 (debug-server sidecar) - Add Service port-forward option - Fix kubectl create role command syntax - Add POST method for placement simulation - Fix field name: filteredPiplieResults -> filteredPipelineResults - Add new fields: placement, aggregatedScores - Update example output with proper JSON formatting - Add troubleshooting section The debug server now runs as a standalone sidecar container with proper ClusterSet validation and permission checks. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Qing Hao <qhao@redhat.com>
haoqing0110
force-pushed
the
update-placement-debug-docs
branch
from
April 8, 2026 08:03
7dd4e92 to
62109fe
Compare
Member
Author
|
/assign @qiujian16 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
content/en/docs/concepts/content-placement/placement.md (1)
657-663: Improve the section title for clarity.
Export placement 9443 port to local.is grammatically awkward. Consider phrasing it as a port-forward action.Suggested doc wording
-#### Export placement 9443 port to local. +#### Port-forward debug server port 9443 to local🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/concepts/content-placement/placement.md` around lines 657 - 663, The section heading "Export placement 9443 port to local." is awkward; update the heading to a clearer port-forward action such as "Port-forward placement port 9443 to localhost" or "Port-forward the placement service (port 9443) to localhost", and ensure the accompanying command block (kubectl port-forward -n open-cluster-management-hub svc/cluster-manager-placement 9443:9443) remains unchanged and still follows the new heading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@content/en/docs/concepts/content-placement/placement.md`:
- Around line 657-663: The section heading "Export placement 9443 port to
local." is awkward; update the heading to a clearer port-forward action such as
"Port-forward placement port 9443 to localhost" or "Port-forward the placement
service (port 9443) to localhost", and ensure the accompanying command block
(kubectl port-forward -n open-cluster-management-hub
svc/cluster-manager-placement 9443:9443) remains unchanged and still follows the
new heading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7fd423a-7fa5-429b-9886-69d687bfdb00
📒 Files selected for processing (1)
content/en/docs/concepts/content-placement/placement.md
qiujian16
reviewed
Apr 8, 2026
| Access the debug endpoint for a specific placement: | ||
|
|
||
| ```bash | ||
| curl -k https://127.0.0.1:9443/debug/placements/<namespace>/<name> |
Member
There was a problem hiding this comment.
should we mention that it should include credential info, like bearer token?
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
2 participants
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.
Update the debug section to reflect changes from ocm PR #1461:
The debug server now runs as a standalone sidecar container with proper ClusterSet validation and permission checks.
Summary by CodeRabbit