diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index d9508290..b125e966 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -15,9 +15,8 @@ import ( "strings" "time" - "go.pinniped.dev/internal/secret" - appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/clock" kubeinformers "k8s.io/client-go/informers" @@ -29,6 +28,7 @@ import ( "k8s.io/klog/v2" "k8s.io/klog/v2/klogr" + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned" pinnipedinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/config/supervisor" @@ -42,6 +42,7 @@ import ( "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider/manager" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/secret" ) const ( @@ -173,6 +174,9 @@ func startControllers( secretCache.SetTokenHMACKey(federationDomainIssuer, symmetricKey) }, ), + func(fd *configv1alpha1.FederationDomain) *corev1.LocalObjectReference { + return &fd.Status.Secrets.TokenSigningKey + }, kubeClient, pinnipedClient, secretInformer, @@ -193,6 +197,9 @@ func startControllers( secretCache.SetStateEncoderHashKey(federationDomainIssuer, symmetricKey) }, ), + func(fd *configv1alpha1.FederationDomain) *corev1.LocalObjectReference { + return &fd.Status.Secrets.StateSigningKey + }, kubeClient, pinnipedClient, secretInformer, @@ -213,6 +220,9 @@ func startControllers( secretCache.SetStateEncoderBlockKey(federationDomainIssuer, symmetricKey) }, ), + func(fd *configv1alpha1.FederationDomain) *corev1.LocalObjectReference { + return &fd.Status.Secrets.StateEncryptionKey + }, kubeClient, pinnipedClient, secretInformer, diff --git a/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go b/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go index 227f55d4..146e4541 100644 --- a/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go +++ b/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go @@ -26,6 +26,7 @@ import ( type federationDomainSecretsController struct { secretHelper SecretHelper + secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference kubeClient kubernetes.Interface pinnipedClient pinnipedclientset.Interface opcInformer configinformers.FederationDomainInformer @@ -37,6 +38,7 @@ type federationDomainSecretsController struct { // provides the parent/child mapping logic. func NewFederationDomainSecretsController( secretHelper SecretHelper, + secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference, kubeClient kubernetes.Interface, pinnipedClient pinnipedclientset.Interface, secretInformer corev1informers.SecretInformer, @@ -48,6 +50,7 @@ func NewFederationDomainSecretsController( Name: fmt.Sprintf("%s%s", secretHelper.NamePrefix(), "controller"), Syncer: &federationDomainSecretsController{ secretHelper: secretHelper, + secretRefFunc: secretRefFunc, kubeClient: kubeClient, pinnipedClient: pinnipedClient, secretInformer: secretInformer, @@ -102,6 +105,7 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro return nil } + op = op.DeepCopy() newSecret, err := c.secretHelper.Generate(op) if err != nil { return fmt.Errorf("failed to generate secret: %w", err) @@ -221,11 +225,13 @@ func (c *federationDomainSecretsController) updateFederationDomain( return fmt.Errorf("failed to get federationdomain %s/%s: %w", newOP.Namespace, newOP.Name, err) } - if reflect.DeepEqual(newOP.Status.Secrets, oldOP.Status.Secrets) { + oldOPSecretRef := c.secretRefFunc(oldOP) + newOPSecretRef := c.secretRefFunc(newOP) + if reflect.DeepEqual(oldOPSecretRef, newOPSecretRef) { return nil } - oldOP.Status.Secrets = newOP.Status.Secrets + *oldOPSecretRef = *newOPSecretRef _, err = opcClient.Update(ctx, oldOP, metav1.UpdateOptions{}) return err }) diff --git a/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go b/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go index d4e90863..e56125a1 100644 --- a/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go +++ b/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go @@ -157,6 +157,7 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) { withInformer := testutil.NewObservableWithInformerOption() _ = NewFederationDomainSecretsController( secretHelper, + nil, // secretRefFunc, not needed nil, // kubeClient, not needed nil, // pinnipedClient, not needed secretInformer, @@ -216,6 +217,7 @@ func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) { withInformer := testutil.NewObservableWithInformerOption() _ = NewFederationDomainSecretsController( secretHelper, + nil, // secretRefFunc, not needed nil, // kubeClient, not needed nil, // pinnipedClient, not needed secretInformer, @@ -293,8 +295,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, } - goodOPWithStatus := goodOP.DeepCopy() - goodOPWithStatus.Status.Secrets.TokenSigningKey.Name = goodSecret.Name + goodOPWithTokenSigningKey := goodOP.DeepCopy() + goodOPWithTokenSigningKey.Status.Secrets.TokenSigningKey.Name = goodSecret.Name + + goodOPWithJWKS := goodOP.DeepCopy() + goodOPWithJWKS.Status.Secrets.JWKS.Name = "some-jwks-key" + + goodOPWithJWKSAndTokenSigningKey := goodOPWithJWKS.DeepCopy() + goodOPWithJWKSAndTokenSigningKey.Status.Secrets.TokenSigningKey = goodOPWithTokenSigningKey.Status.Secrets.TokenSigningKey invalidSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -343,11 +351,56 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) - secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), + }, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), + }, + }, + { + name: "FederationDomain exists and secret does not exist and upon updating FederationDomain we learn a new status field has been set", + storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + *s = nil + }, + secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { + secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + }, + client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { + c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, goodOPWithJWKS, nil + }) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithJWKSAndTokenSigningKey), + }, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), + }, + }, + { + name: "FederationDomain exists and secret does not exist and upon updating FederationDomain we learn all status fields have been set", + storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + *s = nil + }, + secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { + secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + }, + client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { + c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, goodOPWithJWKSAndTokenSigningKey, nil + }) }, wantOPActions: []kubetesting.Action{ kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -362,11 +415,11 @@ func TestFederationDomainSecretsControllerSync(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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) }, wantOPActions: []kubetesting.Action{ kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -389,11 +442,11 @@ func TestFederationDomainSecretsControllerSync(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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) }, wantOPActions: []kubetesting.Action{ kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -459,7 +512,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { 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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) }, client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { once := sync.Once{} @@ -471,7 +524,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, wantOPActions: []kubetesting.Action{ kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -488,7 +541,7 @@ func TestFederationDomainSecretsControllerSync(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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) }, client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { @@ -502,7 +555,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantError: fmt.Sprintf("failed to update federationdomain: failed to get federationdomain %s/%s: some get error", goodOPWithStatus.Namespace, goodOPWithStatus.Name), + wantError: fmt.Sprintf("failed to update federationdomain: failed to get federationdomain %s/%s: some get error", goodOPWithTokenSigningKey.Namespace, goodOPWithTokenSigningKey.Name), }, { name: "FederationDomain exists and invalid secret exists and updating FederationDomain fails due to conflict", @@ -512,7 +565,7 @@ func TestFederationDomainSecretsControllerSync(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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) }, client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { once := sync.Once{} @@ -524,9 +577,9 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, wantOPActions: []kubetesting.Action{ kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -585,6 +638,9 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { c := NewFederationDomainSecretsController( secretHelper, + func(fd *configv1alpha1.FederationDomain) *corev1.LocalObjectReference { + return &fd.Status.Secrets.TokenSigningKey + }, kubeAPIClient, pinnipedAPIClient, kubeInformers.Core().V1().Secrets(),