Fix concurrent map access in NBSetupKey controller

internal/controller/nbsetupkey_controller.go

@@ -5,6 +5,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"sync"
 
 	"github.com/google/uuid"
 	corev1 "k8s.io/api/core/v1"
@@ -24,6 +25,9 @@ type NBSetupKeyReconciler struct {
 	client.Client
 
 	ReferencedSecrets map[string]types.NamespacedName
+	// referencedSecretsMu guards ReferencedSecrets, which Reconcile writes while
+	// the secret watch handler reads it from another goroutine.
+	referencedSecretsMu sync.RWMutex
 }
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
@@ -55,13 +59,15 @@ func (r *NBSetupKeyReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	// Handle updated secret name
+	r.referencedSecretsMu.Lock()
 	for k, v := range r.ReferencedSecrets {
 		if v == req.NamespacedName {
 			delete(r.ReferencedSecrets, k)
 			break
 		}
 	}
 	r.ReferencedSecrets[fmt.Sprintf("%s/%s", nbSetupKey.Namespace, nbSetupKey.Spec.SecretKeyRef.Name)] = req.NamespacedName
+	r.referencedSecretsMu.Unlock()
 
 	secret := corev1.Secret{}
 	err = r.Get(ctx, types.NamespacedName{Namespace: nbSetupKey.Namespace, Name: nbSetupKey.Spec.SecretKeyRef.Name}, &secret)
@@ -125,17 +131,23 @@ func (r *NBSetupKeyReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Named("nbsetupkey").
 		Watches(
 			&corev1.Secret{},
-			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request {
-				if v, ok := r.ReferencedSecrets[fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())]; ok {
-					return []reconcile.Request{
-						{
-							NamespacedName: v,
-						},
-					}
-				}
-
-				return nil
-			}),
+			handler.EnqueueRequestsFromMapFunc(r.mapSecretToNBSetupKey),
 		). // Trigger reconciliation when a referenced secret changes
 		Complete(r)
 }
+
+// mapSecretToNBSetupKey returns a request for the NBSetupKey referencing the changed secret.
+func (r *NBSetupKeyReconciler) mapSecretToNBSetupKey(_ context.Context, obj client.Object) []reconcile.Request {
+	r.referencedSecretsMu.RLock()
+	v, ok := r.ReferencedSecrets[fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())]
+	r.referencedSecretsMu.RUnlock()
+	if !ok {
+		return nil
+	}
+
+	return []reconcile.Request{
+		{
+			NamespacedName: v,
+		},
+	}
+}

internal/controller/nbsetupkey_controller_test.go

@@ -176,6 +176,49 @@ var _ = Describe("NBSetupKey Controller", func() {
 					Expect(controllerReconciler.ReferencedSecrets).To(HaveKey("default/test-resource"))
 				})
 			})
+
+			When("secret events arrive while reconciling", func() {
+				It("should map the secret without racing reconcile", func() {
+					createSecret("setupkey", "EEEEEEEE-EEEE-EEEE-EEEE-EEEEEEEEEEEE")
+
+					controllerReconciler := &NBSetupKeyReconciler{
+						Client:            k8sClient,
+						ReferencedSecrets: make(map[string]types.NamespacedName),
+					}
+					secretObj := &corev1.Secret{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "default",
+							Name:      resourceName,
+						},
+					}
+
+					// Reconcile and the secret watch handler run in different goroutines in the manager.
+					done := make(chan struct{})
+					go func() {
+						defer GinkgoRecover()
+						defer close(done)
+						for range 50 {
+							_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
+								NamespacedName: typeNamespacedName,
+							})
+							Expect(err).NotTo(HaveOccurred())
+						}
+					}()
+					for {
+						select {
+						case <-done:
+							Expect(controllerReconciler.mapSecretToNBSetupKey(ctx, secretObj)).To(Equal([]reconcile.Request{
+								{
+									NamespacedName: typeNamespacedName,
+								},
+							}))
+							return
+						default:
+							controllerReconciler.mapSecretToNBSetupKey(ctx, secretObj)
+						}
+					}
+				})
+			})
 		})
 	})
 })