Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+16 −8
Open

Use combination of namespace, GitHub URL, and runner group when hashing the listener name #4299

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2927df1
name collision modification
nikola-jokic Nov 1, 2025
4815f1b
revert ephemeral runner set secret name
nikola-jokic Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions controllers/actions.github.com/resourcebuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,20 +690,28 @@ func scaleSetListenerConfigName(autoscalingListener *v1alpha1.AutoscalingListene
return fmt.Sprintf("%s-config", autoscalingListener.Name)
}

func scaleSetListenerName(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) string {
namespaceHash := hash.FNVHashString(autoscalingRunnerSet.Namespace)
func hashSuffix(namespace, runnerGroup, configURL string) string {
namespaceHash := hash.FNVHashString(namespace + "@" + runnerGroup + "@" + configURL)
Copy link

Copilot AI Nov 3, 2025

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.

Suggested change
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))

Copilot uses AI. Check for mistakes.
if len(namespaceHash) > 8 {
namespaceHash = namespaceHash[:8]
}
return fmt.Sprintf("%v-%v-listener", autoscalingRunnerSet.Name, namespaceHash)
return namespaceHash
}

func scaleSetListenerName(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) string {
return fmt.Sprintf(
"%v-%v-listener",
autoscalingRunnerSet.Name,
hashSuffix(
autoscalingRunnerSet.Namespace,
autoscalingRunnerSet.Spec.RunnerGroup,
autoscalingRunnerSet.Spec.GitHubConfigUrl,
),
)
}

func proxyListenerSecretName(autoscalingListener *v1alpha1.AutoscalingListener) string {
namespaceHash := hash.FNVHashString(autoscalingListener.Spec.AutoscalingRunnerSetNamespace)
if len(namespaceHash) > 8 {
namespaceHash = namespaceHash[:8]
}
return fmt.Sprintf("%v-%v-listener-proxy", autoscalingListener.Spec.AutoscalingRunnerSetName, namespaceHash)
return autoscalingListener.Name + "-proxy"
Copy link

Copilot AI Nov 3, 2025

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.

Copilot uses AI. Check for mistakes.
}

func proxyEphemeralRunnerSetSecretName(ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet) string {
Expand Down
Loading