diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 26cfb32c..0f2a9665 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -152,6 +152,7 @@ func startControllers( WithController( generator.NewSupervisorSecretsController( supervisorDeployment, + cfg.Labels, kubeClient, secretInformer, func(secret []byte) { diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets.go b/internal/controller/supervisorconfig/generator/supervisor_secrets.go index cc3a9d33..f95379ce 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets.go +++ b/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, diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go index f782cb5c..e3ae8905 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go +++ b/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 }) diff --git a/test/integration/supervisor_keys_test.go b/test/integration/supervisor_keys_test.go deleted file mode 100644 index d59c713e..00000000 --- a/test/integration/supervisor_keys_test.go +++ /dev/null @@ -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). -} diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go new file mode 100644 index 00000000..9c3f8c0f --- /dev/null +++ b/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)) +}