Test filters in SupervisorSecretsController

cmd/pinniped-supervisor/main.go

@@ -159,6 +159,7 @@ func startControllers(
 					plog.Debug("setting csrf cookie secret")
 					secretCache.SetCSRFCookieEncoderHashKey(secret)
 				},
+				controllerlib.WithInformer,
 			),
 			singletonWorker,
 		).

internal/controller/supervisorconfig/generator/supervisor_secrets.go

@@ -27,36 +27,39 @@ import (
 var generateKey = generateSymmetricKey
 
 type supervisorSecretsController struct {
-	owner    *appsv1.Deployment
+	owner          *appsv1.Deployment
-	labels   map[string]string
+	labels         map[string]string
-	client   kubernetes.Interface
+	kubeClient     kubernetes.Interface
-	secrets  corev1informers.SecretInformer
+	secretInformer corev1informers.SecretInformer
-	setCache func(secret []byte)
+	setCacheFunc   func(secret []byte)
 }
 
 // NewSupervisorSecretsController instantiates a new controllerlib.Controller which will ensure existence of a generated secret.
 func NewSupervisorSecretsController(
 	// TODO: generate the name for the secret and label the secret with the UID of the owner? So that we don't have naming conflicts if the user has already created a Secret with that name.
-	// TODO: add tests for the filter like we do in the JWKSWriterController?
 	owner *appsv1.Deployment,
 	labels map[string]string,
-	client kubernetes.Interface,
+	kubeClient kubernetes.Interface,
-	secrets corev1informers.SecretInformer,
+	secretInformer corev1informers.SecretInformer,
-	setCache func(secret []byte),
+	setCacheFunc func(secret []byte),
+	withInformer pinnipedcontroller.WithInformerOptionFunc,
 ) controllerlib.Controller {
 	c := supervisorSecretsController{
-		owner:    owner,
+		owner:          owner,
-		labels:   labels,
+		labels:         labels,
-		client:   client,
+		kubeClient:     kubeClient,
-		secrets:  secrets,
+		secretInformer: secretInformer,
-		setCache: setCache,
+		setCacheFunc:   setCacheFunc,
 	}
-	filter := pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool {
-		return metav1.IsControlledBy(obj, owner)
-	}, nil)
 	return controllerlib.New(
 		controllerlib.Config{Name: owner.Name + "-secret-generator", Syncer: &c},
-		controllerlib.WithInformer(secrets, filter, controllerlib.InformerOption{}),
+		withInformer(
+			secretInformer,
+			pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool {
+				return metav1.IsControlledBy(obj, owner)
+			}, nil),
+			controllerlib.InformerOption{},
+		),
 		controllerlib.WithInitialEvent(controllerlib.Key{
 			Namespace: owner.Namespace,
 			Name:      owner.Name + "-key",
@@ -66,7 +69,7 @@ func NewSupervisorSecretsController(
 
 // Sync implements controllerlib.Syncer.Sync().
 func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error {
-	secret, err := c.secrets.Lister().Secrets(ctx.Key.Namespace).Get(ctx.Key.Name)
+	secret, err := c.secretInformer.Lister().Secrets(ctx.Key.Namespace).Get(ctx.Key.Name)
 	isNotFound := k8serrors.IsNotFound(err)
 	if !isNotFound && err != nil {
 		return fmt.Errorf("failed to list secret %s/%s: %w", ctx.Key.Namespace, ctx.Key.Name, err)
@@ -75,7 +78,7 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error {
 	secretNeedsUpdate := isNotFound || !isValid(secret)
 	if !secretNeedsUpdate {
 		plog.Debug("secret is up to date", "secret", klog.KObj(secret))
-		c.setCache(secret.Data[symmetricKeySecretDataKey])
+		c.setCacheFunc(secret.Data[symmetricKeySecretDataKey])
 		return nil
 	}
 
@@ -93,18 +96,18 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error {
 		return fmt.Errorf("failed to create/update secret %s/%s: %w", newSecret.Namespace, newSecret.Name, err)
 	}
 
-	c.setCache(newSecret.Data[symmetricKeySecretDataKey])
+	c.setCacheFunc(newSecret.Data[symmetricKeySecretDataKey])
 
 	return nil
 }
 
 func (c *supervisorSecretsController) createSecret(ctx context.Context, newSecret *corev1.Secret) error {
-	_, err := c.client.CoreV1().Secrets(newSecret.Namespace).Create(ctx, newSecret, metav1.CreateOptions{})
+	_, err := c.kubeClient.CoreV1().Secrets(newSecret.Namespace).Create(ctx, newSecret, metav1.CreateOptions{})
 	return err
 }
 
 func (c *supervisorSecretsController) updateSecret(ctx context.Context, newSecret **corev1.Secret, secretName string) error {
-	secrets := c.client.CoreV1().Secrets((*newSecret).Namespace)
+	secrets := c.kubeClient.CoreV1().Secrets((*newSecret).Namespace)
 	return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 		currentSecret, err := secrets.Get(ctx, secretName, metav1.GetOptions{})
 		isNotFound := k8serrors.IsNotFound(err)

internal/controller/supervisorconfig/generator/supervisor_secrets_test.go

@@ -21,10 +21,163 @@ import (
 	kubernetesfake "k8s.io/client-go/kubernetes/fake"
 	kubetesting "k8s.io/client-go/testing"
 
+	configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1"
 	"go.pinniped.dev/internal/controllerlib"
+	"go.pinniped.dev/internal/testutil"
 )
 
-func TestController(t *testing.T) {
+var (
+	owner = &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "some-owner-name",
+			UID:  "some-owner-uid",
+		},
+	}
+
+	ownerGVK = schema.GroupVersionKind{
+		Group:   appsv1.SchemeGroupVersion.Group,
+		Version: appsv1.SchemeGroupVersion.Version,
+		Kind:    "Deployment",
+	}
+
+	labels = map[string]string{
+		"some-label-key-1": "some-label-value-1",
+		"some-label-key-2": "some-label-value-2",
+	}
+)
+
+func TestSupervisorSecretsControllerFilterSecret(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name       string
+		secret     corev1.Secret
+		wantAdd    bool
+		wantUpdate bool
+		wantDelete bool
+	}{
+		{
+			name: "no owner reference",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{},
+			},
+		},
+		{
+			name: "owner reference without controller set to true",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "some-namespace",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: configv1alpha1.SchemeGroupVersion.String(),
+							Name:       "some-name",
+							Kind:       ownerGVK.Kind,
+							UID:        owner.GetUID(),
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "owner reference without correct APIVersion",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "some-namespace",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Name:       "some-name",
+							Kind:       ownerGVK.Kind,
+							Controller: boolPtr(true),
+							UID:        owner.GetUID(),
+						}},
+				},
+			},
+			wantAdd:    true,
+			wantUpdate: true,
+			wantDelete: true,
+		},
+		{
+			name: "owner reference without correct Kind",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "some-namespace",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: configv1alpha1.SchemeGroupVersion.String(),
+							Name:       "some-name",
+							Kind:       "IncorrectKind",
+							Controller: boolPtr(true),
+							UID:        owner.GetUID(),
+						},
+					},
+				},
+			},
+			wantAdd:    true,
+			wantUpdate: true,
+			wantDelete: true,
+		},
+		{
+			name: "correct owner reference",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "some-namespace",
+					OwnerReferences: []metav1.OwnerReference{
+						*metav1.NewControllerRef(owner, ownerGVK),
+					},
+				},
+			},
+			wantAdd:    true,
+			wantUpdate: true,
+			wantDelete: true,
+		},
+		{
+			name: "multiple owner references",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "some-namespace",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Kind: "UnrelatedKind",
+						},
+						*metav1.NewControllerRef(owner, ownerGVK),
+					},
+				},
+			},
+			wantAdd:    true,
+			wantUpdate: true,
+			wantDelete: true,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			secretInformer := kubeinformers.NewSharedInformerFactory(
+				kubernetesfake.NewSimpleClientset(),
+				0,
+			).Core().V1().Secrets()
+			withInformer := testutil.NewObservableWithInformerOption()
+			_ = NewSupervisorSecretsController(
+				owner,
+				labels,
+				nil, // kubeClient, not needed
+				secretInformer,
+				nil, // setCache, not needed
+				withInformer.WithInformer,
+			)
+
+			unrelated := corev1.Secret{}
+			filter := withInformer.GetFilterForInformer(secretInformer)
+			require.Equal(t, test.wantAdd, filter.Add(&test.secret))
+			require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.secret))
+			require.Equal(t, test.wantUpdate, filter.Update(&test.secret, &unrelated))
+			require.Equal(t, test.wantDelete, filter.Delete(&test.secret))
+		})
+	}
+}
+
+func TestSupervisorSecretsControllerSync(t *testing.T) {
 	const (
 		generatedSecretNamespace = "some-namespace"
 		generatedSecretName      = "some-name-abc123"
@@ -37,26 +190,9 @@ func TestController(t *testing.T) {
 			Resource: "secrets",
 		}
 
-		owner = &appsv1.Deployment{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "some-owner-name",
-				UID:  "some-owner-uid",
-			},
-		}
-		ownerGVK = schema.GroupVersionKind{
-			Group:   appsv1.SchemeGroupVersion.Group,
-			Version: appsv1.SchemeGroupVersion.Version,
-			Kind:    "Deployment",
-		}
-
 		generatedSymmetricKey      = []byte("some-neato-32-byte-generated-key")
 		otherGeneratedSymmetricKey = []byte("some-funio-32-byte-generated-key")
 
-		labels = map[string]string{
-			"some-label-key-1": "some-label-value-1",
-			"some-label-key-2": "some-label-value-2",
-		}
-
 		generatedSecret = &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      generatedSecretName,
@@ -313,10 +449,17 @@ func TestController(t *testing.T) {
 			secrets := informers.Core().V1().Secrets()
 
 			var callbackSecret []byte
-			c := NewSupervisorSecretsController(owner, labels, apiClient, secrets, func(secret []byte) {
+			c := NewSupervisorSecretsController(
-				require.Nil(t, callbackSecret, "callback was called twice")
+				owner,
-				callbackSecret = secret
+				labels,
-			})
+				apiClient,
+				secrets,
+				func(secret []byte) {
+					require.Nil(t, callbackSecret, "callback was called twice")
+					callbackSecret = secret
+				},
+				testutil.NewObservableWithInformerOption().WithInformer,
+			)
 
 			// Must start informers before calling TestRunSynchronously().
 			informers.Start(ctx.Done())