All FederationDomain Secrets have distinct Types

Signed-off-by: Ryan Richard <richardry@vmware.com>

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.
+	// FederationDomainTokenSigningKeyType for the Secret storing the FederationDomain token signing key.
-	symmetricSecretType corev1.SecretType = "secrets.pinniped.dev/symmetric"
+	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
+	s.updateCacheFunc(federationDomain.Spec.Issuer, secret.Data[symmetricSecretDataKey])
-	if federationDomain != nil {
-		cacheKey = federationDomain.Spec.Issuer
-	}
-
-	s.updateCacheFunc(cacheKey, 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))
+	}
+}

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",
+			name:           "token signing key",
-			secretUsage: SecretUsageTokenSigningKey,
+			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",
+			name:           "state signing key",
-			secretUsage: SecretUsageStateSigningKey,
+			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",
+			name:           "state encryption key",
-			secretUsage: SecretUsageStateEncryptionKey,
+			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
+		name        string
-		child  func(*corev1.Secret)
+		secretUsage SecretUsage
-		parent func(*configv1alpha1.FederationDomain)
+		child       func(*corev1.Secret)
-		want   bool
+		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",
+			name:        "happy path",
-			want: true,
+			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),
 				},

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 {