From 4960e6e5e12536000304a41f2043a6d988a79821 Mon Sep 17 00:00:00 2001 From: Junya Okabe Date: Mon, 13 Jul 2026 00:07:29 +0900 Subject: [PATCH 1/2] fix(gha): compare secret data when reconciling listener config and proxy secrets --- .../autoscalinglistener_controller.go | 13 ++++- .../autoscalinglistener_controller_test.go | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index c0aa81114c..7458a021c6 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -17,6 +17,7 @@ limitations under the License. package actionsgithubcom import ( + "bytes" "context" "fmt" "maps" @@ -303,12 +304,16 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. log.Error(err, "Failed to build desired listener proxy secret") return ctrl.Result{}, err } + dataModified := !maps.EqualFunc(proxySecret.Data, desiredListenerProxy.Data, bytes.Equal) desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredListenerProxy.Labels) labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) - if labelsModified || annotationsModified { + if dataModified || labelsModified || annotationsModified { updatedProxySecret := proxySecret.DeepCopy() + if dataModified { + updatedProxySecret.Data = desiredListenerProxy.Data + } if labelsModified { updatedProxySecret.Labels = desiredLabels } @@ -389,13 +394,17 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if err != nil { return ctrl.Result{}, fmt.Errorf("failed to build listener config secret: %w", err) } + dataModified := !maps.EqualFunc(listenerConfigSecret.Data, desiredSecret.Data, bytes.Equal) desiredLabels := r.filterAndMergeLabels(listenerConfigSecret.Labels, desiredSecret.Labels) labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) - if labelsModified || annotationsModified { + if dataModified || labelsModified || annotationsModified { updatedSecret := listenerConfigSecret.DeepCopy() + if dataModified { + updatedSecret.Data = desiredSecret.Data + } if labelsModified { updatedSecret.Labels = desiredLabels } diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index d48d613e6c..3a3b2bc8c7 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -549,6 +549,58 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingListenerTestInterval, ).Should(BeEquivalentTo(oldSecretUID), "Config secret should persist (not be re-created)") }) + + It("It should propagate rotated GitHub credentials into the listener config secret", func() { + secret := new(corev1.Secret) + Eventually( + func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, secret) + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed(), "Config secret should be created") + + var originalConfig ghalistenerconfig.Config + err := json.Unmarshal(secret.Data["config.json"], &originalConfig) + Expect(err).NotTo(HaveOccurred(), "failed to parse listener configuration file") + Expect(originalConfig.Token).To(Equal(defaultGitHubToken)) + oldHash := secret.Annotations[annotationKeyIntegrityHash] + + // Rotate the GitHub credentials. There is intentionally no watch on the + // credentials secret, so reconciliation is triggered below by mutating the listener. + const rotatedGitHubToken = "gh_token_rotated" + currentCredentials := new(corev1.Secret) + err = k8sClient.Get(ctx, client.ObjectKey{Name: configSecret.Name, Namespace: configSecret.Namespace}, currentCredentials) + Expect(err).NotTo(HaveOccurred(), "failed to get credentials secret") + updatedCredentials := currentCredentials.DeepCopy() + updatedCredentials.Data["github_token"] = []byte(rotatedGitHubToken) + err = k8sClient.Update(ctx, updatedCredentials) + Expect(err).NotTo(HaveOccurred(), "failed to update credentials secret") + + current := new(v1alpha1.AutoscalingListener) + err = k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, current) + Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingListener") + updatedListener := current.DeepCopy() + updatedListener.Labels["arc.test/listener-label"] = "rotated" + err = k8sClient.Patch(ctx, updatedListener, client.MergeFrom(current)) + Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingListener to trigger reconcile") + + Eventually( + func(g Gomega) { + updatedSecret := new(corev1.Secret) + err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, updatedSecret) + g.Expect(err).NotTo(HaveOccurred(), "failed to get config Secret") + + var rotatedConfig ghalistenerconfig.Config + err = json.Unmarshal(updatedSecret.Data["config.json"], &rotatedConfig) + g.Expect(err).NotTo(HaveOccurred(), "failed to parse listener configuration file") + g.Expect(rotatedConfig.Token).To(Equal(rotatedGitHubToken), "config secret should embed the rotated token") + g.Expect(updatedSecret.Annotations[annotationKeyIntegrityHash]).NotTo(Equal(oldHash), "integrity hash annotation should change") + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed()) + }) }) }) From b124b9afa574dc4f091843a657222c97d54964cb Mon Sep 17 00:00:00 2001 From: Junya Okabe Date: Mon, 13 Jul 2026 00:33:11 +0900 Subject: [PATCH 2/2] fix(gha): recompute desired proxy secret data from referenced secrets --- .../autoscalinglistener_controller.go | 13 ++- .../autoscalinglistener_controller_test.go | 94 +++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 7458a021c6..2ac368824d 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -299,7 +299,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. ) switch { case err == nil: - desiredListenerProxy, err := r.newAutoscalingListenerProxySecret(&autoscalingListener, proxySecret.Data) + proxySecretData, err := autoscalingListener.Spec.Proxy.ToSecretData(func(s string) (*corev1.Secret, error) { + var secret corev1.Secret + err := r.Get(ctx, types.NamespacedName{Name: s, Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace}, &secret) + if err != nil { + return nil, fmt.Errorf("failed to get secret %s: %w", s, err) + } + return &secret, nil + }) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to convert proxy config to secret data: %w", err) + } + desiredListenerProxy, err := r.newAutoscalingListenerProxySecret(&autoscalingListener, proxySecretData) if err != nil { log.Error(err, "Failed to build desired listener proxy secret") return ctrl.Result{}, err diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 3a3b2bc8c7..a3a8a6eb8e 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -1108,6 +1108,100 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() { autoscalingListenerTestInterval, ).Should(Succeed(), "failed to delete secret with proxy details") }) + + It("should propagate rotated proxy credentials into the mirror proxy secret", func() { + proxyCredentials := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-credentials", + Namespace: autoscalingNS.Name, + }, + Data: map[string][]byte{ + "username": []byte("test"), + "password": []byte("password"), + }, + } + + err := k8sClient.Create(ctx, proxyCredentials) + Expect(err).NotTo(HaveOccurred(), "failed to create proxy credentials secret") + + proxy := &v1alpha1.ProxyConfig{ + HTTP: &v1alpha1.ProxyServerConfig{ + Url: "http://localhost:8080", + CredentialSecretRef: "proxy-credentials", + }, + HTTPS: &v1alpha1.ProxyServerConfig{ + Url: "https://localhost:8443", + CredentialSecretRef: "proxy-credentials", + }, + NoProxy: []string{ + "example.com", + "example.org", + }, + } + + createRunnerSetAndListener(proxy) + + var proxySecret corev1.Secret + Eventually( + func(g Gomega) { + err := k8sClient.Get( + ctx, + types.NamespacedName{Name: proxyListenerSecretName(autoscalingListener), Namespace: autoscalingNS.Name}, + &proxySecret, + ) + g.Expect(err).NotTo(HaveOccurred(), "failed to get secret") + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed(), "failed to create secret with proxy details") + + // Rotate the proxy credentials. + currentCredentials := new(corev1.Secret) + err = k8sClient.Get(ctx, types.NamespacedName{Name: proxyCredentials.Name, Namespace: autoscalingNS.Name}, currentCredentials) + Expect(err).NotTo(HaveOccurred(), "failed to get proxy credentials secret") + updatedCredentials := currentCredentials.DeepCopy() + updatedCredentials.Data["password"] = []byte("rotated-password") + err = k8sClient.Update(ctx, updatedCredentials) + Expect(err).NotTo(HaveOccurred(), "failed to update proxy credentials secret") + + // There is no watch on the proxy credentials secret, so reconciliation is + // triggered below by mutating the listener. + current := new(v1alpha1.AutoscalingListener) + err = k8sClient.Get(ctx, types.NamespacedName{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, current) + Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingListener") + updated := current.DeepCopy() + if updated.Labels == nil { + updated.Labels = map[string]string{} + } + updated.Labels["arc.test/rotate"] = "true" + err = k8sClient.Patch(ctx, updated, client.MergeFrom(current)) + Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingListener to trigger reconcile") + + Eventually( + func(g Gomega) { + var rotatedProxySecret corev1.Secret + err := k8sClient.Get( + ctx, + types.NamespacedName{Name: proxyListenerSecretName(autoscalingListener), Namespace: autoscalingNS.Name}, + &rotatedProxySecret, + ) + g.Expect(err).NotTo(HaveOccurred(), "failed to get secret") + + expected, err := autoscalingListener.Spec.Proxy.ToSecretData(func(s string) (*corev1.Secret, error) { + var secret corev1.Secret + err := k8sClient.Get(ctx, types.NamespacedName{Name: s, Namespace: autoscalingNS.Name}, &secret) + if err != nil { + return nil, err + } + return &secret, nil + }) + g.Expect(err).NotTo(HaveOccurred(), "failed to convert proxy config to secret data") + g.Expect(rotatedProxySecret.Data).To(Equal(expected), "mirror proxy secret should reflect the rotated credentials") + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed(), "failed to propagate rotated proxy credentials") + }) }) var _ = Describe("Test AutoScalingListener controller with template modification", func() {