-
Notifications
You must be signed in to change notification settings - Fork 66
feat: Refactoring Kubernetes recommended labels #192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update Helm chart templates for OpenFGA by enhancing label management. Two standard Kubernetes labels are added to the common labels template. Label inclusion in the deployment template is consolidated, and labels are now added to Role and RoleBinding resources in the RBAC template. No logic or public interface changes occurred. Changes
Sequence Diagram(s)sequenceDiagram
participant HelmChartUser
participant HelmTemplates
participant KubernetesAPI
HelmChartUser->>HelmTemplates: Install/Upgrade OpenFGA chart
HelmTemplates->>HelmTemplates: Generate resources with openfga.labels (includes new standard labels)
HelmTemplates->>KubernetesAPI: Deploy resources (Deployment, Role, RoleBinding) with consolidated labels
KubernetesAPI-->>HelmChartUser: Resources created with updated labels
✨ Finishing touches🧪 Generate 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/openfga/templates/_helpers.tpl (2)
50-50: Label addition:app.kubernetes.io/component
Added the standard component label with a fixed value. Consider quoting static strings (e.g."authorization-controller") for YAML consistency and to avoid parsing edge cases.
55-55: Label addition:app.kubernetes.io/part-of
Introduced the part-of label to tie resources back to the OpenFGA chart. Ensure this new label is documented in your chart’s README so users are aware of the automatically applied metadata.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/openfga/templates/_helpers.tpl(1 hunks)charts/openfga/templates/deployment.yaml(1 hunks)charts/openfga/templates/rbac.yaml(2 hunks)
🔇 Additional comments (3)
charts/openfga/templates/rbac.yaml (2)
6-7: Apply consolidated labels to Role resource
Theopenfga.labelshelper is correctly injected undermetadata.labelswith proper indentation (nindent 4). This ensures all standard and custom common labels propagate to Roles.
21-22: Apply consolidated labels to RoleBinding resource
Consistently applyingopenfga.labelsto RoleBindings aligns with the rest of the chart. Indentation and scope look correct.charts/openfga/templates/deployment.yaml (1)
28-28: Consolidate pod labels usingopenfga.labels
Replacing separate selector/common labels with a singleopenfga.labelsinclusion simplifies maintenance. Thenindent 8is correct andpodExtraLabelsstill applies as expected.
Description
Setup Kubernetes recommended labels for all resources
References
Review Checklist
mainSummary by CodeRabbit