-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Allow listener service account annotations #4360
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?
Allow listener service account annotations #4360
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 configuring custom annotations and labels on the listener service account through the AutoscalingRunnerSet and AutoscalingListener specifications.
Changes:
- Added
ListenerServiceAccountstruct with annotations and labels fields to the CRD specs - Implemented reconciliation logic to update existing service accounts when metadata changes
- Extended RBAC permissions to allow update, patch, and delete operations on service accounts
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| controllers/actions.github.com/resourcebuilder_test.go | Added test coverage for listener service account metadata propagation |
| controllers/actions.github.com/resourcebuilder.go | Updated service account builder to merge custom labels and annotations |
| controllers/actions.github.com/autoscalinglistener_controller.go | Implemented reconciliation logic for service account updates and added RBAC permissions |
| config/rbac/role.yaml | Added patch and update verbs for service account operations |
| config/crd/bases/actions.github.com_autoscalingrunnersets.yaml | Added listenerServiceAccount field to CRD schema |
| config/crd/bases/actions.github.com_autoscalinglisteners.yaml | Added listenerServiceAccount field to CRD schema |
| charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml | Added listenerServiceAccount field to Helm chart CRD |
| charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml | Added listenerServiceAccount field to Helm chart CRD |
| apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go | Generated deep copy methods for new ListenerServiceAccount type |
| apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go | Added ListenerServiceAccount field to AutoscalingRunnerSetSpec |
| apis/actions.github.com/v1alpha1/autoscalinglistener_types.go | Defined ListenerServiceAccount type and added field to AutoscalingListenerSpec |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
| } | ||
|
|
Copilot
AI
Jan 21, 2026
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.
The logic here prevents overwriting existing labels, but the same pattern is not applied to annotations (lines 444-447). This inconsistency could lead to confusion. Consider either applying the same non-overwrite logic to annotations or documenting why annotations should overwrite while labels should not.
| // Unlike labels above, annotations from ListenerServiceAccount are intended | |
| // to fully define the ServiceAccount's annotations, so we copy them | |
| // directly instead of merging or preserving any existing values. |
What?
Why?
Listener service accounts are created without any annotations/labels and are not reconciled after creation. This adds an explicit spec field so users can provide metadata and have it applied consistently.
The goal is to resolve the following issue #4293 (comment)