Set secret names on OIDCProvider status field

We believe this API is more forwards compatible with future secrets management
use cases. The implementation is a cry for help, but I was trying to follow the
previously established pattern of encapsulating the secret generation
functionality to a single group of packages.

This commit makes a breaking change to the current OIDCProvider API, but that
OIDCProvider API was added after the latest release, so it is technically still
in development until we release, and therefore we can continue to thrash on it.

I also took this opportunity to make some things private that didn't need to be
public.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This commit is contained in:
Andrew Keesler
2020-12-15 09:13:01 -05:00
parent 60d4a7beac
commit 82ae98d9d0
28 changed files with 724 additions and 183 deletions
@@ -27,11 +27,11 @@ func generateSymmetricKey() ([]byte, error) {
}
func isValid(secret *corev1.Secret) bool {
if secret.Type != SymmetricSecretType {
if secret.Type != symmetricSecretType {
return false
}
data, ok := secret.Data[SymmetricSecretDataKey]
data, ok := secret.Data[symmetricSecretDataKey]
if !ok {
return false
}
@@ -49,7 +49,7 @@ func secretDataFunc() (map[string][]byte, error) {
}
return map[string][]byte{
SymmetricSecretDataKey: symmetricKey,
symmetricSecretDataKey: symmetricKey,
}, nil
}
@@ -73,7 +73,7 @@ func generateSecret(namespace, name string, labels map[string]string, secretData
},
Labels: labels,
},
Type: SymmetricSecretType,
Type: symmetricSecretType,
Data: secretData,
}, nil
}
@@ -6,6 +6,7 @@ package generator
import (
"context"
"fmt"
"reflect"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -16,6 +17,7 @@ import (
"k8s.io/klog/v2"
configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1"
pinnipedclientset "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned"
configinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions/config/v1alpha1"
pinnipedcontroller "go.pinniped.dev/internal/controller"
"go.pinniped.dev/internal/controllerlib"
@@ -25,6 +27,7 @@ import (
type oidcProviderSecretsController struct {
secretHelper SecretHelper
kubeClient kubernetes.Interface
pinnipedClient pinnipedclientset.Interface
opcInformer configinformers.OIDCProviderInformer
secretInformer corev1informers.SecretInformer
}
@@ -35,6 +38,7 @@ type oidcProviderSecretsController struct {
func NewOIDCProviderSecretsController(
secretHelper SecretHelper,
kubeClient kubernetes.Interface,
pinnipedClient pinnipedclientset.Interface,
secretInformer corev1informers.SecretInformer,
opcInformer configinformers.OIDCProviderInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc,
@@ -45,6 +49,7 @@ func NewOIDCProviderSecretsController(
Syncer: &oidcProviderSecretsController{
secretHelper: secretHelper,
kubeClient: kubeClient,
pinnipedClient: pinnipedClient,
secretInformer: secretInformer,
opcInformer: opcInformer,
},
@@ -116,7 +121,13 @@ func (c *oidcProviderSecretsController) Sync(ctx controllerlib.Context) error {
"secret",
klog.KObj(existingSecret),
)
c.secretHelper.Notify(op, existingSecret)
op = c.secretHelper.ObserveActiveSecretAndUpdateParentOIDCProvider(op, existingSecret)
if err := c.updateOIDCProvider(ctx.Context, op); err != nil {
return fmt.Errorf("failed to update oidcprovider: %w", err)
}
plog.Debug("updated oidcprovider", "oidcprovider", klog.KObj(op), "secret", klog.KObj(newSecret))
return nil
}
@@ -127,7 +138,11 @@ func (c *oidcProviderSecretsController) Sync(ctx controllerlib.Context) error {
}
plog.Debug("created/updated secret", "oidcprovider", klog.KObj(op), "secret", klog.KObj(newSecret))
c.secretHelper.Notify(op, newSecret)
op = c.secretHelper.ObserveActiveSecretAndUpdateParentOIDCProvider(op, newSecret)
if err := c.updateOIDCProvider(ctx.Context, op); err != nil {
return fmt.Errorf("failed to update oidcprovider: %w", err)
}
plog.Debug("updated oidcprovider", "oidcprovider", klog.KObj(op), "secret", klog.KObj(newSecret))
return nil
}
@@ -195,3 +210,24 @@ func (c *oidcProviderSecretsController) createOrUpdateSecret(
return err
})
}
func (c *oidcProviderSecretsController) updateOIDCProvider(
ctx context.Context,
newOP *configv1alpha1.OIDCProvider,
) error {
opcClient := c.pinnipedClient.ConfigV1alpha1().OIDCProviders(newOP.Namespace)
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
oldOP, err := opcClient.Get(ctx, newOP.Name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get oidcprovider %s/%s: %w", newOP.Namespace, newOP.Name, err)
}
if reflect.DeepEqual(newOP.Status.Secrets, oldOP.Status.Secrets) {
return nil
}
oldOP.Status.Secrets = newOP.Status.Secrets
_, err = opcClient.Update(ctx, oldOP, metav1.UpdateOptions{})
return err
})
}
@@ -158,6 +158,7 @@ func TestOIDCProviderControllerFilterSecret(t *testing.T) {
_ = NewOIDCProviderSecretsController(
secretHelper,
nil, // kubeClient, not needed
nil, // pinnipedClient, not needed
secretInformer,
opcInformer,
withInformer.WithInformer,
@@ -216,6 +217,7 @@ func TestNewOIDCProviderSecretsControllerFilterOPC(t *testing.T) {
_ = NewOIDCProviderSecretsController(
secretHelper,
nil, // kubeClient, not needed
nil, // pinnipedClient, not needed
secretInformer,
opcInformer,
withInformer.WithInformer,
@@ -291,6 +293,9 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
},
}
goodOPWithStatus := goodOP.DeepCopy()
goodOPWithStatus.Status.Secrets.TokenSigningKey.Name = goodSecret.Name
invalidSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
@@ -309,26 +314,24 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
},
}
once := sync.Once{}
tests := []struct {
name string
storage func(**configv1alpha1.OIDCProvider, **corev1.Secret)
client func(*kubernetesfake.Clientset)
client func(*pinnipedfake.Clientset, *kubernetesfake.Clientset)
secretHelper func(*mocksecrethelper.MockSecretHelper)
wantSecretActions []kubetesting.Action
wantOPActions []kubetesting.Action
wantSecretActions []kubetesting.Action
wantError string
}{
{
name: "OIDCProvider exists and secret does not exist",
name: "OIDCProvider does not exist and secret does not exist",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
*op = nil
*s = nil
},
},
{
name: "OIDCProvider exists and secret exists",
name: "OIDCProvider does not exist and secret exists",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
*op = nil
},
@@ -340,21 +343,17 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
},
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus)
},
wantOPActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus),
},
wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewCreateAction(secretGVR, namespace, goodSecret),
},
},
{
name: "OIDCProvider exists and valid secret exists",
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(true)
secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1)
},
},
{
name: "OIDCProvider exists and invalid secret exists",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
@@ -363,7 +362,11 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false)
secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus)
},
wantOPActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus),
},
wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
@@ -386,19 +389,23 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(otherSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(false)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(true)
secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus)
},
wantOPActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus),
},
wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
},
},
{
name: "OIDCProvider exists and invalid secret exists and get fails",
name: "OIDCProvider exists and invalid secret exists and getting secret fails",
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(false)
},
client: func(c *kubernetesfake.Clientset) {
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
c.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some get error")
})
@@ -409,14 +416,14 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
wantError: fmt.Sprintf("failed to create or update secret: failed to get secret %s/%s: some get error", namespace, goodSecret.Name),
},
{
name: "OIDCProvider exists and secret does not exist and create fails",
name: "OIDCProvider exists and secret does not exist and creating secret fails",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
*s = nil
},
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
},
client: func(c *kubernetesfake.Clientset) {
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
c.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some create error")
})
@@ -428,12 +435,12 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
wantError: fmt.Sprintf("failed to create or update secret: failed to create secret %s/%s: some create error", namespace, goodSecret.Name),
},
{
name: "OIDCProvider exists and invalid secret exists and update fails",
name: "OIDCProvider exists and invalid secret exists and updating secret fails",
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(2).Return(false)
},
client: func(c *kubernetesfake.Clientset) {
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
c.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some update error")
})
@@ -445,22 +452,27 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
wantError: "failed to create or update secret: some update error",
},
{
name: "OIDCProvider exists and invalid secret exists and update fails due to conflict",
name: "OIDCProvider exists and invalid secret exists and updating secret fails due to conflict",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
*s = invalidSecret.DeepCopy()
},
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(3).Return(false)
secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus)
},
client: func(c *kubernetesfake.Clientset) {
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
once := sync.Once{}
c.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
var err error
once.Do(func() { err = k8serrors.NewConflict(secretGVR.GroupResource(), namespace, errors.New("some error")) })
return true, nil, err
})
},
wantOPActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus),
},
wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
@@ -468,6 +480,59 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
},
},
{
name: "OIDCProvider exists and invalid secret exists and getting OIDCProvider fails",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
*s = invalidSecret.DeepCopy()
},
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus)
},
client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) {
c.PrependReactor("get", "oidcproviders", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some get error")
})
},
wantOPActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
},
wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
},
wantError: fmt.Sprintf("failed to update oidcprovider: failed to get oidcprovider %s/%s: some get error", goodOPWithStatus.Namespace, goodOPWithStatus.Name),
},
{
name: "OIDCProvider exists and invalid secret exists and updating OIDCProvider fails due to conflict",
storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) {
*s = invalidSecret.DeepCopy()
},
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus)
},
client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) {
once := sync.Once{}
c.PrependReactor("update", "oidcproviders", func(_ kubetesting.Action) (bool, runtime.Object, error) {
var err error
once.Do(func() { err = k8serrors.NewConflict(secretGVR.GroupResource(), namespace, errors.New("some error")) })
return true, nil, err
})
},
wantOPActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus),
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus),
},
wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
},
},
}
for _, test := range tests {
test := test
@@ -477,6 +542,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*3)
defer cancel()
pinnipedAPIClient := pinnipedfake.NewSimpleClientset()
pinnipedInformerClient := pinnipedfake.NewSimpleClientset()
kubeAPIClient := kubernetesfake.NewSimpleClientset()
@@ -488,6 +554,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
test.storage(&op, &secret)
}
if op != nil {
require.NoError(t, pinnipedAPIClient.Tracker().Add(op))
require.NoError(t, pinnipedInformerClient.Tracker().Add(op))
}
if secret != nil {
@@ -496,7 +563,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
}
if test.client != nil {
test.client(kubeAPIClient)
test.client(pinnipedAPIClient, kubeAPIClient)
}
kubeInformers := kubeinformers.NewSharedInformerFactory(
@@ -519,6 +586,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
c := NewOIDCProviderSecretsController(
secretHelper,
kubeAPIClient,
pinnipedAPIClient,
kubeInformers.Core().V1().Secrets(),
pinnipedInformers.Config().V1alpha1().OIDCProviders(),
controllerlib.WithInformer,
@@ -542,6 +610,10 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) {
}
require.NoError(t, err)
if test.wantOPActions == nil {
test.wantOPActions = []kubetesting.Action{}
}
require.Equal(t, test.wantOPActions, pinnipedAPIClient.Actions())
if test.wantSecretActions == nil {
test.wantSecretActions = []kubetesting.Action{}
}
@@ -12,6 +12,7 @@ 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
@@ -22,14 +23,14 @@ type SecretHelper interface {
NamePrefix() string
Generate(*configv1alpha1.OIDCProvider) (*corev1.Secret, error)
IsValid(*configv1alpha1.OIDCProvider, *corev1.Secret) bool
Notify(*configv1alpha1.OIDCProvider, *corev1.Secret)
ObserveActiveSecretAndUpdateParentOIDCProvider(*configv1alpha1.OIDCProvider, *corev1.Secret) *configv1alpha1.OIDCProvider
}
const (
// 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.
SymmetricSecretDataKey = "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.
symmetricSecretDataKey = "key"
// symmetricKeySize is the default length, in bytes, of generated keys. It is set to 32 since this
// seems like reasonable entropy for our keys, and a 32-byte key will allow for AES-256
@@ -37,18 +38,30 @@ const (
symmetricKeySize = 32
)
// SecretUsage describes how a cryptographic secret is going to be used. It is currently used to
// indicate to a SecretHelper which status field to set on the parent OIDCProvider for a Secret.
type SecretUsage int
const (
SecretUsageTokenSigningKey SecretUsage = iota
SecretUsageStateSigningKey
SecretUsageStateEncryptionKey
)
// New returns a SecretHelper that has been parameterized with common symmetric secret generation
// knobs.
func NewSymmetricSecretHelper(
namePrefix string,
labels map[string]string,
rand io.Reader,
secretUsage SecretUsage,
updateCacheFunc func(cacheKey string, cacheValue []byte),
) SecretHelper {
return &symmetricSecretHelper{
namePrefix: namePrefix,
labels: labels,
rand: rand,
secretUsage: secretUsage,
updateCacheFunc: updateCacheFunc,
}
}
@@ -57,6 +70,7 @@ type symmetricSecretHelper struct {
namePrefix string
labels map[string]string
rand io.Reader
secretUsage SecretUsage
updateCacheFunc func(cacheKey string, cacheValue []byte)
}
@@ -82,9 +96,9 @@ func (s *symmetricSecretHelper) Generate(parent *configv1alpha1.OIDCProvider) (*
}),
},
},
Type: SymmetricSecretType,
Type: symmetricSecretType,
Data: map[string][]byte{
SymmetricSecretDataKey: key,
symmetricSecretDataKey: key,
},
}, nil
}
@@ -95,11 +109,11 @@ func (s *symmetricSecretHelper) IsValid(parent *configv1alpha1.OIDCProvider, sec
return false
}
if secret.Type != SymmetricSecretType {
if secret.Type != symmetricSecretType {
return false
}
key, ok := secret.Data[SymmetricSecretDataKey]
key, ok := secret.Data[symmetricSecretDataKey]
if !ok {
return false
}
@@ -110,12 +124,28 @@ func (s *symmetricSecretHelper) IsValid(parent *configv1alpha1.OIDCProvider, sec
return true
}
// Notify implements SecretHelper.Notify().
func (s *symmetricSecretHelper) Notify(op *configv1alpha1.OIDCProvider, secret *corev1.Secret) {
// ObserveActiveSecretAndUpdateParentOIDCProvider implements SecretHelper.ObserveActiveSecretAndUpdateParentOIDCProvider().
func (s *symmetricSecretHelper) ObserveActiveSecretAndUpdateParentOIDCProvider(
op *configv1alpha1.OIDCProvider,
secret *corev1.Secret,
) *configv1alpha1.OIDCProvider {
var cacheKey string
if op != nil {
cacheKey = op.Spec.Issuer
}
s.updateCacheFunc(cacheKey, secret.Data[SymmetricSecretDataKey])
s.updateCacheFunc(cacheKey, secret.Data[symmetricSecretDataKey])
switch s.secretUsage {
case SecretUsageTokenSigningKey:
op.Status.Secrets.TokenSigningKey.Name = secret.Name
case SecretUsageStateSigningKey:
op.Status.Secrets.StateSigningKey.Name = secret.Name
case SecretUsageStateEncryptionKey:
op.Status.Secrets.StateEncryptionKey.Name = secret.Name
default:
plog.Warning("unknown secret usage enum value: %d", s.secretUsage)
}
return op
}
@@ -17,57 +17,98 @@ import (
const keyWith32Bytes = "0123456789abcdef0123456789abcdef"
func TestSymmetricSecretHHelper(t *testing.T) {
labels := map[string]string{
"some-label-key-1": "some-label-value-1",
"some-label-key-2": "some-label-value-2",
}
randSource := strings.NewReader(keyWith32Bytes)
// var notifyParent *configv1alpha1.OIDCProvider
// var notifyChild *corev1.Secret
var oidcProviderIssuerValue string
var symmetricKeyValue []byte
h := NewSymmetricSecretHelper("some-name-prefix-", labels, randSource, func(oidcProviderIssuer string, symmetricKey []byte) {
require.True(t, oidcProviderIssuer == "" && symmetricKeyValue == nil, "expected notify func not to have been called yet")
oidcProviderIssuerValue = oidcProviderIssuer
symmetricKeyValue = symmetricKey
})
func TestSymmetricSecretHelper(t *testing.T) {
t.Parallel()
parent := &configv1alpha1.OIDCProvider{
ObjectMeta: metav1.ObjectMeta{
UID: "some-uid",
Namespace: "some-namespace",
},
}
child, err := h.Generate(parent)
require.NoError(t, err)
require.Equal(t, child, &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "some-name-prefix-some-uid",
Namespace: "some-namespace",
Labels: labels,
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(parent, schema.GroupVersionKind{
Group: configv1alpha1.SchemeGroupVersion.Group,
Version: configv1alpha1.SchemeGroupVersion.Version,
Kind: "OIDCProvider",
}),
tests := []struct {
name string
secretUsage SecretUsage
wantSetOIDCProviderField func(*configv1alpha1.OIDCProvider) string
}{
{
name: "token signing key",
secretUsage: SecretUsageTokenSigningKey,
wantSetOIDCProviderField: func(op *configv1alpha1.OIDCProvider) string {
return op.Status.Secrets.TokenSigningKey.Name
},
},
Type: "secrets.pinniped.dev/symmetric",
Data: map[string][]byte{
"key": []byte(keyWith32Bytes),
{
name: "state signing key",
secretUsage: SecretUsageStateSigningKey,
wantSetOIDCProviderField: func(op *configv1alpha1.OIDCProvider) string {
return op.Status.Secrets.StateSigningKey.Name
},
},
})
{
name: "state encryption key",
secretUsage: SecretUsageStateEncryptionKey,
wantSetOIDCProviderField: func(op *configv1alpha1.OIDCProvider) string {
return op.Status.Secrets.StateEncryptionKey.Name
},
},
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()
require.True(t, h.IsValid(parent, child))
labels := map[string]string{
"some-label-key-1": "some-label-value-1",
"some-label-key-2": "some-label-value-2",
}
randSource := strings.NewReader(keyWith32Bytes)
var oidcProviderIssuerValue string
var symmetricKeyValue []byte
h := NewSymmetricSecretHelper(
"some-name-prefix-",
labels,
randSource,
test.secretUsage,
func(oidcProviderIssuer string, symmetricKey []byte) {
require.True(t, oidcProviderIssuer == "" && symmetricKeyValue == nil, "expected notify func not to have been called yet")
oidcProviderIssuerValue = oidcProviderIssuer
symmetricKeyValue = symmetricKey
},
)
h.Notify(parent, child)
require.Equal(t, parent.Spec.Issuer, oidcProviderIssuerValue)
require.Equal(t, child.Data[SymmetricSecretDataKey], symmetricKeyValue)
parent := &configv1alpha1.OIDCProvider{
ObjectMeta: metav1.ObjectMeta{
UID: "some-uid",
Namespace: "some-namespace",
},
}
child, err := h.Generate(parent)
require.NoError(t, err)
require.Equal(t, child, &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "some-name-prefix-some-uid",
Namespace: "some-namespace",
Labels: labels,
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(parent, schema.GroupVersionKind{
Group: configv1alpha1.SchemeGroupVersion.Group,
Version: configv1alpha1.SchemeGroupVersion.Version,
Kind: "OIDCProvider",
}),
},
},
Type: "secrets.pinniped.dev/symmetric",
Data: map[string][]byte{
"key": []byte(keyWith32Bytes),
},
})
require.True(t, h.IsValid(parent, child))
h.ObserveActiveSecretAndUpdateParentOIDCProvider(parent, child)
require.Equal(t, parent.Spec.Issuer, oidcProviderIssuerValue)
require.Equal(t, child.Name, test.wantSetOIDCProviderField(parent))
require.Equal(t, child.Data["key"], symmetricKeyValue)
})
}
}
func TestSymmetricSecretHHelperIsValid(t *testing.T) {
func TestSymmetricSecretHelperIsValid(t *testing.T) {
tests := []struct {
name string
child func(*corev1.Secret)
@@ -117,7 +158,7 @@ func TestSymmetricSecretHHelperIsValid(t *testing.T) {
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
h := NewSymmetricSecretHelper("none of these args matter", nil, nil, nil)
h := NewSymmetricSecretHelper("none of these args matter", nil, nil, SecretUsageTokenSigningKey, nil)
parent := &configv1alpha1.OIDCProvider{
ObjectMeta: metav1.ObjectMeta{
@@ -79,7 +79,7 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error {
secretNeedsUpdate := isNotFound || !isValid(secret)
if !secretNeedsUpdate {
plog.Debug("secret is up to date", "secret", klog.KObj(secret))
c.setCacheFunc(secret.Data[SymmetricSecretDataKey])
c.setCacheFunc(secret.Data[symmetricSecretDataKey])
return nil
}
@@ -97,7 +97,7 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error {
return fmt.Errorf("failed to create/update secret %s/%s: %w", newSecret.Namespace, newSecret.Name, err)
}
c.setCacheFunc(newSecret.Data[SymmetricSecretDataKey])
c.setCacheFunc(newSecret.Data[symmetricSecretDataKey])
return nil
}
@@ -194,8 +194,8 @@ func TestSupervisorSecretsControllerInitialEvent(t *testing.T) {
initialEventOption.WithInitialEvent,
)
require.Equal(t, &controllerlib.Key{
owner.Namespace,
owner.Name + "-key",
Namespace: owner.Namespace,
Name: owner.Name + "-key",
}, initialEventOption.GetInitialEventKey())
}
@@ -76,7 +76,7 @@ func (c *jwksObserverController) Sync(ctx controllerlib.Context) error {
issuerToActiveJWKMap := map[string]*jose.JSONWebKey{}
for _, provider := range allProviders {
secretRef := provider.Status.JWKSSecret
secretRef := provider.Status.Secrets.JWKS
jwksSecret, err := c.secretInformer.Lister().Secrets(ns).Get(secretRef.Name)
if err != nil {
plog.Debug("jwksObserverController Sync could not find JWKS secret", "namespace", ns, "secretName", secretRef.Name)
@@ -202,7 +202,7 @@ func TestJWKSObserverControllerSync(t *testing.T) {
Namespace: installedInNamespace,
},
Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://no-secret-issuer1.com"},
Status: v1alpha1.OIDCProviderStatus{}, // no JWKSSecret field
Status: v1alpha1.OIDCProviderStatus{}, // no Secrets.JWKS field
}
oidcProviderWithoutSecret2 := &v1alpha1.OIDCProvider{
ObjectMeta: metav1.ObjectMeta{
@@ -219,7 +219,9 @@ func TestJWKSObserverControllerSync(t *testing.T) {
},
Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-secret-issuer.com"},
Status: v1alpha1.OIDCProviderStatus{
JWKSSecret: corev1.LocalObjectReference{Name: "bad-secret-name"},
Secrets: v1alpha1.OIDCProviderSecrets{
JWKS: corev1.LocalObjectReference{Name: "bad-secret-name"},
},
},
}
oidcProviderWithBadJWKSSecret := &v1alpha1.OIDCProvider{
@@ -229,7 +231,9 @@ func TestJWKSObserverControllerSync(t *testing.T) {
},
Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-jwks-secret-issuer.com"},
Status: v1alpha1.OIDCProviderStatus{
JWKSSecret: corev1.LocalObjectReference{Name: "bad-jwks-secret-name"},
Secrets: v1alpha1.OIDCProviderSecrets{
JWKS: corev1.LocalObjectReference{Name: "bad-jwks-secret-name"},
},
},
}
oidcProviderWithBadActiveJWKSecret := &v1alpha1.OIDCProvider{
@@ -239,7 +243,9 @@ func TestJWKSObserverControllerSync(t *testing.T) {
},
Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-active-jwk-secret-issuer.com"},
Status: v1alpha1.OIDCProviderStatus{
JWKSSecret: corev1.LocalObjectReference{Name: "bad-active-jwk-secret-name"},
Secrets: v1alpha1.OIDCProviderSecrets{
JWKS: corev1.LocalObjectReference{Name: "bad-active-jwk-secret-name"},
},
},
}
oidcProviderWithGoodSecret1 := &v1alpha1.OIDCProvider{
@@ -249,7 +255,9 @@ func TestJWKSObserverControllerSync(t *testing.T) {
},
Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://issuer-with-good-secret1.com"},
Status: v1alpha1.OIDCProviderStatus{
JWKSSecret: corev1.LocalObjectReference{Name: "good-jwks-secret-name1"},
Secrets: v1alpha1.OIDCProviderSecrets{
JWKS: corev1.LocalObjectReference{Name: "good-jwks-secret-name1"},
},
},
}
oidcProviderWithGoodSecret2 := &v1alpha1.OIDCProvider{
@@ -259,7 +267,9 @@ func TestJWKSObserverControllerSync(t *testing.T) {
},
Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://issuer-with-good-secret2.com"},
Status: v1alpha1.OIDCProviderStatus{
JWKSSecret: corev1.LocalObjectReference{Name: "good-jwks-secret-name2"},
Secrets: v1alpha1.OIDCProviderSecrets{
JWKS: corev1.LocalObjectReference{Name: "good-jwks-secret-name2"},
},
},
}
expectedJWK1 = string(readJWKJSON(t, "testdata/public-jwk.json"))
@@ -169,7 +169,7 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error {
// Ensure that the OPC points to the secret.
newOPC := opc.DeepCopy()
newOPC.Status.JWKSSecret.Name = secret.Name
newOPC.Status.Secrets.JWKS.Name = secret.Name
if err := c.updateOPC(ctx.Context, newOPC); err != nil {
return fmt.Errorf("cannot update opc: %w", err)
}
@@ -179,13 +179,13 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error {
}
func (c *jwksWriterController) secretNeedsUpdate(opc *configv1alpha1.OIDCProvider) (bool, error) {
if opc.Status.JWKSSecret.Name == "" {
if opc.Status.Secrets.JWKS.Name == "" {
// If the OPC says it doesn't have a secret associated with it, then let's create one.
return true, nil
}
// This OPC says it has a secret associated with it. Let's try to get it from the cache.
secret, err := c.secretInformer.Lister().Secrets(opc.Namespace).Get(opc.Status.JWKSSecret.Name)
secret, err := c.secretInformer.Lister().Secrets(opc.Namespace).Get(opc.Status.Secrets.JWKS.Name)
notFound := k8serrors.IsNotFound(err)
if err != nil && !notFound {
return false, fmt.Errorf("cannot get secret: %w", err)
@@ -301,12 +301,12 @@ func (c *jwksWriterController) updateOPC(
return fmt.Errorf("cannot get opc: %w", err)
}
if newOPC.Status.JWKSSecret.Name == oldOPC.Status.JWKSSecret.Name {
if newOPC.Status.Secrets.JWKS.Name == oldOPC.Status.Secrets.JWKS.Name {
// If the existing OPC is up to date, we don't need to update it.
return nil
}
oldOPC.Status.JWKSSecret.Name = newOPC.Status.JWKSSecret.Name
oldOPC.Status.Secrets.JWKS.Name = newOPC.Status.Secrets.JWKS.Name
_, err = opcClient.Update(ctx, oldOPC, metav1.UpdateOptions{})
return err
})
@@ -253,7 +253,7 @@ func TestJWKSWriterControllerSync(t *testing.T) {
},
}
goodOPCWithStatus := goodOPC.DeepCopy()
goodOPCWithStatus.Status.JWKSSecret.Name = goodOPCWithStatus.Name + "-jwks"
goodOPCWithStatus.Status.Secrets.JWKS.Name = goodOPCWithStatus.Name + "-jwks"
secretGVR := schema.GroupVersionResource{
Group: corev1.SchemeGroupVersion.Group,
@@ -264,7 +264,7 @@ func TestJWKSWriterControllerSync(t *testing.T) {
newSecret := func(activeJWKPath, jwksPath string) *corev1.Secret {
s := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: goodOPCWithStatus.Status.JWKSSecret.Name,
Name: goodOPCWithStatus.Status.Secrets.JWKS.Name,
Namespace: namespace,
Labels: map[string]string{
"myLabelKey1": "myLabelValue1",