diff --git a/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml b/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml index 2990ccc4..a5c9a258 100644 --- a/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml @@ -17,6 +17,8 @@ rules: verbs: - create - delete + - update + - patch - get - apiGroups: - "" diff --git a/charts/gha-runner-scale-set/templates/manager_role.yaml b/charts/gha-runner-scale-set/templates/manager_role.yaml index bbf92799..88ca5a3f 100644 --- a/charts/gha-runner-scale-set/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role.yaml @@ -41,6 +41,8 @@ rules: verbs: - create - delete + - update + - patch - get - apiGroups: - "" diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index c0aa8111..3b61a3ba 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -165,7 +165,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. desiredLabels := r.filterAndMergeLabels(serviceAccount.Labels, desiredServiceAccount.Labels) labelsModified := !maps.Equal(serviceAccount.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) annotationsModified := !maps.Equal(serviceAccount.Annotations, desiredAnnotations) if labelsModified || annotationsModified { updatedServiceAccount := serviceAccount.DeepCopy() @@ -208,7 +208,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. desiredRole := r.newScaleSetListenerRole(&autoscalingListener) desiredLabels := r.filterAndMergeLabels(listenerRole.Labels, desiredRole.Labels) labelsModified := !maps.Equal(listenerRole.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) annotationsModified := !maps.Equal(listenerRole.Annotations, desiredAnnotations) rulesModified := !reflect.DeepEqual(listenerRole.Rules, desiredRole.Rules) if labelsModified || annotationsModified || rulesModified { @@ -250,7 +250,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. ) desiredLabels := r.filterAndMergeLabels(listenerRoleBinding.Labels, desiredRoleBinding.Labels) labelsModified := !maps.Equal(listenerRoleBinding.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) annotationsModified := !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) if labelsModified || annotationsModified { updatedRoleBinding := listenerRoleBinding.DeepCopy() @@ -305,7 +305,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredListenerProxy.Labels) labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) if labelsModified || annotationsModified { updatedProxySecret := proxySecret.DeepCopy() @@ -391,7 +391,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } desiredLabels := r.filterAndMergeLabels(listenerConfigSecret.Labels, desiredSecret.Labels) labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) if labelsModified || annotationsModified { @@ -462,7 +462,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - shouldReCreate := desiredPod.Annotations[annotationKeyIntegrityHash] != listenerPod.Annotations[annotationKeyIntegrityHash] + shouldReCreate := desiredPod.Annotations[AnnotationKeyIntegrityHash] != listenerPod.Annotations[AnnotationKeyIntegrityHash] if shouldReCreate { log.Info("Listener pod dependency changed, recreating listener pod") if err := r.deleteListenerPod(ctx, &autoscalingListener, &listenerPod, log); err != nil { @@ -475,7 +475,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. desiredLabels := r.filterAndMergeLabels(listenerPod.Labels, desiredPod.Labels) labelsModified := !maps.Equal(listenerPod.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) annotationsModified := !maps.Equal(listenerPod.Annotations, desiredAnnotations) if labelsModified || annotationsModified { diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index e48f75ff..1e1e2bb4 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -142,13 +142,13 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } // Something has changed, we need to re-apply the pending phase and change hash annotation to trigger the update of runner scale set and listener. - if targetHash := autoscalingRunnerSet.Hash(); autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] != targetHash { + if targetHash := autoscalingRunnerSet.Hash(); autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] != targetHash { // TODO: apply the version label original := autoscalingRunnerSet.DeepCopy() if autoscalingRunnerSet.Annotations == nil { autoscalingRunnerSet.Annotations = map[string]string{} } - autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] = targetHash + autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] = targetHash if err := r.Patch(ctx, &autoscalingRunnerSet, client.MergeFrom(original)); err != nil { log.Error(err, "Failed to update autoscaling runner set with new change hash and pending phase") return ctrl.Result{}, err @@ -291,12 +291,12 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } - if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != desired.Annotations[annotationKeyIntegrityHash] { + if ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] != desired.Annotations[AnnotationKeyIntegrityHash] { original := ephemeralRunnerSet.DeepCopy() ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata ephemeralRunnerSet.Spec.EphemeralRunnerSpec = desired.Spec.EphemeralRunnerSpec ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerSet.Annotations = r.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerSet.Annotations = r.filterAndMergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) log.Info("Updating ephemeral runner set spec to match the desired spec") if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil { @@ -315,7 +315,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl if ephemeralRunnerLabelsModified || ephemeralRunnerAnnotationsModified || ephemeralRunnerMetadataModified { original := ephemeralRunnerSet.DeepCopy() ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerSet.Annotations = r.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerSet.Annotations = r.filterAndMergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata log.Info("Updating ephemeral runner set metadata to match desired labels and annotations") if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil { diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index 28d44ad1..d6783271 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -478,7 +478,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") originalRunnerSetUID := runnerSet.UID - originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash] + originalRunnerSetHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] patched := autoscalingRunnerSet.DeepCopy() patched.Spec.Template.Spec.Containers[0].Image = "ghcr.io/actions/runner:updated" @@ -492,7 +492,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should be updated in place") g.Expect(current.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image).To(Equal("ghcr.io/actions/runner:updated")) - g.Expect(current.Annotations[annotationKeyIntegrityHash]).NotTo(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec hash should change") + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).NotTo(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec hash should change") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -531,7 +531,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") originalRunnerSetUID := runnerSet.UID - originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash] + originalRunnerSetHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] patched := autoscalingRunnerSet.DeepCopy() max := 20 @@ -557,7 +557,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should not be recreated") - g.Expect(current.Annotations[annotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec should not change") + g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec should not change") }, time.Second*5, autoscalingRunnerSetTestInterval, diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 092b93f6..a6cb4d27 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -135,7 +135,7 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R // If hash spec has changed, delete idle ephemeral runners // in order to apply the change to the runners that did not yet receive a job. ephemeralRunnerIntegrityHash := ephemeralRunnerSetIntegrityHash(&ephemeralRunnerSet) - if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != ephemeralRunnerIntegrityHash { + if ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] != ephemeralRunnerIntegrityHash { log.Info("EphemeralRunnerSpec has changed, deleting idle ephemeral runners to apply the new spec") if _, err := r.cleanUpEphemeralRunners(ctx, &ephemeralRunnerSet, log); err != nil { log.Error(err, "Failed to clean up EphemeralRunners") @@ -152,7 +152,7 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R if ephemeralRunnerSet.Annotations == nil { ephemeralRunnerSet.Annotations = make(map[string]string) } - ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerIntegrityHash + ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] = ephemeralRunnerIntegrityHash if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil { log.Error(err, "Failed to update ephemeral runner set with new spec hash") return ctrl.Result{}, err @@ -467,7 +467,7 @@ func (r *EphemeralRunnerSetReconciler) reconcileEphemeralRunnerSetProxySecret(ct dataModified := !maps.EqualFunc(proxySecret.Data, desiredRunnerSetProxy.Data, bytes.Equal) desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredRunnerSetProxy.Labels) labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) - desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredRunnerSetProxy.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(proxySecret.Annotations, desiredRunnerSetProxy.Annotations) annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) if dataModified || labelsModified || annotationsModified { updatedProxySecret := proxySecret.DeepCopy() diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index fd456b56..7c50a092 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -46,14 +46,14 @@ var commonLabelKeys = [...]string{ LabelKeyGitHubRepository, } -// annotationKeyIntegrityHash is used as a hash of the important fields +// AnnotationKeyIntegrityHash is used as a hash of the important fields // of each resource to determine if more drastic action should be taken. // // For example, annotations/labels are not something that should modify // the behavior of a resource, while the change in spec is. Therefore, // the spec hash should contain the spec fields in order to determine // modifications. -const annotationKeyIntegrityHash = "actions.github.com/integrity-hash" +const AnnotationKeyIntegrityHash = "actions.github.com/integrity-hash" const labelValueKubernetesPartOf = "gha-runner-scale-set" @@ -165,12 +165,12 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. } annotations := map[string]string{ - annotationKeyIntegrityHash: spec.Hash(), + AnnotationKeyIntegrityHash: spec.Hash(), } if autoscalingRunnerSet.Spec.AutoscalingListenerMetadata != nil { labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Labels, labels) - annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) + annotations = b.filterAndMergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) } autoscalingListener := &v1alpha1.AutoscalingListener{ @@ -278,7 +278,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha }, } - desiredSecret.Annotations[annotationKeyIntegrityHash] = scaleSetListenerConfigIntegrityHash(desiredSecret) + desiredSecret.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerConfigIntegrityHash(desiredSecret) if err := b.setControllerReference(autoscalingListener, desiredSecret); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener config secret: %w", err) @@ -426,7 +426,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod( Spec: podSpec, } - newRunnerScaleSetListenerPod.Annotations[annotationKeyIntegrityHash] = scaleSetListenerPodIntegrity( + newRunnerScaleSetListenerPod.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerPodIntegrity( newRunnerScaleSetListenerPod, autoscalingListener, podConfig, @@ -468,11 +468,11 @@ func scaleSetListenerPodIntegrity( d := data{ ListenerPodSpec: &pod.Spec, - AutoscalingListenerIntegrityHash: autoscalingListener.Annotations[annotationKeyIntegrityHash], - ConfigSecretIntegrityHash: podConfig.Annotations[annotationKeyIntegrityHash], - ServiceAccountIntegrityHash: serviceAccount.Annotations[annotationKeyIntegrityHash], - RoleIntegrityHash: role.Annotations[annotationKeyIntegrityHash], - RoleBindingIntegrityHash: roleBinding.Annotations[annotationKeyIntegrityHash], + AutoscalingListenerIntegrityHash: autoscalingListener.Annotations[AnnotationKeyIntegrityHash], + ConfigSecretIntegrityHash: podConfig.Annotations[AnnotationKeyIntegrityHash], + ServiceAccountIntegrityHash: serviceAccount.Annotations[AnnotationKeyIntegrityHash], + RoleIntegrityHash: role.Annotations[AnnotationKeyIntegrityHash], + RoleBindingIntegrityHash: roleBinding.Annotations[AnnotationKeyIntegrityHash], MetricsConfig: metricsConfig, } @@ -611,10 +611,10 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener if autoscalingListener.Spec.ServiceAccountMetadata != nil { base.Labels = b.filterAndMergeLabels(autoscalingListener.Spec.ServiceAccountMetadata.Labels, base.Labels) - base.Annotations = b.mergeAnnotations(autoscalingListener.Spec.ServiceAccountMetadata.Annotations, base.Annotations) + base.Annotations = b.filterAndMergeAnnotations(autoscalingListener.Spec.ServiceAccountMetadata.Annotations, base.Annotations) } - base.Annotations[annotationKeyIntegrityHash] = scaleSetListenerServiceAccountIntegrityHash(base) + base.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerServiceAccountIntegrityHash(base) if err := b.setControllerReference(autoscalingListener, base); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener service account: %w", err) @@ -650,7 +650,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. annotations := make(map[string]string) if autoscalingListener.Spec.RoleMetadata != nil { labels = b.filterAndMergeLabels(autoscalingListener.Spec.RoleMetadata.Labels, labels) - annotations = b.mergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) + annotations = b.filterAndMergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) } newRole := &rbacv1.Role{ @@ -663,7 +663,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. Rules: rulesForListenerRole([]string{autoscalingListener.Spec.EphemeralRunnerSetName}), } - newRole.Annotations[annotationKeyIntegrityHash] = scaleSetRoleIntegrityHash(newRole) + newRole.Annotations[AnnotationKeyIntegrityHash] = scaleSetRoleIntegrityHash(newRole) return newRole } @@ -718,7 +718,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 Subjects: subjects, } - newRoleBinding.Annotations[annotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) + newRoleBinding.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) return newRoleBinding } @@ -777,7 +777,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A if autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata != nil { labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Labels, labels) - annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) + annotations = b.filterAndMergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) } newEphemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{ @@ -791,7 +791,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A Spec: spec, } - newEphemeralRunnerSet.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerSetIntegrityHash(newEphemeralRunnerSet) + newEphemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] = ephemeralRunnerSetIntegrityHash(newEphemeralRunnerSet) if err := b.setControllerReference(autoscalingRunnerSet, newEphemeralRunnerSet); err != nil { return nil, fmt.Errorf("failed to set controller reference for ephemeral runner set: %w", err) @@ -825,7 +825,7 @@ func (b *ResourceBuilder) newAutoscalingListenerProxySecret(autoscalingListener Data: data, } - newProxySecret.Annotations[annotationKeyIntegrityHash] = autoscalingListenerProxySecretIntegrityHash(newProxySecret) + newProxySecret.Annotations[AnnotationKeyIntegrityHash] = autoscalingListenerProxySecretIntegrityHash(newProxySecret) if err := b.setControllerReference(autoscalingListener, newProxySecret); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener proxy secret: %w", err) @@ -857,7 +857,7 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme if ephemeralRunnerSet.Spec.EphemeralRunnerMetadata != nil { labels = b.filterAndMergeLabels(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Labels, labels) - annotations = b.mergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) + annotations = b.filterAndMergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) } ephemeralRunner := &v1alpha1.EphemeralRunner{ @@ -992,7 +992,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSetProxySecret(ephemeralRunnerSet *v Data: data, } - runnerPodProxySecret.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerSetProxySecretZIdentityHash(runnerPodProxySecret) + runnerPodProxySecret.Annotations[AnnotationKeyIntegrityHash] = ephemeralRunnerSetProxySecretZIdentityHash(runnerPodProxySecret) if err := b.setControllerReference(ephemeralRunnerSet, runnerPodProxySecret); err != nil { return nil, fmt.Errorf("failed to set controller reference for ephemeral runner set proxy secret: %w", err) @@ -1093,40 +1093,53 @@ func trimLabelValue(val string) string { return strings.Trim(val, "-_.") } +func (b *ResourceBuilder) filterLabels(k, v string) bool { + for _, prefix := range b.ExcludeLabelPropagationPrefixes { + if strings.HasPrefix(k, prefix) { + return true + } + } + return false +} + func (b *ResourceBuilder) filterAndMergeLabels(base, overwrite map[string]string) map[string]string { + return filterAndMergeMaps(base, overwrite, b.filterLabels) +} + +func filterAndMergeMaps(base, overwrite map[string]string, filter func(k, v string) bool) map[string]string { if base == nil && overwrite == nil { return nil } + var result map[string]string + if len(base) == 0 { + result = make(map[string]string) + } else { + result = maps.Clone(base) + } + if len(overwrite) > 0 { + maps.Copy(result, overwrite) + } + maps.DeleteFunc(result, filter) + return result +} - mergedLabels := make(map[string]string, len(base)) -base: - for k, v := range base { - for _, prefix := range b.ExcludeLabelPropagationPrefixes { - if strings.HasPrefix(k, prefix) { - continue base - } - } - mergedLabels[k] = v +func (b *ResourceBuilder) filterAndMergeAnnotations(base, overwrite map[string]string) map[string]string { + if base == nil && overwrite == nil { + return nil + } + var result map[string]string + if len(base) == 0 { + result = make(map[string]string) + } else { + result = maps.Clone(base) } -overwrite: for k, v := range overwrite { - for _, prefix := range b.ExcludeLabelPropagationPrefixes { - if strings.HasPrefix(k, prefix) { - continue overwrite - } + if k == AnnotationKeyIntegrityHash { + continue } - mergedLabels[k] = v + result[k] = v } - return mergedLabels -} - -func (b *ResourceBuilder) mergeAnnotations(base, overwrite map[string]string) map[string]string { - if base == nil && overwrite == nil { - return nil - } - base = maps.Clone(base) - maps.Copy(base, overwrite) - return base + return result } diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index d0885117..206e6107 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/controllers/actions.github.com/resourcebuilder_test.go @@ -113,7 +113,7 @@ func TestMetadataPropagation(t *testing.T) { assert.Equal(t, labelValueKubernetesPartOf, ephemeralRunnerSet.Labels[LabelKeyKubernetesPartOf]) assert.Equal(t, "runner-set", ephemeralRunnerSet.Labels[LabelKeyKubernetesComponent]) assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], ephemeralRunnerSet.Labels[LabelKeyKubernetesVersion]) - assert.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash]) + assert.NotEmpty(t, ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash]) assert.Equal(t, autoscalingRunnerSet.Name, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetName]) assert.Equal(t, autoscalingRunnerSet.Namespace, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetNamespace]) assert.Equal(t, "", ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise]) @@ -130,7 +130,7 @@ func TestMetadataPropagation(t *testing.T) { assert.Equal(t, labelValueKubernetesPartOf, listener.Labels[LabelKeyKubernetesPartOf]) assert.Equal(t, "runner-scale-set-listener", listener.Labels[LabelKeyKubernetesComponent]) assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], listener.Labels[LabelKeyKubernetesVersion]) - assert.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash]) + assert.NotEmpty(t, ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash]) assert.Equal(t, autoscalingRunnerSet.Name, listener.Labels[LabelKeyGitHubScaleSetName]) assert.Equal(t, autoscalingRunnerSet.Namespace, listener.Labels[LabelKeyGitHubScaleSetNamespace]) assert.Equal(t, "", listener.Labels[LabelKeyGitHubEnterprise]) @@ -221,7 +221,7 @@ func TestEphemeralRunnerSetProxySecretZIdentityHash(t *testing.T) { }) require.NoError(t, err) - actualHash := proxySecret.Annotations[annotationKeyIntegrityHash] + actualHash := proxySecret.Annotations[AnnotationKeyIntegrityHash] assert.NotEmpty(t, actualHash) assert.Equal(t, ephemeralRunnerSetProxySecretZIdentityHash(proxySecret), actualHash) @@ -313,7 +313,7 @@ func TestOwnershipRelationships(t *testing.T) { runnerScaleSetIDAnnotationKey: "1", AnnotationKeyGitHubRunnerGroupName: "test-group", AnnotationKeyGitHubRunnerScaleSetName: "test-scale-set", - annotationKeyIntegrityHash: "test-hash", + AnnotationKeyIntegrityHash: "test-hash", }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{