Compare secret data when reconciling listener config and proxy secrets

controllers/actions.github.com/autoscalinglistener_controller.go

@@ -17,6 +17,7 @@ limitations under the License.
 package actionsgithubcom
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"maps"
@@ -298,17 +299,32 @@ 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
 			}
+			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 +405,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
 			}

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())
+		})
 	})
 })
 
@@ -1056,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() {