Merge pull request #301 from vmware-tanzu/typed-secrets

Put a Type on all of the Secrets that we create in the supervisor
This commit is contained in:
Ryan Richard 2020-12-17 17:42:20 -08:00 committed by GitHub
commit 6c210b67d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 211 additions and 151 deletions

View File

@ -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
}

View File

@ -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
}

View File

@ -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
@ -27,8 +26,18 @@ type SecretHelper interface {
}
const (
// symmetricSecretType is corev1.Secret.Type of all corev1.Secret's generated by this helper.
symmetricSecretType = "secrets.pinniped.dev/symmetric"
// SupervisorCSRFSigningKeySecretType for the Secret storing the CSRF signing key.
SupervisorCSRFSigningKeySecretType corev1.SecretType = "secrets.pinniped.dev/supervisor-csrf-signing-key"
// 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"
@ -96,7 +105,7 @@ func (s *symmetricSecretHelper) Generate(parent *configv1alpha1.FederationDomain
}),
},
},
Type: symmetricSecretType,
Type: s.secretType(),
Data: map[string][]byte{
symmetricSecretDataKey: key,
},
@ -109,7 +118,7 @@ func (s *symmetricSecretHelper) IsValid(parent *configv1alpha1.FederationDomain,
return false
}
if secret.Type != symmetricSecretType {
if secret.Type != s.secretType() {
return false
}
@ -129,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:
@ -144,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))
}
}

View File

@ -23,11 +23,13 @@ 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,
wantSecretType: "secrets.pinniped.dev/federation-domain-token-signing-key",
wantSetFederationDomainField: func(federationDomain *configv1alpha1.FederationDomain) string {
return federationDomain.Status.Secrets.TokenSigningKey.Name
},
@ -35,6 +37,7 @@ func TestSymmetricSecretHelper(t *testing.T) {
{
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
},
@ -42,6 +45,7 @@ func TestSymmetricSecretHelper(t *testing.T) {
{
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),
},
@ -111,12 +115,14 @@ func TestSymmetricSecretHelper(t *testing.T) {
func TestSymmetricSecretHelperIsValid(t *testing.T) {
tests := []struct {
name string
secretUsage SecretUsage
child func(*corev1.Secret)
parent func(*configv1alpha1.FederationDomain)
want bool
}{
{
name: "wrong type",
secretUsage: SecretUsageTokenSigningKey,
child: func(s *corev1.Secret) {
s.Type = "wrong"
},
@ -124,6 +130,7 @@ func TestSymmetricSecretHelperIsValid(t *testing.T) {
},
{
name: "empty type",
secretUsage: SecretUsageTokenSigningKey,
child: func(s *corev1.Secret) {
s.Type = ""
},
@ -131,20 +138,28 @@ func TestSymmetricSecretHelperIsValid(t *testing.T) {
},
{
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",
secretUsage: SecretUsageTokenSigningKey,
child: func(s *corev1.Secret) {
s.Type = FederationDomainTokenSigningKeyType
delete(s.Data, "key")
},
want: false,
},
{
name: "child not owned by parent",
secretUsage: SecretUsageTokenSigningKey,
child: func(s *corev1.Secret) {
s.Type = FederationDomainTokenSigningKeyType
},
parent: func(federationDomain *configv1alpha1.FederationDomain) {
federationDomain.UID = "wrong"
},
@ -152,13 +167,16 @@ func TestSymmetricSecretHelperIsValid(t *testing.T) {
},
{
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),
},

View File

@ -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
}

View File

@ -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,
},

View File

@ -40,6 +40,8 @@ const (
//
// Note! The value for this key will contain only public key material!
jwksKey = "jwks"
jwksSecretTypeValue corev1.SecretType = "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")

View File

@ -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},

View File

@ -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,7 +40,8 @@ const (
controllerName = "upstream-observer"
// Constants related to the client credentials Secret.
oidcClientSecretType = "secrets.pinniped.dev/oidc-client"
oidcClientSecretType corev1.SecretType = "secrets.pinniped.dev/oidc-client"
clientIDDataKey = "clientID"
clientSecretDataKey = "clientSecret"

View File

@ -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/federation-domain-token-signing-key"),
},
{
name: "state signature secret",
secretName: func(federationDomain *configv1alpha1.FederationDomain) string {
return federationDomain.Status.Secrets.StateSigningKey.Name
},
ensureValid: ensureValidSymmetricKey,
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: ensureValidSymmetricKey,
ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/federation-domain-state-encryption-key"),
},
}
for _, test := range tests {
@ -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, corev1.SecretType("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")
@ -157,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) {
func ensureValidSymmetricSecretOfTypeFunc(secretTypeValue string) func(*testing.T, *corev1.Secret) {
return func(t *testing.T, secret *corev1.Secret) {
t.Helper()
require.Equal(t, corev1.SecretType("secrets.pinniped.dev/symmetric"), secret.Type)
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))
}
}