[diw-zw]

Ⅰ. Motivation

The Helm chart templates under deploy/helm/rbgs/templates/ were previously kept in a single flat directory, mixing manager, RBAC, webhook and CRD-upgrade resources together. As the chart grows, this flat layout makes it harder to locate and maintain resources.

This PR reorganizes the templates into a kubebuilder-style directory layout (manager/, rbac/, webhook/, crd-upgrade/). This is the same convention adopted by other Kubernetes projects in the ecosystem such as LWS (LeaderWorkerSet) and Kueue. Since RBG is a kubebuilder-based project, aligning its deployment/config layout with kubebuilder's own organizational logic keeps the project idiomatic, improves readability, and lowers the onboarding cost for contributors already familiar with kubebuilder conventions.

Ⅱ. Modifications

Restructured deploy/helm/rbgs/templates/ into kubebuilder-style subdirectories (pure moves/renames, no functional change to rendered manifests):

Before	After
manager.yaml	manager/manager.yaml
clusterrole.yaml	rbac/clusterrole.yaml
rolebinding.yaml	rbac/clusterrole_binding.yaml
secret-role.yaml	rbac/cert_role.yaml + rbac/cert_role_binding.yaml
serviceaccount.yaml	rbac/service_account.yaml
validating-webhook-configuration.yaml	webhook/manifests.yaml
webhook-service.yaml	webhook/service.yaml
upgrade/crd-upgrade.yaml	crd-upgrade/crd-upgrade.yaml

Additionally, the combined secret-role.yaml (which bundled both a Role and a RoleBinding) is split into two dedicated files — rbac/secret_role.yaml and rbac/secret_role_binding.yaml — following the kubebuilder convention of one resource per file.

Ⅲ. Does this pull request fix one issue?

NONE

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

No tests needed. This change only relocates and renames Helm template files; the rendered Kubernetes manifests remain functionally identical.

Ⅴ. Describe how to verify it

Render the chart and confirm the output is unchanged from main:

helm template deploy/helm/rbgs > /tmp/after.yaml
git stash && helm template deploy/helm/rbgs > /tmp/before.yaml && git stash pop
diff <(yq -P 'sort_keys(..)' /tmp/before.yaml) <(yq -P 'sort_keys(..)' /tmp/after.yaml)

The rendered resources should be equivalent (a helm lint deploy/helm/rbgs should also pass).

VI. Special notes for reviews

This is purely a directory/file reorganization aligning the Helm chart with kubebuilder's conventions (as seen in LWS and Kueue). No template logic or rendered output changes, except that the secret Role/RoleBinding are now in separate files.

Checklist

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

[gemini-code-assist]

Code Review

This pull request refactors the Helm templates by separating the RoleBinding definition for rbgs-controller-secret-rolebinding from secret_role.yaml into its own dedicated file, secret_role_binding.yaml. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[cheyang]

Thanks for the cleanup. I rendered the chart before/after locally and the output is identical apart from the # Source: path comments — same resource set (kind + name) and same document ordering, so the move itself is safe.

A few naming/scope notes (all non-blocking):

• Layout matches kubebuilder's config/ scaffolding (rbac/ manager/ webhook/), which is a fine convention to align to. Left an inline note on the rbac file names + the coupled Makefile target.
• webhook/manifests.yaml: manifests.yaml is kubebuilder's name for the generated webhook config, but in a hand-maintained chart it's vaguer than the previous validating-webhook-configuration.yaml. I'd keep the descriptive name.
• crd-upgrade/crd-upgrade.yaml: the filename repeats the directory. crd-upgrade/job.yaml (or similar) is enough now that it's in its own folder.
• Dropping the deploy/helm/rbgs/templates/engineruntime line from uninstall is a good catch — that path doesn't exist on main, so it was a kubectl delete -f against a missing directory. Since it's outside the pure-move scope, worth a one-line mention in the PR description.

[Copilot]

Pull request overview

This pull request reorganizes the deploy/helm/rbgs Helm chart templates into a kubebuilder-style directory structure (manager/rbac/webhook/crd-upgrade) to improve maintainability and consistency with kubebuilder-based projects, while keeping rendered manifests functionally equivalent.

