docs (k8s): Manage Groups and ACLs with the Redpanda Operator#1639
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for redpanda-docs-preview ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds documentation and testing infrastructure for the Redpanda Operator's Group controller. Changes include a navigation entry pointing to a new documentation page, a new feature test file that validates Group CRD behavior with OIDC-backed group principals, and comprehensive documentation describing how to manage Kafka ACLs through Group custom resources. The documentation covers prerequisites, configuration options, deployment patterns, operational procedures, and best practices for the Group controller. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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
🧹 Nitpick comments (2)
modules/ROOT/nav.adoc (1)
137-137: Consider using empty xref brackets for auto-title rendering.The navigation entry uses hardcoded link text. Based on learnings, prefer
xref:manage:kubernetes/security/authorization/k-group-controller.adoc[]with empty brackets so the target document's title renders automatically. This improves maintainability when page titles change.Note: The sibling entries (lines 135-136) also use hardcoded text and could benefit from the same pattern.
Based on learnings: AsciiDoc linking: prefer using xref links with empty brackets (e.g., xref:section/target.adoc[]) because the title is pulled from the referenced document automatically.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/ROOT/nav.adoc` at line 137, Update the hardcoded xref link text to use AsciiDoc's auto-title rendering by replacing the current xref:manage:kubernetes/security/authorization/k-group-controller.adoc[Manage Groups and ACLs (Operator)] with an empty-bracket form xref:manage:kubernetes/security/authorization/k-group-controller.adoc[]; apply the same change to the sibling entries on lines near the shown xref so the referenced documents supply their titles automatically and reduce maintenance when titles change.modules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc (1)
77-89: Clarify "cluster source" terminology.Line 83 states "The cluster source is immutable" but this section discusses the
spec.clusterfield. Consider using "cluster reference" instead of "cluster source" for clarity, as this matches the field nameclusterRef.📝 Proposed fix
-- *Stable*: Avoid changing group names, as this involves deleting and recreating the Group resource. The cluster source is immutable. +- *Stable*: Avoid changing group names, as this involves deleting and recreating the Group resource. The cluster reference is immutable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc` around lines 77 - 89, Replace the ambiguous phrase "cluster source" with "cluster reference" to match the actual field name used in the Group spec (spec.cluster / clusterRef); update the sentence that reads "The cluster source is immutable" to instead reference the cluster reference (e.g., "The cluster reference is immutable") so the wording aligns with the spec.cluster/clusterRef terminology and avoids confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/manage/examples/kubernetes/group-crds.feature`:
- Around line 39-40: The ACL count assertion is wrong: update the step text
'Then group "engineering" should have 2 ACLs for topic pattern "team-" in
cluster "sasl"' in group-crds.feature to match the manifest; replace it with
'Then group "engineering" should have 2 ACLs in cluster "sasl"' (to count both
topic and subject ACLs) so the step definition that filters by resource type no
longer miscounts.
In
`@modules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc`:
- Around line 20-28: The xref on the prerequisites line uses the wrong path;
open the file k-group-controller.adoc and replace the invalid cross-reference
xref:deploy:deployment-option/self-hosted/kubernetes/k-production-deployment.adoc
with the correct xref target
xref:deploy:redpanda/kubernetes/k-production-deployment.adoc (update the string
in the bullet that references "Redpanda Operator"). Keep the xref syntax
unchanged except for the path segment so the link resolves to the existing
modules/deploy/pages/redpanda/kubernetes/k-production-deployment.adoc file.
---
Nitpick comments:
In
`@modules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc`:
- Around line 77-89: Replace the ambiguous phrase "cluster source" with "cluster
reference" to match the actual field name used in the Group spec (spec.cluster /
clusterRef); update the sentence that reads "The cluster source is immutable" to
instead reference the cluster reference (e.g., "The cluster reference is
immutable") so the wording aligns with the spec.cluster/clusterRef terminology
and avoids confusion.
In `@modules/ROOT/nav.adoc`:
- Line 137: Update the hardcoded xref link text to use AsciiDoc's auto-title
rendering by replacing the current
xref:manage:kubernetes/security/authorization/k-group-controller.adoc[Manage
Groups and ACLs (Operator)] with an empty-bracket form
xref:manage:kubernetes/security/authorization/k-group-controller.adoc[]; apply
the same change to the sibling entries on lines near the shown xref so the
referenced documents supply their titles automatically and reduce maintenance
when titles change.
🪄 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: 1ab09d64-f814-492f-8c3f-f4abb63566aa
📒 Files selected for processing (3)
modules/ROOT/nav.adocmodules/manage/examples/kubernetes/group-crds.featuremodules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc
| And group "engineering" is successfully synced | ||
| Then group "engineering" should have 2 ACLs for topic pattern "team-" in cluster "sasl" |
There was a problem hiding this comment.
Fix the ACL count assertion.
Line 40 asserts "should have 2 ACLs for topic pattern team-" but the manifest defines only 1 topic ACL (lines 25-30) and 1 subject ACL (lines 31-36). If the step definition filters by resource type, this assertion would be incorrect.
Consider one of these fixes:
- Remove "for topic pattern" to count all ACLs:
Then group "engineering" should have 2 ACLs in cluster "sasl" - Change the count to 1:
Then group "engineering" should have 1 ACL for topic pattern "team-" in cluster "sasl" - Add a second assertion for subject ACLs:
And group "engineering" should have 1 ACL for subject pattern "team-" in cluster "sasl"
🧪 Proposed fix: Assert all ACLs
And group "engineering" is successfully synced
- Then group "engineering" should have 2 ACLs for topic pattern "team-" in cluster "sasl"
+ Then group "engineering" should have 2 ACLs in cluster "sasl"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| And group "engineering" is successfully synced | |
| Then group "engineering" should have 2 ACLs for topic pattern "team-" in cluster "sasl" | |
| And group "engineering" is successfully synced | |
| Then group "engineering" should have 2 ACLs in cluster "sasl" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/manage/examples/kubernetes/group-crds.feature` around lines 39 - 40,
The ACL count assertion is wrong: update the step text 'Then group "engineering"
should have 2 ACLs for topic pattern "team-" in cluster "sasl"' in
group-crds.feature to match the manifest; replace it with 'Then group
"engineering" should have 2 ACLs in cluster "sasl"' (to count both topic and
subject ACLs) so the step definition that filters by resource type no longer
miscounts.
modules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc
Show resolved
Hide resolved
|
@JakeSCahill Not sure if we need an enterprise license flash if GBAC is gated by enterprise licenses. |
|
Based on #1584 (comment) we should probably show an enterprise license info banner to help folks understand this is gated by an enterprise license. |
modules/manage/pages/kubernetes/security/authorization/k-group-controller.adoc
Outdated
Show resolved
Hide resolved
Applied critical fixes based on docs team standards review: - Add page metadata (topic type, learning objectives, personas) - Add learning objectives display section - Fix broken xref in Prerequisites (deploy:kubernetes path) - Add link to OIDC authentication documentation - Standardize code block formatting ([,yaml]) - Fix term usage consistency (remove redundant "resources") - Improve sentence flow in opening paragraph These changes ensure the page integrates properly with the documentation architecture and follows Redpanda style standards. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds enterprise license requirement banner to the Group controller documentation as GBAC (Group-Based Access Control) is an enterprise feature that requires OIDC authentication. Addresses PR feedback from david-yu requesting the license banner based on the pattern established in PR redpanda-data#1584. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2 participants
Summary
Groupcustom resource, including creating, updating, and deleting group ACLsinclude::directivesContext
PR redpanda-data/redpanda-operator#1281 adds a new
GroupCRD that allows managing Kafka ACLs for OIDC group principals. This doc page follows the same structure as the existing Manage Roles and ACLs page and covers:New files
modules/manage/pages/kubernetes/security/authorization/k-group-controller.adocmodules/manage/examples/kubernetes/group-crds.featureinclude::directivesModified files
modules/ROOT/nav.adocTest plan
include::directives resolve the tagged example correctly🤖 Generated with Claude Code