feat: add policy namespaces as direct subcommand of policy#789
feat: add policy namespaces as direct subcommand of policy#789jakedoublev merged 5 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (10)
📝 WalkthroughWalkthroughReplaced a shared package-level Cobra command with a builder that constructs independent Changes
Sequence Diagram(s)(omitted — changes are refactor and documentation/test updates that do not introduce a new multi-actor runtime flow requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the namespace command structure to allow the command tree to be registered under multiple parent commands, specifically enabling access via both policy namespaces and policy attributes namespaces. It introduces a helper function to create commands from documentation metadata and adds end-to-end tests for the new direct path. The review feedback identifies several instances where subcommands (delete, reactivate, and unsafe update) incorrectly reference the documentation object of the deactivate command for their flag metadata instead of using their own, which could lead to inconsistencies.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/policy/namespaces.go (1)
414-445: Use each command's own doc for itsidflag instead ofdeactivateDoc.The
deleteCmd,reactivateCmd, andunsafeUpdateCmdall referencedeactivateDoc.GetDocFlag("id")instead of their respective doc variables. While the flag definitions may currently be identical, this coupling is fragile—if any command's doc evolves independently, this will silently use the wrong metadata. This pattern is handled correctly inattributes.go, where each command uses its own doc.♻️ Suggested fix
deleteDoc := man.Docs.GetDoc("policy/namespaces/unsafe/delete") deleteCmd := newCommandFromDoc(deleteDoc, unsafeDeleteAttributeNamespace) deleteCmd.Flags().StringP( - deactivateDoc.GetDocFlag("id").Name, - deactivateDoc.GetDocFlag("id").Shorthand, - deactivateDoc.GetDocFlag("id").Default, - deactivateDoc.GetDocFlag("id").Description, + deleteDoc.GetDocFlag("id").Name, + deleteDoc.GetDocFlag("id").Shorthand, + deleteDoc.GetDocFlag("id").Default, + deleteDoc.GetDocFlag("id").Description, ) reactivateDoc := man.Docs.GetDoc("policy/namespaces/unsafe/reactivate") reactivateCmd := newCommandFromDoc(reactivateDoc, unsafeReactivateAttributeNamespace) reactivateCmd.Flags().StringP( - deactivateDoc.GetDocFlag("id").Name, - deactivateDoc.GetDocFlag("id").Shorthand, - deactivateDoc.GetDocFlag("id").Default, - deactivateDoc.GetDocFlag("id").Description, + reactivateDoc.GetDocFlag("id").Name, + reactivateDoc.GetDocFlag("id").Shorthand, + reactivateDoc.GetDocFlag("id").Default, + reactivateDoc.GetDocFlag("id").Description, ) unsafeUpdateDoc := man.Docs.GetDoc("policy/namespaces/unsafe/update") unsafeUpdateCmd := newCommandFromDoc(unsafeUpdateDoc, unsafeUpdateAttributeNamespace) unsafeUpdateCmd.Flags().StringP( - deactivateDoc.GetDocFlag("id").Name, - deactivateDoc.GetDocFlag("id").Shorthand, - deactivateDoc.GetDocFlag("id").Default, - deactivateDoc.GetDocFlag("id").Description, + unsafeUpdateDoc.GetDocFlag("id").Name, + unsafeUpdateDoc.GetDocFlag("id").Shorthand, + unsafeUpdateDoc.GetDocFlag("id").Default, + unsafeUpdateDoc.GetDocFlag("id").Description, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/policy/namespaces.go` around lines 414 - 445, The id flag for deleteCmd, reactivateCmd, and unsafeUpdateCmd is incorrectly sourced from deactivateDoc; update each flag call to use its own doc variable (use deleteDoc.GetDocFlag("id") for deleteCmd, reactivateDoc.GetDocFlag("id") for reactivateCmd, and unsafeUpdateDoc.GetDocFlag("id") for unsafeUpdateCmd) so each command reads the id metadata from its corresponding doc object (leave the name flag for unsafeUpdateCmd as-is using unsafeUpdateDoc).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/policy/namespaces.go`:
- Around line 414-445: The id flag for deleteCmd, reactivateCmd, and
unsafeUpdateCmd is incorrectly sourced from deactivateDoc; update each flag call
to use its own doc variable (use deleteDoc.GetDocFlag("id") for deleteCmd,
reactivateDoc.GetDocFlag("id") for reactivateCmd, and
unsafeUpdateDoc.GetDocFlag("id") for unsafeUpdateCmd) so each command reads the
id metadata from its corresponding doc object (leave the name flag for
unsafeUpdateCmd as-is using unsafeUpdateDoc).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ab5977b-5366-4190-86b8-d5bcac8d0313
📒 Files selected for processing (15)
cmd/policy/namespaces.godocs/man/policy/namespaces/_index.mddocs/man/policy/namespaces/create.mddocs/man/policy/namespaces/deactivate.mddocs/man/policy/namespaces/get.mddocs/man/policy/namespaces/key/_index.mddocs/man/policy/namespaces/key/assign.mddocs/man/policy/namespaces/key/remove.mddocs/man/policy/namespaces/list.mddocs/man/policy/namespaces/unsafe/_index.mddocs/man/policy/namespaces/unsafe/delete.mddocs/man/policy/namespaces/unsafe/reactivate.mddocs/man/policy/namespaces/unsafe/update.mddocs/man/policy/namespaces/update.mde2e/namespaces.bats
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/policy/namespaces.go`:
- Around line 321-338: The builder uses man.Docs.GetDoc(...) directly which
bypasses the man layer's CommandOpts processing (including recomputing Short
from DocSubcommands); change those calls to use man.Docs.GetCommand(...) before
cloning so the computed metadata is applied. In buildNamespacesCommandTree
replace the GetDoc("policy/namespaces") and the other GetDoc call sites (the
ones around lines building the "unsafe" and "key" subcommands) with
GetCommand("policy/namespaces")/GetCommand(...) and then clone or extract the
underlying Doc as needed so the CommandOpts and derived Short descriptions are
preserved when constructing nsCmd and its subcommands.
🪄 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: d51c17a8-00b4-42c5-afbb-71491baf4248
📒 Files selected for processing (1)
cmd/policy/namespaces.go
2 participants
Summary
policy/attributes/namespacestopolicy/namespacespolicy namespacesas a direct subcommand alongside the existingpolicy attributes namespacespathbuildNamespacesCommandTree()builderpolicy namespacespathSummary by CodeRabbit
New Features
policy namespacescommands alongside the existing attribute-based access; both paths behave consistently.Documentation
policy namespacescommand path across namespace-related docs.Tests
policy namespacesoperations (create, get, list, delete) via the direct command path.