Changes:

• Reorganized Helm chart templates into subdirectories (manager/, rbac/, webhook/, crd-upgrade/).
• Split the previously combined secret Role + RoleBinding template into two separate files under templates/rbac/.
• Updated the Makefile to copy the generated RBAC role into the new template path and removed an uninstall deletion of a non-existent Helm template path.

Reviewed changes

Copilot reviewed 3 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
Makefile	Updates RBAC template copy path to templates/rbac/role.yaml and removes uninstall deletion of a non-existent path.
deploy/helm/rbgs/templates/manager/manager.yaml	Manager Deployment template relocated under manager/.
deploy/helm/rbgs/templates/rbac/role.yaml	ClusterRole template relocated under rbac/ (matches updated Makefile generation path).
deploy/helm/rbgs/templates/rbac/role_binding.yaml	ClusterRoleBinding template relocated under rbac/.
deploy/helm/rbgs/templates/rbac/service_account.yaml	ServiceAccount template relocated under rbac/.
deploy/helm/rbgs/templates/rbac/secret_role.yaml	Secret Role template separated from RoleBinding and placed under rbac/.
deploy/helm/rbgs/templates/rbac/secret_role_binding.yaml	New file: Secret RoleBinding split out into its own template under rbac/.
deploy/helm/rbgs/templates/webhook/manifests.yaml	ValidatingWebhookConfiguration template relocated under webhook/.
deploy/helm/rbgs/templates/webhook/service.yaml	Webhook Service template relocated under webhook/.
deploy/helm/rbgs/templates/crd-upgrade/crd-upgrade.yaml	CRD upgrade hook Job/RBAC/SA templates relocated under crd-upgrade/.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cheyang]

/copilot review

[cheyang]

@gemini-code-assist /review

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[cheyang]

/copilot review

[cheyang]

@gemini-code-assist /review

[gemini-code-assist]

Code Review

This pull request restructures the Helm chart templates for the rbgs service by organizing RBAC and webhook resources into dedicated subdirectories and removing an unused deletion step from the Makefile. The review feedback identifies several naming discrepancies between the implementation, the PR description, and Kubebuilder conventions. Specifically, it suggests renaming clusterrole.yaml to role.yaml, and renaming the certificate-related RBAC files (cert_role.yaml and cert_role_binding.yaml) to secret_role.yaml and secret_role_binding.yaml respectively, to maintain consistency.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[cheyang]

Delta since the previous review (1b33e1b) is limited to four 100%-similarity file renames (role/role_binding -> clusterrole/clusterrole_binding, secret_role/secret_role_binding -> cert_role/cert_role_binding) plus the corresponding Makefile path update. Kind/name coupling between each Role/ClusterRole and its Binding is preserved and CI is fully green on 69aded4. The new names are intentional — clusterrole* matches the actual kind: ClusterRole (clearer than kubebuilder's default role.yaml), and cert_role* was requested in the previous review round because this Role is scoped specifically to the webhook cert secret rather than "secrets" in general. Automated bot feedback (Gemini) pushing back on these names is based on the stale PR description; the code direction is correct. Two follow-ups worth doing before merge: refresh the Before/After table in the PR body so it matches the current filenames, and (optionally) align the in-cluster resource name of cert_role with its filename. No blockers.

[cheyang]

Reviewed this as a pure reorganization and confirmed it's behavior-preserving. Rendered the chart on both the merge-base and this head with helm template — identical after stripping the # Source: path comments (12 objects, unchanged). kustomize build config/default is byte-identical too (17 objects), so make manifests output doesn't drift from the config/rbac rename. helm lint passes, the RBAC roleRef/subject references all still resolve, and nothing outside the chart references the old flat paths. LGTM.

Two minor, non-blocking notes:

• The commit "Rename rbac resources' name: role -> clusterrole, secret_role -> cert_role" only renames files — the resource metadata.name values are unchanged (which is exactly why the render stays identical). Might be worth rewording so it isn't read as a resource-name change.
• The controller edit is comment-only (updates a file path in a doc comment) — correct, just noting it's not a behavior change.

[cheyang]

/lgtm
/approve