Skip to content

chore(makefile): support to generate rbac in make manifests#209

Merged
cheyang merged 2 commits intosgl-project:mainfrom
Syspretor:chore/generate-rbac-in-make-manifest
Mar 16, 2026
Merged

chore(makefile): support to generate rbac in make manifests#209
cheyang merged 2 commits intosgl-project:mainfrom
Syspretor:chore/generate-rbac-in-make-manifest

Conversation

@Syspretor
Copy link
Copy Markdown
Collaborator

Ⅰ. Motivation

Ⅱ. Modifications

Ⅲ. Does this pull request fix one issue?

fixes #XXXX

Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅴ. Describe how to verify it

VI. Special notes for reviews

Checklist

  • Format your code make fmt.
  • Add unit tests or integration tests.
  • Update the documentation related to the change.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the management of Kubernetes RBAC roles for the controller by introducing automated generation. It ensures that the controller's required permissions are always up-to-date with its code annotations and are correctly packaged within the Helm chart for deployment. This change enhances maintainability and reduces the potential for manual errors in RBAC configuration.

Highlights

  • Automated RBAC Generation: The Makefile has been updated to automatically generate Kubernetes RBAC ClusterRole definitions using controller-gen, streamlining the process of defining necessary permissions for the controller.
  • RBAC File Restructuring: The previous static RBAC definition file config/rbac/rbgs_controller_role.yaml has been removed and replaced by a dynamically generated config/rbac/role.yaml, which is then copied into the Helm chart templates.
  • Expanded Controller Permissions: Numerous +kubebuilder:rbac annotations across several controller files have been updated to grant broader permissions (including create, delete, list, watch) for status subresources and to include new resource types such as coordinatedpolicies, instances, instancesets, and various core Kubernetes resources like pods, configmaps, events, and services.
  • Helm Chart Integration: The Helm chart's clusterrole.yaml template has been modified to directly incorporate the newly generated and expanded RBAC rules, ensuring consistency between the controller's code and its deployment manifests.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • Makefile
    • Updated manifests target to include RBAC generation using controller-gen and to copy the generated role.yaml to the Helm chart templates.
  • config/rbac/rbgs_controller_role.yaml
    • Removed the static RBAC ClusterRole definition file.
  • config/rbac/role.yaml
    • Added a new dynamically generated RBAC ClusterRole definition file.
  • deploy/helm/rbgs/templates/clusterrole.yaml
    • Updated the ClusterRole definition to reflect the new, expanded RBAC rules, aligning with the automatically generated role.
  • internal/controller/workloads/rolebasedgroup_controller.go
    • Extended RBAC permissions for rolebasedgroups/status to include create, delete, list, watch verbs.
    • Added RBAC permissions for coordinatedpolicies, clusterengineruntimeprofiles, pods, configmaps, events, services, statefulsets, deployments, customresourcedefinitions, podgroups, leaderworkersets, and leases.
  • internal/controller/workloads/rolebasedgroupscalingadapter_controller.go
    • Extended RBAC permissions for rolebasedgroupscalingadapters/status to include create, delete, list, watch verbs.
  • internal/controller/workloads/rolebasedgroupset_controller.go
    • Extended RBAC permissions for rolebasedgroupsets/status to include create, delete, list, watch verbs.
    • Added RBAC permissions for clusterengineruntimeprofiles and clusterengineruntimeprofiles/status.
  • internal/controller/workloads/roleinstance_controller.go
    • Extended RBAC permissions for roleinstances/status to include create, delete, list, watch verbs.
  • internal/controller/workloads/roleinstanceset_controller.go
    • Extended RBAC permissions for roleinstancesets/status and roleinstances/status to include create, delete, list, watch verbs.
    • Added RBAC permissions for instances, instances/status, instancesets, and instancesets/status.
  • pkg/reconciler/roleinstanceset/statefulmode/stateful_instance_set_controller.go
    • Extended RBAC permissions for instances/status and instancesets/status to include create, delete, list, watch verbs.
    • Updated events resource group from core to "".
