[RHCLOUD-47517] Validate the migrate from RBAC v1 to V2 - #130
Merged
astrozzc merged 8 commits intoAug 25, 2026
Conversation
…e to reference migration pattern references
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a migration pattern reference for RBAC v1-to-v2 authorization. It updates the migration guide with internal links, expanded testing guidance, corrected identifiers, deployment references, and Astro/Starlight syntax. ChangesRBAC v1-to-v2 migration documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: ⚪ Minimal · up to This documentation-focused change has no actionable merge-blocking risk remaining and is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/content/docs/building-with-kessel/reference/migration-pattern-reference.md (1)
395-397: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAlign the caching guidance with the migration guide.
The Node, Python, and Ruby SDK workspace helpers issue the RBAC request directly and do not provide built-in caching. Remove the SDK caching claim, or identify an SDK and version that implements it.
🤖 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 `@src/content/docs/building-with-kessel/reference/migration-pattern-reference.md` around lines 395 - 397, Update the Caching section to remove the claim that Kessel client libraries provide built-in caching, or replace it with a specific SDK and version that demonstrably supports this behavior. Preserve the guidance to cache immutable workspace IDs while accurately reflecting the Node, Python, and Ruby SDK implementations.Source: MCP tools
🤖 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 `@src/content/docs/building-with-kessel/how-to/migrate-from-rbac-v1-to-v2.mdx`:
- Around line 347-349: Remove the remaining ephemeral-environment authentication
and credential runbook content following the “Deploying Kessel in an ephemeral
environment” Aside, including Keycloak credential extraction, token creation,
RBAC and token-validation changes, secret creation, and pod deletion steps. Keep
only the VPN-marked InScope reference in the public page, or move the full
section to internal documentation.
- Around line 34-40: Update the migration decision tree by adding a Pattern 1
branch for workspace-aware low-cardinality or high-access resources, including
host operations, and add coverage for workspace-aware non-host resources. Narrow
the existing Pattern 2 bullet to workspace-aware resources that meet its
high-cardinality, low-access thresholds, using the terminology and links from
the Migration Pattern Reference.
In
`@src/content/docs/building-with-kessel/reference/migration-pattern-reference.md`:
- Around line 26-32: Define how the cardinality thresholds apply at exactly
10,000 results and exactly 80% accessibility across the referenced pattern
criteria. Update the table and corresponding sections to use inclusive,
non-overlapping boundaries or explicitly document a tie-break rule so every case
maps to a pattern.
- Line 38: Add language identifiers to every fenced code block in the migration
pattern reference: use text for ASCII tree blocks, zed or text for the schema
block, and http for the request block, including the blocks around lines 38,
160, 317, and 366.
- Around line 14-20: Update “The Compatibility Constraint” to describe downward
inheritance: a role binding at a workspace or ancestor may authorize checks at
that workspace or its descendants, while a descendant binding cannot authorize
checks at an ancestor. Replace the exact-level-match requirement and adjust the
example to reflect this compatibility rule, keeping the surrounding coordination
guidance intact.
- Around line 362-365: Update the “Looking Up Built-In Workspaces” section to
state that workspace lookup requests use the service’s own OAuth token, include
the x-rh-rbac-org-id header, and must not forward x-rh-identity; alternatively,
link directly to the migration guide section documenting these requirements.
- Around line 349-358: Update the “Why gRPC” section to link StreamedListObjects
to its kessel.inventory.v1beta2 definition in the Buf Schema Registry, and
explicitly state that /api/rbac/v2 calls are RBAC management endpoints rather
than Inventory API endpoints. Preserve the existing gRPC rationale and SDK
links.
- Around line 89-94: Update the “Detail views / writes” guidance in the
migration pattern reference to require CheckForUpdate for reads that expose
credentials or other highly sensitive data, while retaining Check for ordinary
reads and CheckForUpdate for writes.
---
Nitpick comments:
In
`@src/content/docs/building-with-kessel/reference/migration-pattern-reference.md`:
- Around line 395-397: Update the Caching section to remove the claim that
Kessel client libraries provide built-in caching, or replace it with a specific
SDK and version that demonstrably supports this behavior. Preserve the guidance
to cache immutable workspace IDs while accurately reflecting the Node, Python,
and Ruby SDK implementations.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 51baa36a-927c-4a8f-8663-51ebeaf53bc5
📒 Files selected for processing (2)
src/content/docs/building-with-kessel/how-to/migrate-from-rbac-v1-to-v2.mdxsrc/content/docs/building-with-kessel/reference/migration-pattern-reference.md
Comment on lines
+26
to
+32
| | Category | Pattern | When to Use (all conditions satisfied) | Examples | | ||
| |----------|---------|---------------------------------------|----------| | ||
| | Workspace-aware assets | [1. Native](#pattern-1-native-workspace-aware-low-cardinality) | Resource is workspace-aware AND EITHER queries return <10k results OR >80% of results are accessible | Workspaces, Hosts (low-cardinality ops) | | ||
| | Workspace-aware assets | [2. Native workspace-level list](#pattern-2-native-workspace-level-list-workspace-aware-high-cardinality) | Resource is workspace-aware AND queries return >10k results AND <80% of results are accessible | Hosts (high-cardinality ops) | | ||
| | Non-workspace-aware assets | [3. Default Workspace](#pattern-3-default-workspace-non-workspace-aware-assets) | Resource is NOT workspace-aware AND can be conceptualized as an "asset" (customer manages CRUD; could be placed in a workspace) | Repositories, SCAP policies | | ||
| | Org-wide settings | [4. Root Workspace](#pattern-4-root-workspace-org-wide-asset-centric-settings) | Resource is the organization AND the operation is "asset-centric" (could be workspace-level in the future) | Advisor recommendation acknowledgement | | ||
| | Org-wide settings | [5. Organization-level](#pattern-5-organization-level-non-asset-centric-settings) | Resource is the organization AND the operation is NOT asset-centric (would never belong at workspace level) | Authentication policy | |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define the boundary cases for the cardinality thresholds.
Pattern 1 uses <10,000 or >80%. Pattern 2 uses >10,000 and <80%. Exactly 10,000 results or exactly 80% accessible results match neither pattern. Define inclusive boundaries or document a tie-break rule.
Also applies to: 41-44, 66-70, 131-133
🤖 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
`@src/content/docs/building-with-kessel/reference/migration-pattern-reference.md`
around lines 26 - 32, Define how the cardinality thresholds apply at exactly
10,000 results and exactly 80% accessibility across the referenced pattern
criteria. Update the table and corresponding sections to use inclusive,
non-overlapping boundaries or explicitly document a tie-break rule so every case
maps to a pattern.
|
|
||
| ## Decision Tree | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to every fenced code block.
markdownlint-cli2 reports MD040 at Lines 38, 160, 317, and 366. Use text for the ASCII trees, zed or text for the schema block, and http for the request block.
Proposed fix
-```
+```textAlso applies to: 160-160, 317-317, 366-366
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 38-38: 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
`@src/content/docs/building-with-kessel/reference/migration-pattern-reference.md`
at line 38, Add language identifiers to every fenced code block in the migration
pattern reference: use text for ASCII tree blocks, zed or text for the schema
block, and http for the request block, including the blocks around lines 38,
160, 317, and 366.
Source: Linters/SAST tools
…ern-reference.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… in ephemeral with an example of a possible solution
astrozzc
reviewed
Aug 21, 2026
|
|
||
| ### Caching | ||
|
|
||
| The Default and Root Workspace IDs are **immutable** for a given tenant. Cache them aggressively to avoid the latency and reliability cost of an RBAC lookup on every request. This lookup adds extra latency and a dependency on RBAC availability. Kessel client libraries are expected to encapsulate this lookup with built-in caching of these immutable values. |
Contributor
There was a problem hiding this comment.
In the How-to (lines 199–201) says don't cache; which is right?
Contributor
Author
There was a problem hiding this comment.
Updated the document to mention to don't cache! 😅 😅 😅
astrozzc
approved these changes
Aug 24, 2026
astrozzc
left a comment
astrozzc
left a comment
Contributor
There was a problem hiding this comment.
Overall looks good to me
Comment on lines
+369
to
+370
| GET /api/rbac/v2/workspaces/?type=root | ||
| GET /api/rbac/v2/workspaces/?type=default |
Contributor
There was a problem hiding this comment.
Now we add the ?with_ancestry=true to bypass permission check
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.
Related JIRA link: https://redhat.atlassian.net/browse/RHCLOUD-47517
Acceptance Criteria listed in the JIRA ticket:
Summary by CodeRabbit