From 187bd9060c12526e90a28184b0480a77c1c7cc05 Mon Sep 17 00:00:00 2001 From: aram price Date: Thu, 17 Dec 2020 17:07:38 -0800 Subject: [PATCH] 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 {