Integration test for Supervisor secret controllers

This forced us to add labels to the CSRF cookie secret, just as we do
for other Supervisor secrets. Yay tests.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

cmd/pinniped-supervisor/main.go

@@ -152,6 +152,7 @@ func startControllers(
 		WithController(
 			generator.NewSupervisorSecretsController(
 				supervisorDeployment,
+				cfg.Labels,
 				kubeClient,
 				secretInformer,
 				func(secret []byte) {

internal/controller/supervisorconfig/generator/supervisor_secrets.go

@@ -46,6 +46,7 @@ func generateSymmetricKey() ([]byte, error) {
 
 type supervisorSecretsController struct {
 	owner    *appsv1.Deployment
+	labels   map[string]string
 	client   kubernetes.Interface
 	secrets  corev1informers.SecretInformer
 	setCache func(secret []byte)
@@ -53,16 +54,17 @@ type supervisorSecretsController struct {
 
 // NewSupervisorSecretsController instantiates a new controllerlib.Controller which will ensure existence of a generated secret.
 func NewSupervisorSecretsController(
-	// TODO: label the generated secret like we do in the JWKSWriterController
 	// 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,
 	secrets corev1informers.SecretInformer,
 	setCache func(secret []byte),
 ) controllerlib.Controller {
 	c := supervisorSecretsController{
 		owner:    owner,
+		labels:   labels,
 		client:   client,
 		secrets:  secrets,
 		setCache: setCache,
@@ -95,7 +97,7 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error {
 		return nil
 	}
 
-	newSecret, err := generateSecret(ctx.Key.Namespace, ctx.Key.Name, secretDataFunc, c.owner)
+	newSecret, err := generateSecret(ctx.Key.Namespace, ctx.Key.Name, c.labels, secretDataFunc, c.owner)
 	if err != nil {
 		return fmt.Errorf("failed to generate secret: %w", err)
 	}
@@ -141,7 +143,7 @@ func secretDataFunc() (map[string][]byte, error) {
 	}, nil
 }
 
-func generateSecret(namespace, name string, secretDataFunc func() (map[string][]byte, error), owner metav1.Object) (*corev1.Secret, error) {
+func generateSecret(namespace, name string, labels map[string]string, secretDataFunc func() (map[string][]byte, error), owner metav1.Object) (*corev1.Secret, error) {
 	secretData, err := secretDataFunc()
 	if err != nil {
 		return nil, err
@@ -159,6 +161,7 @@ func generateSecret(namespace, name string, secretDataFunc func() (map[string][]
 			OwnerReferences: []metav1.OwnerReference{
 				*metav1.NewControllerRef(owner, deploymentGVK),
 			},
+			Labels: labels,
 		},
 		Type: symmetricKeySecretType,
 		Data: secretData,

internal/controller/supervisorconfig/generator/supervisor_secrets_test.go

@@ -28,7 +28,6 @@ func TestController(t *testing.T) {
 	const (
 		generatedSecretNamespace = "some-namespace"
 		generatedSecretName      = "some-name-abc123"
-		otherGeneratedSecretName = "some-other-name-abc123"
 	)
 
 	var (
@@ -53,6 +52,11 @@ func TestController(t *testing.T) {
 		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,
@@ -60,6 +64,7 @@ func TestController(t *testing.T) {
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(owner, ownerGVK),
 				},
+				Labels: labels,
 			},
 			Type: "secrets.pinniped.dev/symmetric",
 			Data: map[string][]byte{
@@ -74,6 +79,7 @@ func TestController(t *testing.T) {
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(owner, ownerGVK),
 				},
+				Labels: labels,
 			},
 			Type: "secrets.pinniped.dev/symmetric",
 			Data: map[string][]byte{
@@ -307,7 +313,7 @@ func TestController(t *testing.T) {
 			secrets := informers.Core().V1().Secrets()
 
 			var callbackSecret []byte
-			c := NewSupervisorSecretsController(owner, apiClient, secrets, func(secret []byte) {
+			c := NewSupervisorSecretsController(owner, labels, apiClient, secrets, func(secret []byte) {
 				require.Nil(t, callbackSecret, "callback was called twice")
 				callbackSecret = secret
 			})

test/integration/supervisor_keys_test.go

@@ -1,101 +0,0 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package integration
-
-import (
-	"context"
-	"encoding/json"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gopkg.in/square/go-jose.v2"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1"
-	"go.pinniped.dev/test/library"
-)
-
-func TestSupervisorOIDCKeys(t *testing.T) {
-	env := library.IntegrationEnv(t)
-	kubeClient := library.NewClientset(t)
-	supervisorClient := library.NewSupervisorClientset(t)
-
-	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
-	defer cancel()
-
-	// Create our OPC under test.
-	opc := library.CreateTestOIDCProvider(ctx, t, "", "", "")
-
-	// Ensure a secret is created with the OPC's JWKS.
-	var updatedOPC *configv1alpha1.OIDCProvider
-	var err error
-	assert.Eventually(t, func() bool {
-		updatedOPC, err = supervisorClient.
-			ConfigV1alpha1().
-			OIDCProviders(env.SupervisorNamespace).
-			Get(ctx, opc.Name, metav1.GetOptions{})
-		return err == nil && updatedOPC.Status.JWKSSecret.Name != ""
-	}, time.Second*10, time.Millisecond*500)
-	require.NoError(t, err)
-	require.NotEmpty(t, updatedOPC.Status.JWKSSecret.Name)
-
-	// Ensure the secret actually exists.
-	secret, err := kubeClient.
-		CoreV1().
-		Secrets(env.SupervisorNamespace).
-		Get(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.GetOptions{})
-	require.NoError(t, err)
-
-	// Ensure that the secret was labelled.
-	for k, v := range env.SupervisorCustomLabels {
-		require.Equalf(t, v, secret.Labels[k], "expected secret to have label `%s: %s`", k, v)
-	}
-	require.Equal(t, env.SupervisorAppName, secret.Labels["app"])
-
-	// Ensure the secret has an active key.
-	jwkData, ok := secret.Data["activeJWK"]
-	require.True(t, ok, "secret is missing active jwk")
-
-	// Ensure the secret's active key is valid.
-	var activeJWK jose.JSONWebKey
-	require.NoError(t, json.Unmarshal(jwkData, &activeJWK))
-	require.True(t, activeJWK.Valid(), "active jwk is invalid")
-	require.False(t, activeJWK.IsPublic(), "active jwk is public")
-
-	// Ensure the secret has a JWKS.
-	jwksData, ok := secret.Data["jwks"]
-	require.True(t, ok, "secret is missing jwks")
-
-	// Ensure the secret's JWKS is valid, public, and contains the singing key.
-	var jwks jose.JSONWebKeySet
-	require.NoError(t, json.Unmarshal(jwksData, &jwks))
-	foundActiveJWK := false
-	for _, jwk := range jwks.Keys {
-		require.Truef(t, jwk.Valid(), "jwk %s is invalid", jwk.KeyID)
-		require.Truef(t, jwk.IsPublic(), "jwk %s is not public", jwk.KeyID)
-		if jwk.KeyID == activeJWK.KeyID {
-			foundActiveJWK = true
-		}
-	}
-	require.True(t, foundActiveJWK, "could not find active JWK in JWKS: %s", jwks)
-
-	// Ensure upon deleting the secret, it is eventually brought back.
-	err = kubeClient.
-		CoreV1().
-		Secrets(env.SupervisorNamespace).
-		Delete(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.DeleteOptions{})
-	require.NoError(t, err)
-	assert.Eventually(t, func() bool {
-		secret, err = kubeClient.
-			CoreV1().
-			Secrets(env.SupervisorNamespace).
-			Get(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.GetOptions{})
-		return err == nil
-	}, time.Second*10, time.Millisecond*500)
-	require.NoError(t, err)
-
-	// Upon deleting the OPC, the secret is deleted (we test this behavior in our uninstall tests).
-}

test/integration/supervisor_secrets_test.go

@@ -0,0 +1,166 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package integration
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/square/go-jose.v2"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1"
+	"go.pinniped.dev/test/library"
+)
+
+func TestSupervisorSecrets(t *testing.T) {
+	env := library.IntegrationEnv(t)
+	kubeClient := library.NewClientset(t)
+	supervisorClient := library.NewSupervisorClientset(t)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	// Create our OP under test.
+	op := library.CreateTestOIDCProvider(ctx, t, "", "", "")
+
+	tests := []struct {
+		name        string
+		secretName  func(op *configv1alpha1.OIDCProvider) string
+		ensureValid func(t *testing.T, secret *corev1.Secret)
+	}{
+		{
+			name: "csrf cookie signing key",
+			secretName: func(op *configv1alpha1.OIDCProvider) string {
+				return env.SupervisorAppName + "-key"
+			},
+			ensureValid: ensureValidSymmetricKey,
+		},
+		{
+			name: "jwks",
+			secretName: func(op *configv1alpha1.OIDCProvider) string {
+				return op.Status.JWKSSecret.Name
+			},
+			ensureValid: ensureValidJWKS,
+		},
+		{
+			name: "hmac signing secret",
+			secretName: func(op *configv1alpha1.OIDCProvider) string {
+				return "pinniped-oidc-provider-hmac-key-" + string(op.UID)
+			},
+			ensureValid: ensureValidSymmetricKey,
+		},
+		{
+			name: "state signature secret",
+			secretName: func(op *configv1alpha1.OIDCProvider) string {
+				return "pinniped-oidc-provider-upstream-state-signature-key-" + string(op.UID)
+			},
+			ensureValid: ensureValidSymmetricKey,
+		},
+		{
+			name: "state encryption secret",
+			secretName: func(op *configv1alpha1.OIDCProvider) string {
+				return "pinniped-oidc-provider-upstream-state-encryption-key-" + string(op.UID)
+			},
+			ensureValid: ensureValidSymmetricKey,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			// Ensure a secret is created with the OP's JWKS.
+			var updatedOP *configv1alpha1.OIDCProvider
+			var err error
+			assert.Eventually(t, func() bool {
+				updatedOP, err = supervisorClient.
+					ConfigV1alpha1().
+					OIDCProviders(env.SupervisorNamespace).
+					Get(ctx, op.Name, metav1.GetOptions{})
+				return err == nil && test.secretName(updatedOP) != ""
+			}, time.Second*10, time.Millisecond*500)
+			require.NoError(t, err)
+			require.NotEmpty(t, test.secretName(updatedOP))
+
+			// Ensure the secret actually exists.
+			secret, err := kubeClient.
+				CoreV1().
+				Secrets(env.SupervisorNamespace).
+				Get(ctx, test.secretName(updatedOP), metav1.GetOptions{})
+			require.NoError(t, err)
+
+			// Ensure that the secret was labelled.
+			for k, v := range env.SupervisorCustomLabels {
+				require.Equalf(t, v, secret.Labels[k], "expected secret to have label `%s: %s`", k, v)
+			}
+			require.Equal(t, env.SupervisorAppName, secret.Labels["app"])
+
+			// Ensure that the secret is valid.
+			test.ensureValid(t, secret)
+
+			// Ensure upon deleting the secret, it is eventually brought back.
+			err = kubeClient.
+				CoreV1().
+				Secrets(env.SupervisorNamespace).
+				Delete(ctx, test.secretName(updatedOP), metav1.DeleteOptions{})
+			require.NoError(t, err)
+			assert.Eventually(t, func() bool {
+				secret, err = kubeClient.
+					CoreV1().
+					Secrets(env.SupervisorNamespace).
+					Get(ctx, test.secretName(updatedOP), metav1.GetOptions{})
+				return err == nil
+			}, time.Second*10, time.Millisecond*500)
+			require.NoError(t, err)
+
+			// Ensure that the new secret is valid.
+			test.ensureValid(t, secret)
+		})
+	}
+
+	// Upon deleting the OP, the secret is deleted (we test this behavior in our uninstall tests).
+}
+
+func ensureValidJWKS(t *testing.T, secret *corev1.Secret) {
+	t.Helper()
+
+	// Ensure the secret has an active key.
+	jwkData, ok := secret.Data["activeJWK"]
+	require.True(t, ok, "secret is missing active jwk")
+
+	// Ensure the secret's active key is valid.
+	var activeJWK jose.JSONWebKey
+	require.NoError(t, json.Unmarshal(jwkData, &activeJWK))
+	require.True(t, activeJWK.Valid(), "active jwk is invalid")
+	require.False(t, activeJWK.IsPublic(), "active jwk is public")
+
+	// Ensure the secret has a JWKS.
+	jwksData, ok := secret.Data["jwks"]
+	require.True(t, ok, "secret is missing jwks")
+
+	// Ensure the secret's JWKS is valid, public, and contains the singing key.
+	var jwks jose.JSONWebKeySet
+	require.NoError(t, json.Unmarshal(jwksData, &jwks))
+	foundActiveJWK := false
+	for _, jwk := range jwks.Keys {
+		require.Truef(t, jwk.Valid(), "jwk %s is invalid", jwk.KeyID)
+		require.Truef(t, jwk.IsPublic(), "jwk %s is not public", jwk.KeyID)
+		if jwk.KeyID == activeJWK.KeyID {
+			foundActiveJWK = true
+		}
+	}
+	require.True(t, foundActiveJWK, "could not find active JWK in JWKS: %s", jwks)
+}
+
+func ensureValidSymmetricKey(t *testing.T, secret *corev1.Secret) {
+	t.Helper()
+	require.Equal(t, corev1.SecretType("secrets.pinniped.dev/symmetric"), secret.Type)
+	key, ok := secret.Data["key"]
+	require.Truef(t, ok, "secret data does not contain 'key': %s", secret.Data)
+	require.Equal(t, 32, len(key))
+}