Skip to content
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

fix: role binding ref name reference change. #335

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Conversation

Greyeye
Copy link
Collaborator

@Greyeye Greyeye commented Dec 18, 2024

  1. change roleRef.name to use variable

@Greyeye Greyeye changed the title role binding ref name reference change. feat: role binding ref name reference change. Dec 18, 2024
1. change roleRef.name to use variable
@Greyeye Greyeye force-pushed the rolebinding-ref-fix branch from 3af692b to a6fa8b3 Compare December 18, 2024 05:27
Copy link

github-actions bot commented Dec 18, 2024

Temporary image deleted.

@Greyeye Greyeye changed the title feat: role binding ref name reference change. fix: role binding ref name reference change. Dec 18, 2024
@zapier-sre-bot
Copy link
Collaborator

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of charts/kubechecks/Chart.yaml

@@ -1,7 +1,7 @@
 apiVersion: v2
 name: kubechecks
 description: A Helm chart for kubechecks
-version: 0.5.1
+version: 0.5.2
 type: application
 maintainers:
   - name: zapier

Feedback & Suggestions: The version bump from 0.5.1 to 0.5.2 is appropriate if there are backward-compatible bug fixes or minor improvements. Ensure that the changes in the codebase reflect this version update. 📈


😼 Mergecat review of charts/kubechecks/templates/rolebinding.yaml

@@ -5,7 +5,7 @@ metadata:
   namespace: {{ .Values.argocd.namespace }}
 roleRef:
   kind: Role
-  name: kubechecks
+  name: {{ include "kubechecks.fullname" . }}
   apiGroup: rbac.authorization.k8s.io
 subjects:
   - kind: ServiceAccount

Feedback & Suggestions:

  • 🛡️ Security Consideration: Ensure that the {{ include "kubechecks.fullname" . }} template function resolves to the correct role name. If the role name is dynamic, verify that it does not inadvertently escalate privileges by binding to a more privileged role than intended.

  • 🔍 Consistency Check: Double-check that the {{ include "kubechecks.fullname" . }} is consistent with the role name defined elsewhere in your configuration. This helps avoid runtime errors due to mismatched names.

  • 📄 Documentation: Consider documenting the rationale for using {{ include "kubechecks.fullname" . }} for the role name. This can help future maintainers understand the change and its context.



Dependency Review

Click to read mergecats review!

No suggestions found

@Greyeye Greyeye merged commit 1867877 into main Dec 18, 2024
4 checks passed
@Greyeye Greyeye deleted the rolebinding-ref-fix branch December 18, 2024 05:40
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.

3 participants