Skip to content

chore: Update authz overview to clarify RBAC concepts#546

Merged
LakshanSS merged 2 commits intoopenchoreo:mainfrom
binoyPeries:authz-rename
Apr 8, 2026
Merged

chore: Update authz overview to clarify RBAC concepts#546
LakshanSS merged 2 commits intoopenchoreo:mainfrom
binoyPeries:authz-rename

Conversation

@binoyPeries
Copy link
Copy Markdown
Contributor

@binoyPeries binoyPeries commented Apr 7, 2026

Purpose

Summary

"Hierarchical RBAC" collides with the NIST/INCITS 359 term, which means role-to-role inheritance. OpenChoreo roles are flat;
the hierarchy is over resources, not roles. This PR drops the label, restructures the authz overview, and documents the
effect: allow | deny rule that was previously missing from the overview.

Changes

screencapture-localhost-3000-docs-platform-engineer-guide-authorization-overview-2026-04-07-12_56_44 (1)

Applied to both docs/platform-engineer-guide/authorization/overview.md and
versioned_docs/version-v1.0.x/platform-engineer-guide/authorization/overview.md:

  • Removed "Hierarchical" from the title, description, and opening paragraph. Introduced OpenChoreo RBAC as the canonical
    name.
  • Added "What can I do with OpenChoreo RBAC?" — use-case bullets.
  • Added "How OpenChoreo RBAC works" — short intro with the Subject / Role / Scope model.
  • Core Concepts kept unchanged.
  • Added a paragraph to Effective Permissions introducing effect: allow | deny.
  • Added "How OpenChoreo RBAC determines access" — documents the three per-binding match conditions and the
    deny-overrides-allow rule.

Related Issues

Refer discussion: openchoreo/openchoreo#2958 (comment)

Checklist

  • Updated sidebars.ts if adding a new documentation page
  • Run npm run start to preview the changes locally
  • Run npm run build to ensure the build passes without errors
  • Verified all links are working (no broken links)

Signed-off-by: binoyPeries <binoyperies98@gmail.com>
@binoyPeries binoyPeries requested a review from Mirage20 April 7, 2026 07:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Enhanced RBAC documentation with improved explanations of authorization concepts.
    • Added sections clarifying what can be accomplished with OpenChoreo RBAC and how the authorization system evaluates access requests.
    • Expanded details on access control decision semantics for better understanding of permission enforcement.

Walkthrough

Documentation updated to generalize RBAC framing to OpenChoreo, add user-facing permission use-cases, and explain RBAC mechanics including subjects, roles, scopes, and binding evaluation with an effect field (allow default, deny explicit). Access requires ≥1 matching allow and no matching deny.

Changes

Cohort / File(s) Summary
Authorization Documentation
docs/platform-engineer-guide/authorization/overview.md
Rewrote overview to remove Kubernetes-specific wording, added "What can I do with OpenChoreo RBAC?" use-cases, added "How OpenChoreo RBAC works" with definitions (subject, role, scope), introduced effect on role bindings (allow/deny), and specified evaluation semantics (binding match rules and deny-overrides logic).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely describes the main change: updating authorization documentation to clarify RBAC concepts by removing the misleading 'Hierarchical' terminology.
Description check ✅ Passed The PR description includes a comprehensive Purpose section explaining the motivation and changes, references a related discussion, and provides a detailed checklist. All required template sections are present and well-populated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@binoyPeries binoyPeries requested a review from binura-g April 7, 2026 07:24
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/platform-engineer-guide/authorization/overview.md (1)

105-105: Optional: Minor style suggestion.

The static analysis tool suggests removing "of" for conciseness ("all the following" instead of "all of the following"). Both forms are correct; the current phrasing is perfectly acceptable and commonly used in documentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/platform-engineer-guide/authorization/overview.md` at line 105, Edit the
sentence that currently reads "When a request arrives, OpenChoreo evaluates it
against every role binding the subject matches. For each binding, all of the
following must hold for the binding to apply:" and remove the word "of" so it
reads "...all the following must hold for the binding to apply:"; update the
line in the Authorization overview section (the sentence beginning "When a
request arrives...") to use the more concise phrasing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/platform-engineer-guide/authorization/overview.md`:
- Line 105: Edit the sentence that currently reads "When a request arrives,
OpenChoreo evaluates it against every role binding the subject matches. For each
binding, all of the following must hold for the binding to apply:" and remove
the word "of" so it reads "...all the following must hold for the binding to
apply:"; update the line in the Authorization overview section (the sentence
beginning "When a request arrives...") to use the more concise phrasing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: da03d23b-a6e5-473b-a81f-efeb27294454

📥 Commits

Reviewing files that changed from the base of the PR and between caab665 and 701646a.

⛔ Files ignored due to path filters (1)
  • versioned_docs/version-v1.0.x/platform-engineer-guide/authorization/overview.md is excluded by !versioned_docs/**
📒 Files selected for processing (1)
  • docs/platform-engineer-guide/authorization/overview.md

Signed-off-by: binoyPeries <binoyperies98@gmail.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/platform-engineer-guide/authorization/overview.md (1)

101-102: Consider adding a YAML snippet showing the effect field.

The text clearly explains the effect field, but a small YAML example showing effect: deny in context could help users visualize how to use it. For instance:

apiVersion: platform.openchoreo.io/v1alpha1
kind: AuthzRoleBinding
metadata:
  name: deny-secret-project
spec:
  subject: groups:developers
  roleMappings:
    - roleRef:
        name: developer
      effect: deny  # <-- explicit denial
      scope:
        namespace: acme
        project: secret

This is optional since the API reference likely has full examples, but it could improve readability here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/platform-engineer-guide/authorization/overview.md` around lines 101 -
102, Add a short YAML example demonstrating the effect field (showing effect:
deny) near the paragraph that explains it; include an AuthzRoleBinding example
that references roleMappings → roleRef (e.g., name: developer), subject (e.g.,
groups:developers) and a scope (namespace/project) so readers can see effect:
deny in context and understand it revokes access from that role binding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/platform-engineer-guide/authorization/overview.md`:
- Around line 101-102: Add a short YAML example demonstrating the effect field
(showing effect: deny) near the paragraph that explains it; include an
AuthzRoleBinding example that references roleMappings → roleRef (e.g., name:
developer), subject (e.g., groups:developers) and a scope (namespace/project) so
readers can see effect: deny in context and understand it revokes access from
that role binding.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8b017185-efb2-4695-91e6-019b49330fad

📥 Commits

Reviewing files that changed from the base of the PR and between 701646a and 2c85398.

⛔ Files ignored due to path filters (1)
  • versioned_docs/version-v1.0.x/platform-engineer-guide/authorization/overview.md is excluded by !versioned_docs/**
📒 Files selected for processing (1)
  • docs/platform-engineer-guide/authorization/overview.md

@LakshanSS LakshanSS merged commit 11bd0a4 into openchoreo:main Apr 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants