feat: Add fine-grained aggregatable ClusterRoles for RBAC by Melonbun233 · Pull Request #852 · open-component-model/ocm-controller

Melonbun233 · 2026-03-02T23:58:18Z

Description

This PR implements fine-grained aggregatable ClusterRoles to enable flexible Kubernetes RBAC configurations using the standard ClusterRole aggregation pattern.

Fixes open-component-model/ocm#1848

Problem Statement

The current monolithic ClusterRole consolidates all permissions (secrets, configmaps, OCM CRDs, Flux CRDs) into a single role, making it impossible to:

Grant read-only access to OCM/Flux CRDs without also exposing secrets
Leverage Kubernetes ClusterRole aggregation for fine-grained access control
Compose custom permission sets for different user personas (developers, platform teams, etc.)

Solution

This PR refactors the ClusterRole into 8 fine-grained roles that can be selectively aggregated to standard K8s roles (view, edit, admin):

Fine-Grained ClusterRoles

ocm-controller-core-reader - Read configmaps & serviceaccounts
ocm-controller-secrets-reader - Read secrets (isolated for security)
ocm-controller-core-writer - Manage pods, services, deployments
ocm-controller-ocm-reader - Read OCM CRDs (delivery.ocm.software)
ocm-controller-ocm-writer - Full management of OCM CRDs
ocm-controller-flux-reader - Read ALL Flux CRDs (sources + deployments)
ocm-controller-flux-writer - Full management of ALL Flux CRDs
ocm-controller-manager-role - Main role with aggregationRule (aggregates all sub-roles)

Key Features

✅ 100% Backward Compatible - Aggregation disabled by default; existing installations work unchanged
✅ Security Best Practice - Secrets are isolated and NOT aggregated to view role
✅ Configurable - Aggregation behavior fully controllable via values.yaml
✅ Standards Compliant - Follows Kubernetes RBAC aggregation conventions
✅ Progressive Adoption - Users can opt-in when ready
✅ Simplified Structure - One reader + one writer per functional area (Core, OCM, Flux)

Configuration

Example 1: Grant read-only CRD access without secrets (the requested use case)

manager:
  clusterRole:
    aggregation:
      enabled: true
      standardRoles:
        view:
          enabled: true

Result: Users with the view ClusterRole can:

✅ List/get all OCM CRDs (ComponentVersions, Configurations, etc.)
✅ List/get ALL Flux CRDs (GitRepositories, HelmReleases, Kustomizations, etc.)
✅ View configmaps and serviceaccounts
❌ Cannot access secrets (security isolation)

Example 2: Default behavior (backward compatible)

# No changes needed
manager:
  clusterRole:
    aggregation:
      enabled: false  # Default

Result: Uses the existing monolithic ClusterRole. Zero impact on existing deployments.

Example 3: Advanced configuration

manager:
  clusterRole:
    aggregation:
      enabled: true
      standardRoles:
        view:
          enabled: true
        edit:
          enabled: true
      roles:
        # Fine-tune individual role aggregation
        fluxWriter:
          aggregateToEdit: true  # Grant flux management to edit role

Changes Made

1. Updated `deploy/values.yaml`

Added manager.clusterRole.aggregation configuration section
Default: enabled: false for backward compatibility
Configurable standard role aggregation (view, edit)
Individual role controls for advanced customization
Removed non-functional admin.enabled field (admin inherits from edit automatically)
Simplified Flux configuration: Merged fluxWriterSource and fluxWriterDeploy into single fluxWriter

2. Refactored `deploy/templates/role.yaml`

Added conditional rendering based on aggregation.enabled flag
Aggregation mode: Renders 8 fine-grained ClusterRoles with aggregation labels
Legacy mode: Renders the original monolithic ClusterRole (unchanged)
Helper template function to generate aggregation labels dynamically
Fixed YAML separators: Moved inside conditionals to prevent orphaned ---
Removed duplicate permissions: Write roles now only contain write verbs (create, delete, patch, update)
Maintained label format: Used toJson format for backward compatibility (prevents GitOps reconciliation)
Used dig function: Idiomatic Helm pattern for safe key access instead of hasKey
Always aggregate to controller: All fine-grained roles always have ocm.software/aggregate-to-controller: "true" label
Consolidated Flux roles: Merged flux-writer-source and flux-writer-deploy into single flux-writer
Expanded flux-reader: Now includes read access to helmrepositories, ocirepositories, helmreleases, and kustomizations

3. Verified `deploy/templates/role_binding.yaml`

No changes required
ClusterRoleBinding continues to reference ocm-controller-manager-role
Works in both legacy and aggregation modes

Review Feedback Addressed

All feedback from @Skarlso has been addressed:

✅ YAML separators - Moved inside conditional blocks to prevent orphaned ---
✅ Duplicate read permissions - Removed get/list/watch verbs from all writer roles
✅ Label format - Maintained toJson format instead of toYaml for backward compatibility
✅ Non-functional field - Removed admin.enabled from values.yaml
✅ aggregateToController flag - Removed configuration flag; controller aggregation is now always applied
✅ dig function - Replaced and (hasKey ...) with idiomatic dig function for safer key access
✅ Simplified structure - Consolidated Flux roles for cleaner configuration

Testing

Helm templates validated in multiple configurations:

✅ Test 1: Legacy Mode (Default)

helm template ocm-controller ./deploy --namespace ocm-system

Renders single monolithic ClusterRole
Exact same permissions as before
Backward compatible

✅ Test 2: Aggregation Mode (All defaults)

helm template ocm-controller ./deploy --set manager.clusterRole.aggregation.enabled=true

Renders 8 ClusterRoles (7 fine-grained + 1 main)
Main role uses aggregationRule with selector ocm.software/aggregate-to-controller: "true"
Read-only roles have rbac.authorization.k8s.io/aggregate-to-view: "true" label (when enabled)
Secrets-reader does NOT aggregate to view (verified)
Write roles contain only write verbs (no read verbs)

✅ Test 3: Flux Reader Permissions

Has read access to buckets, gitrepositories
NEW: Has read access to helmrepositories, ocirepositories
NEW: Has read access to helmreleases
NEW: Has read access to kustomizations
All with get/list/watch permissions

✅ Test 4: Flux Writer Permissions

Has write access to all Flux source CRDs (helmrepositories, ocirepositories)
Has write access to all Flux deployment CRDs (helmreleases, kustomizations)
All with create/delete/patch/update permissions

✅ Test 5: View Aggregation Control

helm template ocm-controller ./deploy \
  --set manager.clusterRole.aggregation.enabled=true \
  --set manager.clusterRole.aggregation.standardRoles.view.enabled=false

No aggregate-to-view labels applied when disabled
Controller still gets full permissions via controller aggregation

✅ Test 6: Individual Role Control

Roles can be individually disabled
Other roles continue to function correctly
Configuration flexibility maintained

✅ Test 7: Edit Aggregation

Write roles can aggregate to edit when configured
Enables platform team access patterns

Migration Path

Current Release - Aggregation disabled by default (zero impact)
Documentation - Update docs with examples and use cases
User Adoption - Users opt-in by setting aggregation.enabled: true
Future - Consider making aggregation default after community validation

Benefits

Solves the Issue - Enables the exact use case described in #1848
Security - Principle of least privilege through permission separation
Flexibility - Users can compose custom permission sets
Standards - Aligns with Kubernetes RBAC best practices
Safe - No breaking changes; fully backward compatible
Maintainable - Cleaner code following best practices
Simplified - Consistent pattern: one reader + one writer per area

Checklist

Code follows project conventions
Backward compatibility maintained
Helm templates render successfully
Both legacy and aggregation modes validated
Security considerations addressed (secrets isolation)
Configuration documented with examples
All review feedback addressed
DCO sign-off corrected
Flux roles consolidated and tested

Related Documentation

Skarlso · 2026-03-03T07:08:28Z

@Melonbun233 Hello. Please sign the DCO :) Thanks.

Skarlso

There are a few things that needs cleanup and restructuring. Thanks for the PR! :)

Skarlso · 2026-03-03T07:26:01Z

deploy/templates/role.yaml

-  labels: {{ $.Values.manager.clusterRole.labels | toJson }}
+  labels:
+    {{- if .Values.manager.clusterRole.labels }}
+    {{- toYaml .Values.manager.clusterRole.labels | nindent 4 }}
+    {{- end }}


This is a change that will trigger any gitops flow because it changes how the labels are set from json to yaml.

I would recommend keeping the JSON format. We could also try and talk to people about this first, create a minor version, and explicit document that updating will trigger a flurry of updates through gitops flow. Any particular reason for this change?

I reverted back to the Json format.

Skarlso · 2026-03-03T07:26:45Z

deploy/values.yaml

+        coreReader:
+          enabled: true
+          aggregateToView: true
+          aggregateToController: true


You aren't using aggregateToController. You hardcoded adding ocm.software/aggregate-to-controller: "true" everywhere. So someone setting this to false won't actually do anything.

Thanks, I removed the this control because OCM controller need the fine grained roles the same as the legacy behavior.

Skarlso · 2026-03-03T07:29:05Z

deploy/templates/role.yaml

+{{- if .Values.manager.clusterRole.aggregation.enabled }}
+{{- /* AGGREGATION MODE: Create fine-grained ClusterRoles */ -}}
+
+---


If nothing is enabled, these separators will end up as:

--- --- ---

Move them inside the if statements.

Thanks for the comment. I moved all the separators into the if statements.

Skarlso · 2026-03-03T07:29:57Z

deploy/values.yaml

+        admin:
+          # Admin inherits from edit automatically in K8s
+          # This setting has no effect (included for documentation)
+          enabled: false


I don't see any purpose of this even if documented. If something is there, it needs a purpose. If it doesn't have any, just remove it.

Thanks, the admin has been removed.

Skarlso · 2026-03-03T07:33:49Z

deploy/templates/role.yaml

+
+---
+# 5. OCM Writer - Full management of OCM CRDs
+{{- if .Values.manager.clusterRole.aggregation.roles.ocmWriter.enabled }}


These rules are supposed to be additive. All your writing rules including reading permissions. These should ONLY be in the reading rules. Then, you combine reading with writing, where writing provides ONLY writing rules. So move out any duplications from the write rules that have reading permissions so these rules can then be combined which is the whole purpose of aggregation. Same goes for fluxReading and fluxWriting.

Thanks for the comment. I removed the reading rules in the writing roles so there's no duplications.

dee0sap · 2026-03-03T16:54:02Z

deploy/templates/role.yaml

+{{- define "ocm-controller.aggregationLabels" -}}
+{{- $config := .config -}}
+ocm.software/aggregate-to-controller: "true"
+{{- if and (hasKey $config "aggregateToView") $config.aggregateToView }}


Instead of using 'and ( hasKey ... ) for the safely checking aggregateToEdit you should be able to use 'dig'.
e.g.
dig "aggregateToEdit" "" $config
This will give you the value of aggregateToEdit if it is present or empty string if it is not.

Thanks for the review. I changed to the dig function instead.

dee0sap

While this seems a bit overly complex I do think it work.
So, I approve but of course the OCM team is going to have to weigh in :)

This commit addresses all feedback from PR open-component-model#852 review: 1. Fix YAML separator placement - Move separators inside conditional blocks to prevent orphaned `---` - Separators now only appear when roles are actually rendered 2. Remove duplicate read permissions from write roles - Removed get/list/watch verbs from all writer roles - Writers now only contain write verbs (create, delete, patch, update) - Enables proper aggregation: reader + writer = full access - Affected roles: core-writer, ocm-writer, flux-writer-source, flux-writer-deploy 3. Maintain consistent label formatting - Reverted to original `toJson` format instead of `toYaml` - Prevents triggering GitOps reconciliation on label changes - Maintains backward compatibility 4. Remove non-functional admin.enabled field - Kubernetes admin role inherits from edit automatically - Configuration field had no effect and caused confusion 5. Make aggregateToController flag functional - Added conditional check in helper template - Label now respects user configuration instead of being hardcoded - Users can disable controller aggregation per role if needed Testing: - Legacy mode renders correctly (backward compatible) - Aggregation mode renders 9 ClusterRoles properly - No orphaned YAML separators - Write roles contain only write verbs - aggregateToController flag works as expected Related: open-component-model#852 Signed-off-by: Henry Zeng <zeng_zh@foxmail.com>

This commit implements support for Kubernetes ClusterRole aggregation to enable more flexible RBAC configurations. The monolithic ClusterRole has been refactored into 9 fine-grained roles that can be selectively aggregated to standard K8s roles (view, edit, admin). Key features: - Separate ClusterRoles for read vs write permissions - Isolated secrets access (not aggregated to view role by default) - Configurable aggregation via values.yaml - 100% backward compatible (aggregation disabled by default) This enables use cases such as: - Grant read-only access to OCM/Flux CRDs without exposing secrets - Compose custom permission sets for different user personas - Follow Kubernetes RBAC best practices The 9 ClusterRoles created in aggregation mode: 1. ocm-controller-core-reader - Read configmaps & serviceaccounts 2. ocm-controller-secrets-reader - Read secrets (isolated) 3. ocm-controller-core-writer - Manage pods, services, deployments 4. ocm-controller-ocm-reader - Read OCM CRDs 5. ocm-controller-ocm-writer - Manage OCM CRDs 6. ocm-controller-flux-reader - Read Flux source CRDs 7. ocm-controller-flux-writer-source - Manage Flux source repos 8. ocm-controller-flux-writer-deploy - Manage Flux deployments 9. ocm-controller-manager-role - Main aggregating role Configuration example: ```yaml manager: clusterRole: aggregation: enabled: true standardRoles: view: enabled: true ``` Fixes open-component-model/ocm#1848 Signed-off-by: Henry Zeng <zeng_zh@foxmail.com>

This commit addresses all feedback from PR open-component-model#852 review: 1. Fix YAML separator placement - Move separators inside conditional blocks to prevent orphaned `---` - Separators now only appear when roles are actually rendered 2. Remove duplicate read permissions from write roles - Removed get/list/watch verbs from all writer roles - Writers now only contain write verbs (create, delete, patch, update) - Enables proper aggregation: reader + writer = full access - Affected roles: core-writer, ocm-writer, flux-writer-source, flux-writer-deploy 3. Maintain consistent label formatting - Reverted to original `toJson` format instead of `toYaml` - Prevents triggering GitOps reconciliation on label changes - Maintains backward compatibility 4. Remove non-functional admin.enabled field - Kubernetes admin role inherits from edit automatically - Configuration field had no effect and caused confusion 5. Make aggregateToController flag functional - Added conditional check in helper template - Label now respects user configuration instead of being hardcoded - Users can disable controller aggregation per role if needed Testing: - Legacy mode renders correctly (backward compatible) - Aggregation mode renders 9 ClusterRoles properly - No orphaned YAML separators - Write roles contain only write verbs - aggregateToController flag works as expected Related: open-component-model#852 Signed-off-by: Henry Zeng <zeng_zh@foxmail.com>

Replace 'and (hasKey ...)' pattern with idiomatic 'dig' function for checking aggregation flags. This is the Helm best practice for safely accessing potentially missing keys. Benefits: - Cleaner, more readable syntax - Single function call instead of nested conditions - Safer with explicit default values - More idiomatic Helm template code All three aggregation flags now use consistent pattern: - aggregateToController: dig "aggregateToController" false $config - aggregateToView: dig "aggregateToView" false $config - aggregateToEdit: dig "aggregateToEdit" false $config Signed-off-by: Henry Zeng <zeng_zh@foxmail.com>

