-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Use combination of namespace, GitHub URL, and runner group when hashing the listener name #4299
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
|
Hello! Thank you for your contribution. Please review our contribution guidelines to understand the project's testing and code conventions. |
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 refactors resource naming functions to improve uniqueness by incorporating additional attributes into hash generation. The changes aim to prevent naming collisions when multiple AutoscalingRunnerSets in different contexts share the same name.
- Introduced
hashSuffixhelper function that combines namespace, runner group, and config URL for hash generation - Updated
scaleSetListenerNameto use the enhanced hash suffix - Simplified
proxyListenerSecretNameto derive name from listener name rather than constructing it independently
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespaceHash = namespaceHash[:8] | ||
| } | ||
| return fmt.Sprintf("%v-%v-listener-proxy", autoscalingListener.Spec.AutoscalingRunnerSetName, namespaceHash) | ||
| return autoscalingListener.Name + "-proxy" |
Copilot
AI
Nov 3, 2025
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 proxy secret naming has changed from being based on AutoscalingRunnerSetName to being based on autoscalingListener.Name. This is a breaking change that will cause existing proxy secrets to be orphaned and new ones to be created with different names. Consider implementing a migration strategy or documenting this breaking change for users upgrading.
| func scaleSetListenerName(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) string { | ||
| namespaceHash := hash.FNVHashString(autoscalingRunnerSet.Namespace) | ||
| func hashSuffix(namespace, runnerGroup, configURL string) string { | ||
| namespaceHash := hash.FNVHashString(namespace + "@" + runnerGroup + "@" + configURL) |
Copilot
AI
Nov 3, 2025
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 hash input format uses '@' as a delimiter between components. If any of these fields (namespace, runnerGroup, or configURL) can contain '@' characters, this could lead to hash collisions. Consider using a more robust delimiter strategy or including field separators that cannot appear in the input values.
| namespaceHash := hash.FNVHashString(namespace + "@" + runnerGroup + "@" + configURL) | |
| data, err := json.Marshal([]string{namespace, runnerGroup, configURL}) | |
| if err != nil { | |
| panic(fmt.Sprintf("failed to marshal hash input: %v", err)) | |
| } | |
| namespaceHash := hash.FNVHashString(string(data)) |
GitHub URL, namespace and runner group should provide a unique hash part of the name. Since all listeners go into a single namespace, if the scale set is named the same belonging to a different runner group, the collisions could occur.
This change uses combination of the namespace, GitHub URL, and the runner group along with the name should provide a unique combination especially during testing canary release in the same namespace.
Fixes #3587