certrotationcontroller: ensure secrets/configmaps are unique

Collect a list of secrets/configmaps tracked in controllers to
make sure any secret/configmap is not being declared twice
to avoid races when updating it

Author: vrutkovs

pkg/operator/certrotation/client_cert_rotation_controller.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -80,6 +81,8 @@ func NewCertRotationController(
 	rotatedSelfSignedCertKeySecret RotatedSelfSignedCertKeySecret,
 	recorder events.Recorder,
 	reporter StatusReporter,
+	controlledSecrets *[]metav1.ObjectMeta,
+	controlledConfigMaps *[]metav1.ObjectMeta,
 ) factory.Controller {
 	c := &CertRotationController{
 		Name:                   name,
@@ -88,6 +91,19 @@ func NewCertRotationController(
 		RotatedTargetSecrets:   []RotatedSelfSignedCertKeySecret{rotatedSelfSignedCertKeySecret},
 		StatusReporter:         reporter,
 	}
+	*controlledSecrets = append(*controlledSecrets, metav1.ObjectMeta{
+		Name:      rotatedSigningCASecret.Name,
+		Namespace: rotatedSigningCASecret.Namespace,
+	})
+	*controlledConfigMaps = append(*controlledConfigMaps, metav1.ObjectMeta{
+		Name:      caBundleConfigMap.Name,
+		Namespace: caBundleConfigMap.Namespace,
+	})
+	*controlledSecrets = append(*controlledSecrets, metav1.ObjectMeta{
+		Name:      rotatedSelfSignedCertKeySecret.Name,
+		Namespace: rotatedSelfSignedCertKeySecret.Namespace,
+	})
+
 	return factory.New().
 		ResyncEvery(time.Minute).
 		WithSync(c.Sync).
@@ -112,14 +128,28 @@ func NewCertRotationControllerMultipleTargets(
 	rotatedTargetSecrets []RotatedSelfSignedCertKeySecret,
 	recorder events.Recorder,
 	reporter StatusReporter,
+	controlledSecrets *[]metav1.ObjectMeta,
+	controlledConfigMaps *[]metav1.ObjectMeta,
 ) factory.Controller {
 	informers := sets.New[factory.Informer](
 		rotatedSigningCASecret.Informer.Informer(),
 		caBundleConfigMap.Informer.Informer(),
 	)
 
+	*controlledSecrets = append(*controlledSecrets, metav1.ObjectMeta{
+		Name:      rotatedSigningCASecret.Name,
+		Namespace: rotatedSigningCASecret.Namespace,
+	})
+	*controlledConfigMaps = append(*controlledConfigMaps, metav1.ObjectMeta{
+		Name:      caBundleConfigMap.Name,
+		Namespace: caBundleConfigMap.Namespace,
+	})
 	for _, target := range rotatedTargetSecrets {
 		informers = informers.Insert(target.Informer.Informer())
+		*controlledSecrets = append(*controlledSecrets, metav1.ObjectMeta{
+			Name:      target.Name,
+			Namespace: target.Namespace,
+		})
 	}
 
 	c := &CertRotationController{

pkg/operator/certrotation/client_cert_rotation_controller_test.go

@@ -121,7 +121,9 @@ func TestCertRotationController(t *testing.T) {
 		UseSecretUpdateOnly:   true,
 	}
 
-	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{})
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
+	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 
 	time.AfterFunc(1*time.Second, func() {
 		cancel()
@@ -233,8 +235,10 @@ func TestCertRotationControllerIdempotent(t *testing.T) {
 		UseSecretUpdateOnly:   true,
 	}
 
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
 	// Run sync once to get signer / cabundle / target filled up
-	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{})
+	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 	syncCtx := factory.NewSyncContext("test", eventstesting.NewTestingEventRecorder(t))
 	err := c.Sync(controllerCtx, syncCtx)
 	if err != nil {
@@ -334,8 +338,10 @@ func TestCertRotationControllerSignerUpdate(t *testing.T) {
 		UseSecretUpdateOnly:   true,
 	}
 
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
 	// Run sync once to get signer / cabundle / target filled up
-	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{})
+	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 	syncCtx := factory.NewSyncContext("test", eventstesting.NewTestingEventRecorder(t))
 	err := c.Sync(controllerCtx, syncCtx)
 	if err != nil {
@@ -472,8 +478,10 @@ func TestCertRotationControllerFilterExpiredSigners(t *testing.T) {
 		UseSecretUpdateOnly:   true,
 	}
 
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
 	// Run sync once to get signer / cabundle / target filled up
-	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{})
+	c := NewCertRotationController("operator", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 	syncCtx := factory.NewSyncContext("test", eventstesting.NewTestingEventRecorder(t))
 	err := c.Sync(controllerCtx, syncCtx)
 	if err != nil {
@@ -624,8 +632,10 @@ func TestCertRotationControllerParallelUpdate(t *testing.T) {
 		UseSecretUpdateOnly:   false,
 	}
 
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
 	syncCtx := factory.NewSyncContext("test", eventstesting.NewTestingEventRecorder(t))
-	c1 := NewCertRotationController("c1", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{})
+	c1 := NewCertRotationController("c1", signerSecret, caBundleConfigMap, targetSecret, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 
 	// Sync first controller to get first copy of signer/cabundle
 	err := c1.Sync(controllerCtx, syncCtx)
@@ -684,7 +694,9 @@ func TestCertRotationControllerParallelUpdate(t *testing.T) {
 			UseSecretUpdateOnly:   true,
 		}
 		name := fmt.Sprintf("c%d", i)
-		ctrl := NewCertRotationController(name, signerSecret, caBundleConfigMap, targetNewSecret, eventRecorder, &fakeStatusReporter{})
+		controlledSecrets := []metav1.ObjectMeta{}
+		controlledConfigMaps := []metav1.ObjectMeta{}
+		ctrl := NewCertRotationController(name, signerSecret, caBundleConfigMap, targetNewSecret, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 		controllers[name] = ctrl
 	}
 
@@ -812,7 +824,9 @@ func TestCertRotationControllerMultipleTargets(t *testing.T) {
 		UseSecretUpdateOnly:   true,
 	}
 
-	c := NewCertRotationControllerMultipleTargets("operator", signerSecret, caBundleConfigMap, []RotatedSelfSignedCertKeySecret{targetFirstSecret, targetSecondSecret}, eventRecorder, &fakeStatusReporter{})
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
+	c := NewCertRotationControllerMultipleTargets("operator", signerSecret, caBundleConfigMap, []RotatedSelfSignedCertKeySecret{targetFirstSecret, targetSecondSecret}, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 
 	time.AfterFunc(1*time.Second, func() {
 		cancel()
@@ -954,7 +968,9 @@ func TestCertRotationControllerMultipleTargetsPostRunHooks(t *testing.T) {
 	// Ensure we don't leak goroutines
 	initialNumGoroutine := runtime.NumGoroutine()
 
-	c := NewCertRotationControllerMultipleTargets("operator", signerSecret, caBundleConfigMap, []RotatedSelfSignedCertKeySecret{targetFirstSecret, targetSecondSecret}, eventRecorder, &fakeStatusReporter{})
+	controlledSecrets := []metav1.ObjectMeta{}
+	controlledConfigMaps := []metav1.ObjectMeta{}
+	c := NewCertRotationControllerMultipleTargets("operator", signerSecret, caBundleConfigMap, []RotatedSelfSignedCertKeySecret{targetFirstSecret, targetSecondSecret}, eventRecorder, &fakeStatusReporter{}, &controlledSecrets, &controlledConfigMaps)
 
 	informerFactory.Start(controllerCtx.Done())
 	hasSecretSynced := cache.WaitForCacheSync(controllerCtx.Done(), informerFactory.Core().V1().Secrets().Informer().HasSynced)