Remove aggregateToController configuration flag and always apply the controller aggregation label to all fine-grained roles. Rationale: - The controller ALWAYS needs all permissions from fine-grained roles - Making this configurable adds unnecessary complexity - Users should not be able to disable controller permissions - Simplifies configuration by removing 8 redundant flags Changes: - Removed conditional check for aggregateToController in helper template - Label 'ocm.software/aggregate-to-controller: true' is now always added - Removed aggregateToController field from all 8 roles in values.yaml - Controller will always receive full aggregated permissions This ensures the controller functions correctly regardless of user configuration while still allowing users to control view/edit aggregation. Signed-off-by: Henry Zeng <zeng_zh@foxmail.com>

Skarlso · 2026-03-03T19:39:12Z

Thanks for the changes 🙇

Merge the two Flux writer roles (flux-writer-source and flux-writer-deploy) into a single flux-writer role for simplicity and consistency. Expand flux-reader to include read access to all Flux resources: - helmrepositories and ocirepositories (previously write-only) - helmreleases and kustomizations (previously write-only) This provides a cleaner separation: - flux-reader: read access to ALL Flux CRDs - flux-writer: write access to ALL Flux CRDs Changes: - Merged ocm-controller-flux-writer-source and ocm-controller-flux-writer-deploy into single ocm-controller-flux-writer role - Added helmrepositories, ocirepositories, helmreleases, and kustomizations to flux-reader with get/list/watch permissions - Updated role count from 9 to 8 (7 fine-grained + 1 main) - Simplified values.yaml configuration (removed fluxWriterSource and fluxWriterDeploy, added single fluxWriter) Benefits: - Simpler configuration with fewer roles to manage - Consistent pattern: one reader + one writer per functional area - Users can grant read-only Flux access without complexity - Easier to understand and maintain Signed-off-by: Henry Zeng <zeng_zh@foxmail.com>

Melonbun233 requested a review from a team as a code owner March 2, 2026 23:58

Melonbun233 mentioned this pull request Mar 2, 2026

Separate the OCM controller Helm Cluster Roles for K8s Aggregated RBAC open-component-model/ocm#1848

Closed

Melonbun233 closed this Mar 3, 2026

Melonbun233 force-pushed the feat/aggregatable-clusterroles branch from 3c8ea82 to 430eaa2 Compare March 3, 2026 00:02

Melonbun233 reopened this Mar 3, 2026

Skarlso requested changes Mar 3, 2026

View reviewed changes

dee0sap reviewed Mar 3, 2026

View reviewed changes

dee0sap approved these changes Mar 3, 2026

View reviewed changes

Melonbun233 added 4 commits March 3, 2026 11:27

Melonbun233 force-pushed the feat/aggregatable-clusterroles branch from e972689 to 5da7d68 Compare March 3, 2026 19:30

Skarlso approved these changes Mar 4, 2026

View reviewed changes

Skarlso merged commit fbc8e06 into open-component-model:main Mar 4, 2026
9 checks passed

Conversation

Melonbun233 commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem Statement

Solution

Fine-Grained ClusterRoles

Key Features

Configuration

Example 1: Grant read-only CRD access without secrets (the requested use case)

Example 2: Default behavior (backward compatible)

Example 3: Advanced configuration

Changes Made

1. Updated deploy/values.yaml

2. Refactored deploy/templates/role.yaml

3. Verified deploy/templates/role_binding.yaml

Review Feedback Addressed

Testing

✅ Test 1: Legacy Mode (Default)

✅ Test 2: Aggregation Mode (All defaults)

✅ Test 3: Flux Reader Permissions

✅ Test 4: Flux Writer Permissions

✅ Test 5: View Aggregation Control

✅ Test 6: Individual Role Control

✅ Test 7: Edit Aggregation

Migration Path

Benefits

Checklist

Related Documentation

Uh oh!

Skarlso commented Mar 3, 2026

Uh oh!

Skarlso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dee0sap left a comment

Choose a reason for hiding this comment

Uh oh!

Skarlso commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Melonbun233 commented Mar 2, 2026 •

edited

Loading

1. Updated `deploy/values.yaml`

2. Refactored `deploy/templates/role.yaml`

3. Verified `deploy/templates/role_binding.yaml`