-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add service account support #79
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds support for using a custom ServiceAccount for Horizon stream pods. Key changes include adding ServiceAccount configuration options in values.yaml, updating multiple templates (upgrade, serviceaccount, deployment, backup, rbac) to reference the new ServiceAccount, and introducing a helper template to dynamically derive the ServiceAccount name.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
values.yaml | Added ServiceAccount configuration section with documentation for new parameters |
templates/upgrade.yml | Injected ServiceAccount name and token autmount settings into the upgrade job spec |
templates/serviceaccount.yml | Created a new resource template for generating a ServiceAccount based on configuration |
templates/rbac.yml | Updated role binding subjects and conditionally applied changes using the new ServiceAccount flag |
templates/deployment.yml | Added ServiceAccount configuration to the pod spec |
templates/backup.yml | Added ServiceAccount configuration to the backup pod spec |
templates/_helpers.tpl | Added helper template ("stream.serviceAccountName") to determine the proper ServiceAccount name |
Comments suppressed due to low confidence (1)
templates/rbac.yml:1
- Please verify that restricting the role binding only when both serviceAccount.create and rbac.create are true is the intended behavior, as it will skip binding if serviceAccount.create is false and rely on the default ServiceAccount.
{{ if and .Values.serviceAccount.create .Values.rbac.create }}
{{- default (include "common.names.fullname" .) (print .Values.serviceAccount.name) -}} | ||
{{- else -}} | ||
{{- default "default" (print .Values.serviceAccount.name) -}} |
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.
[nitpick] Using print() to coerce .Values.serviceAccount.name may be redundant if it is already a string; consider removing print() for clarity.
{{- default (include "common.names.fullname" .) (print .Values.serviceAccount.name) -}} | |
{{- else -}} | |
{{- default "default" (print .Values.serviceAccount.name) -}} | |
{{- default (include "common.names.fullname" .) .Values.serviceAccount.name -}} | |
{{- else -}} | |
{{- default "default" .Values.serviceAccount.name -}} |
Copilot uses AI. Check for mistakes.
No description provided.