Activity
  • The pull request was created by Syspretor.
  • The description body contains template sections for motivation, modifications, issue linking, test cases, verification, and special notes, none of which have been filled out, indicating no specific human-provided context beyond the title.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a great improvement, automating RBAC manifest generation from controller-gen markers. This enhances maintainability and ensures permissions stay in sync with code. My review focuses on the RBAC markers themselves. I've found a recurring issue where status subresources (*/status) are granted excessive permissions like create, delete, list, and watch. These verbs are generally not applicable to status subresources. I've provided suggestions to scope down these permissions to follow the principle of least privilege.

Comment thread internal/controller/workloads/rolebasedgroup_controller.go Outdated
Comment thread internal/controller/workloads/rolebasedgroup_controller.go
Comment thread internal/controller/workloads/rolebasedgroup_controller.go Outdated
Comment thread internal/controller/workloads/rolebasedgroup_controller.go Outdated
Comment thread internal/controller/workloads/rolebasedgroup_controller.go Outdated
Comment thread internal/controller/workloads/roleinstanceset_controller.go Outdated
Comment thread internal/controller/workloads/roleinstanceset_controller.go Outdated
Comment thread internal/controller/workloads/roleinstanceset_controller.go Outdated
Comment thread pkg/reconciler/roleinstanceset/statefulmode/stateful_instance_set_controller.go Outdated
Comment thread pkg/reconciler/roleinstanceset/statefulmode/stateful_instance_set_controller.go Outdated
Copy link
Copy Markdown
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@cheyang cheyang merged commit 23dd17d into sgl-project:main Mar 16, 2026
9 of 10 checks passed
sebest added a commit to sebest/rbg that referenced this pull request Apr 9, 2026
- config/rbac/kustomization.yaml: update rbgs_controller_role.yaml to
  role.yaml (renamed in sgl-project#209 but reference not updated, breaking
  kustomize build)
- config/crd/kustomization.yaml: add 5 missing CRD resources
  (coordinatedpolicies, instances, instancesets, roleinstances,
  roleinstancesets)
- Regenerate deploy/kubectl/manifests.yaml and
  config/manager/kustomization.yaml via make build-installer
sebest added a commit to sebest/rbg that referenced this pull request Apr 12, 2026
- config/rbac/kustomization.yaml: update rbgs_controller_role.yaml to
  role.yaml (renamed in sgl-project#209 but reference not updated, breaking
  kustomize build)
- config/crd/kustomization.yaml: add 5 missing CRD resources
  (coordinatedpolicies, instances, instancesets, roleinstances,
  roleinstancesets)
- Regenerate deploy/kubectl/manifests.yaml and
  config/manager/kustomization.yaml via make build-installer
sebest added a commit to sebest/rbg that referenced this pull request Apr 12, 2026
- config/rbac/kustomization.yaml: update rbgs_controller_role.yaml to
  role.yaml (renamed in sgl-project#209 but reference not updated, breaking
  kustomize build)
- config/crd/kustomization.yaml: add 5 missing CRD resources
  (coordinatedpolicies, instances, instancesets, roleinstances,
  roleinstancesets)
- Regenerate deploy/kubectl/manifests.yaml and
  config/manager/kustomization.yaml via make build-installer
sebest added a commit to sebest/rbg that referenced this pull request Apr 22, 2026
- config/rbac/kustomization.yaml: update rbgs_controller_role.yaml to
  role.yaml (renamed in sgl-project#209 but reference not updated, breaking
  kustomize build)
- config/crd/kustomization.yaml: add the instances and instancesets CRDs
  that were missing from the resources list, and sort entries
  alphabetically
- Regenerate deploy/kubectl/manifests.yaml to reflect the two fixes
  above
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.

2 participants