From b27e3e1a8956e53d0720a7100c6cd7c40fd76303 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 17 Dec 2020 14:48:49 -0800 Subject: [PATCH 1/4] Put a Type on the Secrets that we create for FederationDomain JWKS Signed-off-by: Aram Price --- .../supervisorconfig/jwks_writer.go | 9 ++++++++ .../supervisorconfig/jwks_writer_test.go | 22 +++++++++++++++++++ test/integration/supervisor_secrets_test.go | 3 +++ 3 files changed, 34 insertions(+) diff --git a/internal/controller/supervisorconfig/jwks_writer.go b/internal/controller/supervisorconfig/jwks_writer.go index 559b1afa..5b77a93c 100644 --- a/internal/controller/supervisorconfig/jwks_writer.go +++ b/internal/controller/supervisorconfig/jwks_writer.go @@ -40,6 +40,8 @@ const ( // // Note! The value for this key will contain only public key material! jwksKey = "jwks" + + jwksSecretTypeValue = "secrets.pinniped.dev/federation-domain-jwks" ) const ( @@ -251,6 +253,7 @@ func (c *jwksWriterController) generateSecret(federationDomain *configv1alpha1.F activeJWKKey: jwkData, jwksKey: jwksData, }, + Type: jwksSecretTypeValue, } return &s, nil @@ -285,6 +288,7 @@ func (c *jwksWriterController) createOrUpdateSecret( } oldSecret.Data = newSecret.Data + oldSecret.Type = jwksSecretTypeValue _, err = secretClient.Update(ctx, oldSecret, metav1.UpdateOptions{}) return err }) @@ -322,6 +326,11 @@ func isFederationDomainControllee(obj metav1.Object) bool { // isValid returns whether the provided secret contains a valid active JWK and verification JWKS. func isValid(secret *corev1.Secret) bool { + if secret.Type != jwksSecretTypeValue { + plog.Debug("secret does not have the expected type", "expectedType", jwksSecretTypeValue, "actualType", secret.Type) + return false + } + jwkData, ok := secret.Data[activeJWKKey] if !ok { plog.Debug("secret does not contain active jwk") diff --git a/internal/controller/supervisorconfig/jwks_writer_test.go b/internal/controller/supervisorconfig/jwks_writer_test.go index 1c3d6565..d1947d98 100644 --- a/internal/controller/supervisorconfig/jwks_writer_test.go +++ b/internal/controller/supervisorconfig/jwks_writer_test.go @@ -281,6 +281,7 @@ func TestJWKSWriterControllerSync(t *testing.T) { }, }, }, + Type: "secrets.pinniped.dev/federation-domain-jwks", } s.Data = make(map[string][]byte) if activeJWKPath != "" { @@ -294,6 +295,9 @@ func TestJWKSWriterControllerSync(t *testing.T) { goodSecret := newSecret("testdata/good-jwk.json", "testdata/good-jwks.json") + secretWithWrongType := newSecret("testdata/good-jwk.json", "testdata/good-jwks.json") + secretWithWrongType.Type = "not-the-right-type" + tests := []struct { name string key controllerlib.Key @@ -407,6 +411,24 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, + { + name: "wrong type in secret", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, + }, + secrets: []*corev1.Secret{ + secretWithWrongType, + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + }, + }, { name: "invalid jwk JSON in secret", key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index a0387ec0..25ad71dc 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -129,6 +129,9 @@ func TestSupervisorSecrets(t *testing.T) { func ensureValidJWKS(t *testing.T, secret *corev1.Secret) { t.Helper() + // Ensure the secret has the right type. + require.Equal(t, "secrets.pinniped.dev/federation-domain-jwks", secret.Type) + // Ensure the secret has an active key. jwkData, ok := secret.Data["activeJWK"] require.True(t, ok, "secret is missing active jwk") From 50964c6677b1c3d5668dcb6ee8dc40ebfdf9f962 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 17 Dec 2020 15:30:26 -0800 Subject: [PATCH 2/4] Supervisor CSRF Secret has unique Type Signed-off-by: aram price --- .../generator/federation_domain_secrets.go | 12 ++ .../supervisorconfig/generator/generator.go | 104 ------------------ .../generator/secret_helper.go | 3 + .../generator/supervisor_secrets.go | 79 +++++++++++++ .../generator/supervisor_secrets_test.go | 4 +- test/integration/supervisor_secrets_test.go | 22 ++-- 6 files changed, 108 insertions(+), 116 deletions(-) delete mode 100644 internal/controller/supervisorconfig/generator/generator.go diff --git a/internal/controller/supervisorconfig/generator/federation_domain_secrets.go b/internal/controller/supervisorconfig/generator/federation_domain_secrets.go index e9e169d1..879e488a 100644 --- a/internal/controller/supervisorconfig/generator/federation_domain_secrets.go +++ b/internal/controller/supervisorconfig/generator/federation_domain_secrets.go @@ -24,6 +24,10 @@ import ( "go.pinniped.dev/internal/plog" ) +const ( + federationDomainKind = "FederationDomain" +) + type federationDomainSecretsController struct { secretHelper SecretHelper secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference @@ -236,3 +240,11 @@ func (c *federationDomainSecretsController) updateFederationDomain( return err }) } + +// isFederationDomainControllee returns whether the provided obj is controlled by an FederationDomain. +func isFederationDomainControllee(obj metav1.Object) bool { + controller := metav1.GetControllerOf(obj) + return controller != nil && + controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && + controller.Kind == federationDomainKind +} diff --git a/internal/controller/supervisorconfig/generator/generator.go b/internal/controller/supervisorconfig/generator/generator.go deleted file mode 100644 index d37ec235..00000000 --- a/internal/controller/supervisorconfig/generator/generator.go +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package generator - -import ( - "crypto/rand" - - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - - configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1" -) - -const ( - federationDomainKind = "FederationDomain" -) - -func generateSymmetricKey() ([]byte, error) { - b := make([]byte, symmetricKeySize) - if _, err := rand.Read(b); err != nil { - return nil, err - } - return b, nil -} - -func isValid(secret *corev1.Secret, labels map[string]string) bool { - if secret.Type != symmetricSecretType { - return false - } - - data, ok := secret.Data[symmetricSecretDataKey] - if !ok { - return false - } - if len(data) != symmetricKeySize { - return false - } - - for key, value := range labels { - if secret.Labels[key] != value { - return false - } - } - - return true -} - -func secretDataFunc() (map[string][]byte, error) { - symmetricKey, err := generateKey() - if err != nil { - return nil, err - } - - return map[string][]byte{ - symmetricSecretDataKey: symmetricKey, - }, nil -} - -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 - } - - deploymentGVK := schema.GroupVersionKind{ - Group: appsv1.SchemeGroupVersion.Group, - Version: appsv1.SchemeGroupVersion.Version, - Kind: "Deployment", - } - - blockOwnerDeletion := true - isController := false - - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: deploymentGVK.GroupVersion().String(), - Kind: deploymentGVK.Kind, - Name: owner.GetName(), - UID: owner.GetUID(), - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - }, - }, - Labels: labels, - }, - Type: symmetricSecretType, - Data: secretData, - }, nil -} - -// isFederationDomainControllee returns whether the provided obj is controlled by an FederationDomain. -func isFederationDomainControllee(obj metav1.Object) bool { - controller := metav1.GetControllerOf(obj) - return controller != nil && - controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && - controller.Kind == federationDomainKind -} diff --git a/internal/controller/supervisorconfig/generator/secret_helper.go b/internal/controller/supervisorconfig/generator/secret_helper.go index c0e66fb7..75bfd7ac 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper.go +++ b/internal/controller/supervisorconfig/generator/secret_helper.go @@ -27,6 +27,9 @@ type SecretHelper interface { } const ( + // SupervisorCSRFSigningKeySecretType is corev1.Secret.Type for the Supervisor's CSRF signing key Secret. + SupervisorCSRFSigningKeySecretType = "secrets.pinniped.dev/supervisor-csrf-signing-key" + // symmetricSecretType is corev1.Secret.Type of all corev1.Secret's generated by this helper. symmetricSecretType = "secrets.pinniped.dev/symmetric" // symmetricSecretDataKey is the corev1.Secret.Data key for the symmetric key value generated by this helper. diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets.go b/internal/controller/supervisorconfig/generator/supervisor_secrets.go index 9f5ca1c4..b7325648 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets.go +++ b/internal/controller/supervisorconfig/generator/supervisor_secrets.go @@ -6,12 +6,14 @@ package generator import ( "context" + "crypto/rand" "fmt" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/util/retry" @@ -143,3 +145,80 @@ func (c *supervisorSecretsController) updateSecret(ctx context.Context, newSecre return err }) } + +func generateSymmetricKey() ([]byte, error) { + b := make([]byte, symmetricKeySize) + if _, err := rand.Read(b); err != nil { + return nil, err + } + return b, nil +} + +func isValid(secret *corev1.Secret, labels map[string]string) bool { + if secret.Type != SupervisorCSRFSigningKeySecretType { + return false + } + + data, ok := secret.Data[symmetricSecretDataKey] + if !ok { + return false + } + if len(data) != symmetricKeySize { + return false + } + + for key, value := range labels { + if secret.Labels[key] != value { + return false + } + } + + return true +} + +func secretDataFunc() (map[string][]byte, error) { + symmetricKey, err := generateKey() + if err != nil { + return nil, err + } + + return map[string][]byte{ + symmetricSecretDataKey: symmetricKey, + }, nil +} + +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 + } + + deploymentGVK := schema.GroupVersionKind{ + Group: appsv1.SchemeGroupVersion.Group, + Version: appsv1.SchemeGroupVersion.Version, + Kind: "Deployment", + } + + blockOwnerDeletion := true + isController := false + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: deploymentGVK.GroupVersion().String(), + Kind: deploymentGVK.Kind, + Name: owner.GetName(), + UID: owner.GetUID(), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, + }, + }, + Labels: labels, + }, + Type: SupervisorCSRFSigningKeySecretType, + Data: secretData, + }, nil +} diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go index 2522fe7d..f0b75f7c 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go +++ b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go @@ -258,7 +258,7 @@ func TestSupervisorSecretsControllerSync(t *testing.T) { }, Labels: labels, }, - Type: "secrets.pinniped.dev/symmetric", + Type: "secrets.pinniped.dev/supervisor-csrf-signing-key", Data: map[string][]byte{ "key": generatedSymmetricKey, }, @@ -280,7 +280,7 @@ func TestSupervisorSecretsControllerSync(t *testing.T) { }, Labels: labels, }, - Type: "secrets.pinniped.dev/symmetric", + Type: "secrets.pinniped.dev/supervisor-csrf-signing-key", Data: map[string][]byte{ "key": otherGeneratedSymmetricKey, }, diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index 25ad71dc..2866b340 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -40,7 +40,7 @@ func TestSupervisorSecrets(t *testing.T) { secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return env.SupervisorAppName + "-key" }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/supervisor-csrf-signing-key"), }, { name: "jwks", @@ -54,21 +54,21 @@ func TestSupervisorSecrets(t *testing.T) { secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.TokenSigningKey.Name }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), }, { name: "state signature secret", secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateSigningKey.Name }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), }, { name: "state encryption secret", secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateEncryptionKey.Name }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), }, } for _, test := range tests { @@ -160,10 +160,12 @@ func ensureValidJWKS(t *testing.T, secret *corev1.Secret) { 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)) +func ensureValidSymmetricSecretOfTypeFunc(secretTypeValue string) func(*testing.T, *corev1.Secret) { + return func(t *testing.T, secret *corev1.Secret) { + t.Helper() + require.Equal(t, corev1.SecretType(secretTypeValue), 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)) + } } From 587cced768618c1daec43f878d5ca783cae4263f Mon Sep 17 00:00:00 2001 From: aram price Date: Thu, 17 Dec 2020 15:43:20 -0800 Subject: [PATCH 3/4] Add extra type info where SecretType is used --- .../supervisorconfig/generator/secret_helper.go | 8 ++++---- internal/controller/supervisorconfig/jwks_writer.go | 2 +- .../supervisorconfig/upstreamwatcher/upstreamwatcher.go | 8 +++++--- test/integration/supervisor_secrets_test.go | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/controller/supervisorconfig/generator/secret_helper.go b/internal/controller/supervisorconfig/generator/secret_helper.go index 75bfd7ac..ed554421 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper.go +++ b/internal/controller/supervisorconfig/generator/secret_helper.go @@ -27,11 +27,11 @@ type SecretHelper interface { } const ( - // SupervisorCSRFSigningKeySecretType is corev1.Secret.Type for the Supervisor's CSRF signing key Secret. - SupervisorCSRFSigningKeySecretType = "secrets.pinniped.dev/supervisor-csrf-signing-key" + // SupervisorCSRFSigningKeySecretType for the Secret storing the CSRF signing key. + SupervisorCSRFSigningKeySecretType corev1.SecretType = "secrets.pinniped.dev/supervisor-csrf-signing-key" - // symmetricSecretType is corev1.Secret.Type of all corev1.Secret's generated by this helper. - symmetricSecretType = "secrets.pinniped.dev/symmetric" + // symmetricSecretType for all corev1.Secret's generated by this helper. + symmetricSecretType corev1.SecretType = "secrets.pinniped.dev/symmetric" // symmetricSecretDataKey is the corev1.Secret.Data key for the symmetric key value generated by this helper. symmetricSecretDataKey = "key" diff --git a/internal/controller/supervisorconfig/jwks_writer.go b/internal/controller/supervisorconfig/jwks_writer.go index 5b77a93c..34f0b3aa 100644 --- a/internal/controller/supervisorconfig/jwks_writer.go +++ b/internal/controller/supervisorconfig/jwks_writer.go @@ -41,7 +41,7 @@ const ( // Note! The value for this key will contain only public key material! jwksKey = "jwks" - jwksSecretTypeValue = "secrets.pinniped.dev/federation-domain-jwks" + jwksSecretTypeValue corev1.SecretType = "secrets.pinniped.dev/federation-domain-jwks" ) const ( diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go index 19b79fb4..df90a5ab 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go @@ -18,6 +18,7 @@ import ( "github.com/coreos/go-oidc" "github.com/go-logr/logr" "golang.org/x/oauth2" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -39,9 +40,10 @@ const ( controllerName = "upstream-observer" // Constants related to the client credentials Secret. - oidcClientSecretType = "secrets.pinniped.dev/oidc-client" - clientIDDataKey = "clientID" - clientSecretDataKey = "clientSecret" + oidcClientSecretType corev1.SecretType = "secrets.pinniped.dev/oidc-client" + + clientIDDataKey = "clientID" + clientSecretDataKey = "clientSecret" // Constants related to the OIDC provider discovery cache. These do not affect the cache of JWKS. validatorCacheTTL = 15 * time.Minute diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index 2866b340..06763435 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -130,7 +130,7 @@ func ensureValidJWKS(t *testing.T, secret *corev1.Secret) { t.Helper() // Ensure the secret has the right type. - require.Equal(t, "secrets.pinniped.dev/federation-domain-jwks", secret.Type) + require.Equal(t, corev1.SecretType("secrets.pinniped.dev/federation-domain-jwks"), secret.Type) // Ensure the secret has an active key. jwkData, ok := secret.Data["activeJWK"] From 187bd9060c12526e90a28184b0480a77c1c7cc05 Mon Sep 17 00:00:00 2001 From: aram price Date: Thu, 17 Dec 2020 17:07:38 -0800 Subject: [PATCH 4/4] All FederationDomain Secrets have distinct Types Signed-off-by: Ryan Richard --- .../generator/secret_helper.go | 38 ++++++++---- .../generator/secret_helper_test.go | 58 ++++++++++++------- test/integration/supervisor_secrets_test.go | 6 +- 3 files changed, 67 insertions(+), 35 deletions(-) diff --git a/internal/controller/supervisorconfig/generator/secret_helper.go b/internal/controller/supervisorconfig/generator/secret_helper.go index ed554421..0113a929 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper.go +++ b/internal/controller/supervisorconfig/generator/secret_helper.go @@ -12,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1" - "go.pinniped.dev/internal/plog" ) // SecretHelper describes an object that can Generate() a Secret and determine whether a Secret @@ -30,8 +29,15 @@ const ( // SupervisorCSRFSigningKeySecretType for the Secret storing the CSRF signing key. SupervisorCSRFSigningKeySecretType corev1.SecretType = "secrets.pinniped.dev/supervisor-csrf-signing-key" - // symmetricSecretType for all corev1.Secret's generated by this helper. - symmetricSecretType corev1.SecretType = "secrets.pinniped.dev/symmetric" + // FederationDomainTokenSigningKeyType for the Secret storing the FederationDomain token signing key. + FederationDomainTokenSigningKeyType corev1.SecretType = "secrets.pinniped.dev/federation-domain-token-signing-key" + + // FederationDomainStateSigningKeyType for the Secret storing the FederationDomain state signing key. + FederationDomainStateSigningKeyType corev1.SecretType = "secrets.pinniped.dev/federation-domain-state-signing-key" + + // FederationDomainStateEncryptionKeyType for the Secret storing the FederationDomain state encryption key. + FederationDomainStateEncryptionKeyType corev1.SecretType = "secrets.pinniped.dev/federation-domain-state-encryption-key" + // symmetricSecretDataKey is the corev1.Secret.Data key for the symmetric key value generated by this helper. symmetricSecretDataKey = "key" @@ -99,7 +105,7 @@ func (s *symmetricSecretHelper) Generate(parent *configv1alpha1.FederationDomain }), }, }, - Type: symmetricSecretType, + Type: s.secretType(), Data: map[string][]byte{ symmetricSecretDataKey: key, }, @@ -112,7 +118,7 @@ func (s *symmetricSecretHelper) IsValid(parent *configv1alpha1.FederationDomain, return false } - if secret.Type != symmetricSecretType { + if secret.Type != s.secretType() { return false } @@ -132,12 +138,7 @@ func (s *symmetricSecretHelper) ObserveActiveSecretAndUpdateParentFederationDoma federationDomain *configv1alpha1.FederationDomain, secret *corev1.Secret, ) *configv1alpha1.FederationDomain { - var cacheKey string - if federationDomain != nil { - cacheKey = federationDomain.Spec.Issuer - } - - s.updateCacheFunc(cacheKey, secret.Data[symmetricSecretDataKey]) + s.updateCacheFunc(federationDomain.Spec.Issuer, secret.Data[symmetricSecretDataKey]) switch s.secretUsage { case SecretUsageTokenSigningKey: @@ -147,8 +148,21 @@ func (s *symmetricSecretHelper) ObserveActiveSecretAndUpdateParentFederationDoma case SecretUsageStateEncryptionKey: federationDomain.Status.Secrets.StateEncryptionKey.Name = secret.Name default: - plog.Warning("unknown secret usage enum value: %d", s.secretUsage) + panic(fmt.Sprintf("unknown secret usage enum value: %d", s.secretUsage)) } return federationDomain } + +func (s *symmetricSecretHelper) secretType() corev1.SecretType { + switch s.secretUsage { + case SecretUsageTokenSigningKey: + return FederationDomainTokenSigningKeyType + case SecretUsageStateSigningKey: + return FederationDomainStateSigningKeyType + case SecretUsageStateEncryptionKey: + return FederationDomainStateEncryptionKeyType + default: + panic(fmt.Sprintf("unknown secret usage enum value: %d", s.secretUsage)) + } +} diff --git a/internal/controller/supervisorconfig/generator/secret_helper_test.go b/internal/controller/supervisorconfig/generator/secret_helper_test.go index f25a515b..b83bed50 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper_test.go +++ b/internal/controller/supervisorconfig/generator/secret_helper_test.go @@ -23,25 +23,29 @@ func TestSymmetricSecretHelper(t *testing.T) { tests := []struct { name string secretUsage SecretUsage + wantSecretType corev1.SecretType wantSetFederationDomainField func(*configv1alpha1.FederationDomain) string }{ { - name: "token signing key", - secretUsage: SecretUsageTokenSigningKey, + name: "token signing key", + secretUsage: SecretUsageTokenSigningKey, + wantSecretType: "secrets.pinniped.dev/federation-domain-token-signing-key", wantSetFederationDomainField: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.TokenSigningKey.Name }, }, { - name: "state signing key", - secretUsage: SecretUsageStateSigningKey, + name: "state signing key", + secretUsage: SecretUsageStateSigningKey, + wantSecretType: "secrets.pinniped.dev/federation-domain-state-signing-key", wantSetFederationDomainField: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateSigningKey.Name }, }, { - name: "state encryption key", - secretUsage: SecretUsageStateEncryptionKey, + name: "state encryption key", + secretUsage: SecretUsageStateEncryptionKey, + wantSecretType: "secrets.pinniped.dev/federation-domain-state-encryption-key", wantSetFederationDomainField: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateEncryptionKey.Name }, @@ -92,7 +96,7 @@ func TestSymmetricSecretHelper(t *testing.T) { }), }, }, - Type: "secrets.pinniped.dev/symmetric", + Type: test.wantSecretType, Data: map[string][]byte{ "key": []byte(keyWith32Bytes), }, @@ -110,55 +114,69 @@ func TestSymmetricSecretHelper(t *testing.T) { func TestSymmetricSecretHelperIsValid(t *testing.T) { tests := []struct { - name string - child func(*corev1.Secret) - parent func(*configv1alpha1.FederationDomain) - want bool + name string + secretUsage SecretUsage + child func(*corev1.Secret) + parent func(*configv1alpha1.FederationDomain) + want bool }{ { - name: "wrong type", + name: "wrong type", + secretUsage: SecretUsageTokenSigningKey, child: func(s *corev1.Secret) { s.Type = "wrong" }, want: false, }, { - name: "empty type", + name: "empty type", + secretUsage: SecretUsageTokenSigningKey, child: func(s *corev1.Secret) { s.Type = "" }, want: false, }, { - name: "data key is too short", + name: "data key is too short", + secretUsage: SecretUsageTokenSigningKey, child: func(s *corev1.Secret) { + s.Type = FederationDomainTokenSigningKeyType s.Data["key"] = []byte("short") }, want: false, }, { - name: "data key does not exist", + name: "data key does not exist", + secretUsage: SecretUsageTokenSigningKey, child: func(s *corev1.Secret) { + s.Type = FederationDomainTokenSigningKeyType delete(s.Data, "key") }, want: false, }, { - name: "child not owned by parent", + name: "child not owned by parent", + secretUsage: SecretUsageTokenSigningKey, + child: func(s *corev1.Secret) { + s.Type = FederationDomainTokenSigningKeyType + }, parent: func(federationDomain *configv1alpha1.FederationDomain) { federationDomain.UID = "wrong" }, want: false, }, { - name: "happy path", - want: true, + name: "happy path", + secretUsage: SecretUsageTokenSigningKey, + child: func(s *corev1.Secret) { + s.Type = FederationDomainTokenSigningKeyType + }, want: true, }, } for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - h := NewSymmetricSecretHelper("none of these args matter", nil, nil, SecretUsageTokenSigningKey, nil) + h := NewSymmetricSecretHelper("none of these args matter", nil, nil, test.secretUsage, nil) parent := &configv1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{ @@ -179,7 +197,7 @@ func TestSymmetricSecretHelperIsValid(t *testing.T) { }), }, }, - Type: "secrets.pinniped.dev/symmetric", + Type: "invalid default", Data: map[string][]byte{ "key": []byte(keyWith32Bytes), }, diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index 06763435..c5c21a16 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -54,21 +54,21 @@ func TestSupervisorSecrets(t *testing.T) { secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.TokenSigningKey.Name }, - ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/federation-domain-token-signing-key"), }, { name: "state signature secret", secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateSigningKey.Name }, - ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/federation-domain-state-signing-key"), }, { name: "state encryption secret", secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateEncryptionKey.Name }, - ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/federation-domain-state-encryption-key"), }, } for _, test := range tests {