feat(base-cluster/rbac): adjust rbac stuff for OIDC accounts#1538
Conversation
cwrau
requested review from
marvinWolff,
tasches and
teutonet-bot
as code owners
WalkthroughThe changes introduce a new RBAC email-based configuration file, update Helm templates to distinguish between user emails and service accounts, and centralize subject generation logic for role bindings. Global monitoring and cluster configuration values are removed, and documentation is updated to clarify kubeconfig generation for email-based users versus service accounts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Kubernetes API
User->>Helm Chart: Deploy with RBAC values (including emails)
Helm Chart->>Helm Chart: For each account in rbac.accounts
alt Account contains "@"
Helm Chart-->>Kubernetes API: Create RoleBinding/ClusterRoleBinding with subject kind "User"
Helm Chart--xKubernetes API: Skip ServiceAccount/Secret creation
else Account does not contain "@"
Helm Chart-->>Kubernetes API: Create ServiceAccount and Secret
Helm Chart-->>Kubernetes API: Create RoleBinding/ClusterRoleBinding with subject kind "ServiceAccount"
end
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
Adjust RBAC templates and values to support OIDC (email-based) accounts alongside ServiceAccounts.
- Introduce a new Helm helper (
base-cluster.rbac.subjects) to generatesubjectsentries based on whether an account looks like an email. - Refactor
roleBindings.yamlto use the new helper and remove duplicatedsubjectsblocks. - Update
accounts.yamlandNOTES.txtto conditionally create ServiceAccounts and clarify kubeconfig instructions.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| charts/base-cluster/templates/rbac/roleBindings.yaml | Add base-cluster.rbac.subjects helper; refactor subjects blocks to use it |
| charts/base-cluster/templates/rbac/accounts.yaml | Wrap ServiceAccount creation in a conditional to skip email-based accounts |
| charts/base-cluster/templates/NOTES.txt | Clarify kubeconfig instructions for OIDC users |
| charts/base-cluster/ci/rbac-values.yaml | Remove obsolete keys in CI values |
| charts/base-cluster/ci/rbac-emails-values.yaml | Add sample CI values for email-based accounts |
Comments suppressed due to low confidence (5)
charts/base-cluster/templates/rbac/roleBindings.yaml:31
- Inline inclusion of the YAML list may place the dash on the same line as the colon, resulting in invalid YAML. Consider putting
subjects:on its own line and then including the helper indented so the list starts on the next line.
subjects: {{- include "base-cluster.rbac.subjects" (dict "accounts" $accounts "context" $) | nindent 2 }}
charts/base-cluster/templates/rbac/roleBindings.yaml:45
- Same inline formatting issue here: the list produced by the helper should start on a new indented line. Split
subjects:onto its own line and indent the included list accordingly.
subjects: {{- include "base-cluster.rbac.subjects" (dict "accounts" $clusterMapping "context" $) | nindent 2 }}
charts/base-cluster/templates/rbac/accounts.yaml:3
- Using
{{-and-}}strips newlines, which can merge the---document separator with the previous line. Remove the hyphens or adjust whitespace control so that the---stays on its own line.
{{- if not (contains "@" $name) -}}
charts/base-cluster/templates/rbac/roleBindings.yaml:4
- The new
base-cluster.rbac.subjectshelper contains logic for both ServiceAccounts and OIDC users. Consider adding Helm template/unit tests to cover both branches (email vs non-email) to prevent regressions.
{{- define "base-cluster.rbac.subjects" -}}
charts/base-cluster/templates/NOTES.txt:46
- [nitpick] This sentence is lengthy and may confuse readers. Consider splitting it or rephrasing to clearly separate the instructions for non-email accounts (token-based) versus email accounts (OIDC flow).
Use the following commands to create a kubeconfig for an account if it's not a user with an email, for that you use OIDC as per your hoster;
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
charts/base-cluster/templates/NOTES.txt (2)
46-46: Clarify and punctuate the instruction.The sentence is a bit clunky and missing punctuation. Consider rephrasing for clarity, for example:
- Use the following commands to create a kubeconfig for an account if it's not a user with an email, for that you use OIDC as per your hoster; + Use the following commands to create a kubeconfig for non-email service accounts. For email-based users, authenticate via OIDC according to your hosting provider.
74-74: Capitalize the example header.For consistency with other examples, change to:
- # example usage: + # Example usage:charts/base-cluster/templates/rbac/accounts.yaml (1)
23-24: Normalize Helm whitespace control on closing tags.To mirror the opening tag’s
{{- … -}}trimming, update theendstatements to trim leading whitespace:- {{ end -}} - {{- end -}} + {{- end -}} +{{- end -}}charts/base-cluster/templates/rbac/roleBindings.yaml (1)
4-15: Central subjects helper is well-factored; consider addingapiGroupfor User subjects.By default
Userkind assumes core API group, but explicitapiGroup: rbac.authorization.k8s.iocan improve clarity and future compatibility. For example:{{- if contains "@" $account -}} - {{- $subjects = append $subjects (dict "kind" "User" "name" $account) -}} + {{- $subjects = append $subjects (dict "kind" "User" "name" $account "apiGroup" "rbac.authorization.k8s.io") -}} {{- else -}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
charts/base-cluster/ci/rbac-emails-values.yaml(1 hunks)charts/base-cluster/ci/rbac-values.yaml(0 hunks)charts/base-cluster/templates/NOTES.txt(2 hunks)charts/base-cluster/templates/rbac/accounts.yaml(2 hunks)charts/base-cluster/templates/rbac/roleBindings.yaml(3 hunks)
💤 Files with no reviewable changes (1)
- charts/base-cluster/ci/rbac-values.yaml
🧰 Additional context used
🪛 LanguageTool
charts/base-cluster/templates/NOTES.txt
[typographical] ~46-~46: Consider adding a comma here.
Context: ...count if it's not a user with an email, for that you use OIDC as per your hoster; funct...
(FOR_THAT_COMMA)
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/rbac/accounts.yaml
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
charts/base-cluster/templates/rbac/roleBindings.yaml
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 2
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 4
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 6
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 6
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: wrong indentation: expected 0 but found 2
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: generateDiffCommentBody
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
🔇 Additional comments (6)
charts/base-cluster/templates/rbac/accounts.yaml (1)
3-3: Conditional generation for non-email accounts looks correct.Your use of
if not (contains "@" $name)ensures Secret and ServiceAccount resources are only created for service accounts. Nice and concise.charts/base-cluster/ci/rbac-emails-values.yaml (3)
7-24: Confirm broad wildcard permissions.The
adminrole grants full access ('*'on apiGroups, resources, nonResourceURLs). Ensure this is intentionally as permissive and documented appropriately.
25-32: Edit role definition is clear.The scoped verbs on pods are correct and follow least-privilege principles.
33-50: Non-email account included in email values file.
test2lacks an "@" and will be treated as a ServiceAccount. Since this file is meant for email-based users, please verify iftest2should instead reside in your standard values file or be updated to an email format.charts/base-cluster/templates/rbac/roleBindings.yaml (2)
31-31: Use of centralized helper for namespace bindings.Replacing inline subject blocks with
include "base-cluster.rbac.subjects"greatly reduces duplication.
45-45: Use of centralized helper for cluster bindings.Same pattern applied here enhances maintainability and consistency.
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
🤖 I have created a release *beep* *boop* --- ## [8.2.0](base-cluster-v8.1.0...base-cluster-v8.2.0) (2025-07-21) ### Features * **base-cluster/flux:** add alert about suspended resources ([#1540](#1540)) ([bb1555e](bb1555e)) * **base-cluster/monitoring:** lower cpu request for prometheus ([#1578](#1578)) ([4a83bf6](4a83bf6)) * **base-cluster/monitoring:** non-critical alerts aren't routed to on-call ([#1533](#1533)) ([0d080f4](0d080f4)) * **base-cluster/rbac:** adjust rbac stuff for OIDC accounts ([#1538](#1538)) ([3f9aa69](3f9aa69)) ### Bug Fixes * **base-cluster/docs:** there is no 9.0.0 release for now... ([#1563](#1563)) ([8e417fb](8e417fb)) ### Miscellaneous Chores * **base-cluster/dependencies:** update common docker tag to v1.5.0 ([#1520](#1520)) ([cb4a522](cb4a522)) * **base-cluster/dependencies:** update docker.io/bitnami/kubectl docker tag to v1.33.3-debian-12-r1 ([#1521](#1521)) ([4ed2a77](4ed2a77)) * **base-cluster/dependencies:** update docker.io/curlimages/curl docker tag to v8.13.0 ([#1523](#1523)) ([e451428](e451428)) * **base-cluster/dependencies:** update docker.io/curlimages/curl docker tag to v8.14.1 ([#1544](#1544)) ([02ba163](02ba163)) * **base-cluster/dependencies:** update docker.io/curlimages/curl docker tag to v8.15.0 ([#1585](#1585)) ([a672a67](a672a67)) * **base-cluster/dependencies:** update docker.io/fluxcd/flux-cli docker tag to v2.6.1 ([#1524](#1524)) ([956ad7e](956ad7e)) * **base-cluster/dependencies:** update docker.io/fluxcd/flux-cli docker tag to v2.6.4 ([#1536](#1536)) ([32a69cc](32a69cc)) * **base-cluster/dependencies:** update docker.io/vladgh/gpg docker tag to v1.3.6 ([#1509](#1509)) ([e521e61](e521e61)) * **base-cluster/dependencies:** update external-dns docker tag to v8.8.4 ([#1510](#1510)) ([b8b3f80](b8b3f80)) * **base-cluster/dependencies:** update external-dns docker tag to v8.9.1 ([#1559](#1559)) ([f9c5642](f9c5642)) * **base-cluster/dependencies:** update external-dns docker tag to v8.9.2 ([#1575](#1575)) ([88b2630](88b2630)) * **base-cluster/dependencies:** update grafana-tempo docker tag to v4 ([#1570](#1570)) ([63c2593](63c2593)) * **base-cluster/dependencies:** update grafana-tempo docker tag to v4.0.13 ([#1580](#1580)) ([5b9df00](5b9df00)) * **base-cluster/dependencies:** update helm release alloy to v1 ([#1573](#1573)) ([013a670](013a670)) * **base-cluster/dependencies:** update helm release alloy to v1.2.0 ([#1596](#1596)) ([aec70ba](aec70ba)) * **base-cluster/dependencies:** update helm release descheduler to v0.33.0 ([#1525](#1525)) ([36d8eca](36d8eca)) * **base-cluster/dependencies:** update helm release ingress-nginx to v4.12.3 ([#1511](#1511)) ([3dd2aa7](3dd2aa7)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v75.12.0 ([#1598](#1598)) ([eec214f](eec214f)) * **base-cluster/dependencies:** update helm release kyverno to v3.4.4 ([#1512](#1512)) ([bc20fb9](bc20fb9)) * **base-cluster/dependencies:** update helm release kyverno-policies to v3.4.4 ([#1513](#1513)) ([f79dce1](f79dce1)) * **base-cluster/dependencies:** update helm release loki to v6.30.1 ([#1526](#1526)) ([4bf6daa](4bf6daa)) * **base-cluster/dependencies:** update helm release loki to v6.31.0 ([#1566](#1566)) ([b188c09](b188c09)) * **base-cluster/dependencies:** update helm release loki to v6.32.0 ([#1586](#1586)) ([57c7d86](57c7d86)) * **base-cluster/dependencies:** update helm release reflector to v9 ([#1590](#1590)) ([e679195](e679195)) * **base-cluster/dependencies:** update helm release tetragon to v1.4.1 ([#1581](#1581)) ([ff4f27d](ff4f27d)) * **base-cluster/dependencies:** update helm release traefik to v35.4.0 ([#1527](#1527)) ([9d30e5e](9d30e5e)) * **base-cluster/dependencies:** update helm release traefik to v36 ([#1592](#1592)) ([8c9cafa](8c9cafa)) * **base-cluster/dependencies:** update helm release trivy-operator to v0.29.2 ([#1528](#1528)) ([a815190](a815190)) * **base-cluster/dependencies:** update helm release trivy-operator to v0.29.3 ([#1582](#1582)) ([b078902](b078902)) * **base-cluster/dependencies:** update helm release velero to v7.2.2 ([#1172](#1172)) ([a33e9cb](a33e9cb)) * **base-cluster/dependencies:** update metrics-server docker tag to v7.4.10 ([#1576](#1576)) ([c29757a](c29757a)) * **base-cluster/dependencies:** update metrics-server docker tag to v7.4.6 ([#1514](#1514)) ([c897fbd](c897fbd)) * **base-cluster/dependencies:** update metrics-server docker tag to v7.4.9 ([#1541](#1541)) ([134e3c1](134e3c1)) * **base-cluster/dependencies:** update oauth2-proxy docker tag to v6.2.13 ([#1515](#1515)) ([0c04e1c](0c04e1c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an alert for suspended resources in the flux component. * Reduced CPU requests for Prometheus in the monitoring component. * Updated routing to exclude non-critical alerts from on-call notifications. * Improved RBAC configurations for better OIDC account support. * **Bug Fixes** * Clarified that there is no 9.0.0 release currently. * **Chores** * Updated multiple dependencies and docker image tags. * Bumped chart version to 8.2.0. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit