From b96d49df0fe5e809c91b33f974fb11f793275bd2 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 17 Dec 2020 11:34:49 -0800 Subject: [PATCH] Rename all "op" and "opc" usages Signed-off-by: Aram Price --- cmd/pinniped-supervisor/main.go | 2 +- ...atcher.go => federation_domain_watcher.go} | 77 ++--- ...t.go => federation_domain_watcher_test.go} | 84 +++--- ...ecrets.go => federation_domain_secrets.go} | 102 +++---- ...t.go => federation_domain_secrets_test.go} | 248 +++++++-------- .../supervisorconfig/generator/generator.go | 4 +- .../supervisorconfig/jwks_writer.go | 120 ++++---- .../supervisorconfig/jwks_writer_test.go | 284 +++++++++--------- ...rovider.go => federation_domain_issuer.go} | 19 +- ...st.go => federation_domain_issuer_test.go} | 8 +- internal/oidc/provider/manager/manager.go | 6 +- .../oidc/provider/manager/manager_test.go | 8 +- test/integration/supervisor_discovery_test.go | 8 +- test/library/client.go | 20 +- 14 files changed, 496 insertions(+), 494 deletions(-) rename internal/controller/supervisorconfig/{oidcproviderconfig_watcher.go => federation_domain_watcher.go} (72%) rename internal/controller/supervisorconfig/{oidcproviderconfig_watcher_test.go => federation_domain_watcher_test.go} (91%) rename internal/controller/supervisorconfig/generator/{oidc_provider_secrets.go => federation_domain_secrets.go} (60%) rename internal/controller/supervisorconfig/generator/{oidc_provider_secrets_test.go => federation_domain_secrets_test.go} (65%) rename internal/oidc/provider/{oidcprovider.go => federation_domain_issuer.go} (66%) rename internal/oidc/provider/{oidcprovider_test.go => federation_domain_issuer_test.go} (89%) diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index b125e966..a351cdba 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -102,7 +102,7 @@ func startControllers( supervisorstorage.GarbageCollectorController( clock.RealClock{}, kubeClient, - kubeInformers.Core().V1().Secrets(), + secretInformer, controllerlib.WithInformer, ), singletonWorker, diff --git a/internal/controller/supervisorconfig/oidcproviderconfig_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go similarity index 72% rename from internal/controller/supervisorconfig/oidcproviderconfig_watcher.go rename to internal/controller/supervisorconfig/federation_domain_watcher.go index ec3d16b8..a6dd6e53 100644 --- a/internal/controller/supervisorconfig/oidcproviderconfig_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -29,14 +29,14 @@ import ( // If there are no longer any valid issuers, then it can be called with no arguments. // Implementations of this type should be thread-safe to support calls from multiple goroutines. type ProvidersSetter interface { - SetProviders(federationDomains ...*provider.FederationDomain) + SetProviders(federationDomains ...*provider.FederationDomainIssuer) } type federationDomainWatcherController struct { - providerSetter ProvidersSetter - clock clock.Clock - client pinnipedclientset.Interface - opcInformer configinformers.FederationDomainInformer + providerSetter ProvidersSetter + clock clock.Clock + client pinnipedclientset.Interface + federationDomainInformer configinformers.FederationDomainInformer } // NewFederationDomainWatcherController creates a controllerlib.Controller that watches @@ -45,21 +45,21 @@ func NewFederationDomainWatcherController( providerSetter ProvidersSetter, clock clock.Clock, client pinnipedclientset.Interface, - opcInformer configinformers.FederationDomainInformer, + federationDomainInformer configinformers.FederationDomainInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: "FederationDomainWatcherController", Syncer: &federationDomainWatcherController{ - providerSetter: providerSetter, - clock: clock, - client: client, - opcInformer: opcInformer, + providerSetter: providerSetter, + clock: clock, + client: client, + federationDomainInformer: federationDomainInformer, }, }, withInformer( - opcInformer, + federationDomainInformer, pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), @@ -68,7 +68,7 @@ func NewFederationDomainWatcherController( // Sync implements controllerlib.Syncer. func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) error { - all, err := c.opcInformer.Lister().List(labels.Everything()) + federationDomains, err := c.federationDomainInformer.Lister().List(labels.Everything()) if err != nil { return err } @@ -89,8 +89,8 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro uniqueSecretNamesPerIssuerAddress := make(map[string]map[string]bool) issuerURLToHostnameKey := lowercaseHostWithoutPort - for _, opc := range all { - issuerURL, err := url.Parse(opc.Spec.Issuer) + for _, federationDomain := range federationDomains { + issuerURL, err := url.Parse(federationDomain.Spec.Issuer) if err != nil { continue // Skip url parse errors because they will be validated again below. } @@ -102,26 +102,26 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro setOfSecretNames = make(map[string]bool) uniqueSecretNamesPerIssuerAddress[issuerURLToHostnameKey(issuerURL)] = setOfSecretNames } - if opc.Spec.TLS != nil { - setOfSecretNames[opc.Spec.TLS.SecretName] = true + if federationDomain.Spec.TLS != nil { + setOfSecretNames[federationDomain.Spec.TLS.SecretName] = true } } errs := multierror.New() - federationDomains := make([]*provider.FederationDomain, 0) - for _, opc := range all { - issuerURL, urlParseErr := url.Parse(opc.Spec.Issuer) + federationDomainIssuers := make([]*provider.FederationDomainIssuer, 0) + for _, federationDomain := range federationDomains { + issuerURL, urlParseErr := url.Parse(federationDomain.Spec.Issuer) // Skip url parse errors because they will be validated below. if urlParseErr == nil { if issuerCount := issuerCounts[issuerURLToIssuerKey(issuerURL)]; issuerCount > 1 { if err := c.updateStatus( ctx.Context, - opc.Namespace, - opc.Name, + federationDomain.Namespace, + federationDomain.Name, configv1alpha1.DuplicateFederationDomainStatusCondition, - "Duplicate issuer: "+opc.Spec.Issuer, + "Duplicate issuer: "+federationDomain.Spec.Issuer, ); err != nil { errs.Add(fmt.Errorf("could not update status: %w", err)) } @@ -133,8 +133,8 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro if urlParseErr == nil && len(uniqueSecretNamesPerIssuerAddress[issuerURLToHostnameKey(issuerURL)]) > 1 { if err := c.updateStatus( ctx.Context, - opc.Namespace, - opc.Name, + federationDomain.Namespace, + federationDomain.Name, configv1alpha1.SameIssuerHostMustUseSameSecretFederationDomainStatusCondition, "Issuers with the same DNS hostname (address not including port) must use the same secretName: "+issuerURLToHostnameKey(issuerURL), ); err != nil { @@ -143,12 +143,12 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro continue } - federationDomain, err := provider.NewFederationDomain(opc.Spec.Issuer) // This validates the Issuer URL. + federationDomainIssuer, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer) // This validates the Issuer URL. if err != nil { if err := c.updateStatus( ctx.Context, - opc.Namespace, - opc.Name, + federationDomain.Namespace, + federationDomain.Name, configv1alpha1.InvalidFederationDomainStatusCondition, "Invalid: "+err.Error(), ); err != nil { @@ -159,18 +159,19 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro if err := c.updateStatus( ctx.Context, - opc.Namespace, - opc.Name, + federationDomain.Namespace, + federationDomain.Name, configv1alpha1.SuccessFederationDomainStatusCondition, "Provider successfully created", ); err != nil { errs.Add(fmt.Errorf("could not update status: %w", err)) continue } - federationDomains = append(federationDomains, federationDomain) + + federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer) } - c.providerSetter.SetProviders(federationDomains...) + c.providerSetter.SetProviders(federationDomainIssuers...) return errs.ErrOrNil() } @@ -182,28 +183,28 @@ func (c *federationDomainWatcherController) updateStatus( message string, ) error { return retry.RetryOnConflict(retry.DefaultRetry, func() error { - opc, err := c.client.ConfigV1alpha1().FederationDomains(namespace).Get(ctx, name, metav1.GetOptions{}) + federationDomain, err := c.client.ConfigV1alpha1().FederationDomains(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { return fmt.Errorf("get failed: %w", err) } - if opc.Status.Status == status && opc.Status.Message == message { + if federationDomain.Status.Status == status && federationDomain.Status.Message == message { return nil } plog.Debug( "attempting status update", - "federationdomainconfig", + "federationdomain", klog.KRef(namespace, name), "status", status, "message", message, ) - opc.Status.Status = status - opc.Status.Message = message - opc.Status.LastUpdateTime = timePtr(metav1.NewTime(c.clock.Now())) - _, err = c.client.ConfigV1alpha1().FederationDomains(namespace).Update(ctx, opc, metav1.UpdateOptions{}) + federationDomain.Status.Status = status + federationDomain.Status.Message = message + federationDomain.Status.LastUpdateTime = timePtr(metav1.NewTime(c.clock.Now())) + _, err = c.client.ConfigV1alpha1().FederationDomains(namespace).Update(ctx, federationDomain, metav1.UpdateOptions{}) return err }) } diff --git a/internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go b/internal/controller/supervisorconfig/federation_domain_watcher_test.go similarity index 91% rename from internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go rename to internal/controller/supervisorconfig/federation_domain_watcher_test.go index 623d750a..7681a90c 100644 --- a/internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher_test.go @@ -40,15 +40,15 @@ func TestInformerFilters(t *testing.T) { it.Before(func() { r = require.New(t) observableWithInformerOption = testutil.NewObservableWithInformerOption() - opcInformer := pinnipedinformers.NewSharedInformerFactoryWithOptions(nil, 0).Config().V1alpha1().FederationDomains() + federationDomainInformer := pinnipedinformers.NewSharedInformerFactoryWithOptions(nil, 0).Config().V1alpha1().FederationDomains() _ = NewFederationDomainWatcherController( nil, nil, nil, - opcInformer, + federationDomainInformer, observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters ) - configMapInformerFilter = observableWithInformerOption.GetFilterForInformer(opcInformer) + configMapInformerFilter = observableWithInformerOption.GetFilterForInformer(federationDomainInformer) }) when("watching FederationDomain objects", func() { @@ -84,10 +84,10 @@ func TestInformerFilters(t *testing.T) { type fakeProvidersSetter struct { SetProvidersWasCalled bool - FederationDomainsReceived []*provider.FederationDomain + FederationDomainsReceived []*provider.FederationDomainIssuer } -func (f *fakeProvidersSetter) SetProviders(federationDomains ...*provider.FederationDomain) { +func (f *fakeProvidersSetter) SetProviders(federationDomains ...*provider.FederationDomainIssuer) { f.SetProvidersWasCalled = true f.FederationDomainsReceived = federationDomains } @@ -99,8 +99,8 @@ func TestSync(t *testing.T) { var r *require.Assertions var subject controllerlib.Controller - var opcInformerClient *pinnipedfake.Clientset - var opcInformers pinnipedinformers.SharedInformerFactory + var federationDomainInformerClient *pinnipedfake.Clientset + var federationDomainInformers pinnipedinformers.SharedInformerFactory var pinnipedAPIClient *pinnipedfake.Clientset var timeoutContext context.Context var timeoutContextCancel context.CancelFunc @@ -117,7 +117,7 @@ func TestSync(t *testing.T) { providersSetter, clock.NewFakeClock(frozenNow), pinnipedAPIClient, - opcInformers.Config().V1alpha1().FederationDomains(), + federationDomainInformers.Config().V1alpha1().FederationDomains(), controllerlib.WithInformer, ) @@ -132,7 +132,7 @@ func TestSync(t *testing.T) { } // Must start informers before calling TestRunSynchronously() - opcInformers.Start(timeoutContext.Done()) + federationDomainInformers.Start(timeoutContext.Done()) controllerlib.TestRunSynchronously(t, subject) } @@ -144,8 +144,8 @@ func TestSync(t *testing.T) { timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) - opcInformerClient = pinnipedfake.NewSimpleClientset() - opcInformers = pinnipedinformers.NewSharedInformerFactory(opcInformerClient, 0) + federationDomainInformerClient = pinnipedfake.NewSimpleClientset() + federationDomainInformers = pinnipedinformers.NewSharedInformerFactory(federationDomainInformerClient, 0) pinnipedAPIClient = pinnipedfake.NewSimpleClientset() federationDomainGVR = schema.GroupVersionResource{ @@ -171,14 +171,14 @@ func TestSync(t *testing.T) { Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer1.com"}, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain1)) - r.NoError(opcInformerClient.Tracker().Add(federationDomain1)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomain1)) federationDomain2 = &v1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer2.com"}, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain2)) - r.NoError(opcInformerClient.Tracker().Add(federationDomain2)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomain2)) }) it("calls the ProvidersSetter", func() { @@ -186,15 +186,15 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - provider1, err := provider.NewFederationDomain(federationDomain1.Spec.Issuer) + provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer) r.NoError(err) - provider2, err := provider.NewFederationDomain(federationDomain2.Spec.Issuer) + provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) r.ElementsMatch( - []*provider.FederationDomain{ + []*provider.FederationDomainIssuer{ provider1, provider2, }, @@ -247,7 +247,7 @@ func TestSync(t *testing.T) { federationDomain1.Status.LastUpdateTime = timePtr(metav1.NewTime(frozenNow)) r.NoError(pinnipedAPIClient.Tracker().Update(federationDomainGVR, federationDomain1, federationDomain1.Namespace)) - r.NoError(opcInformerClient.Tracker().Update(federationDomainGVR, federationDomain1, federationDomain1.Namespace)) + r.NoError(federationDomainInformerClient.Tracker().Update(federationDomainGVR, federationDomain1, federationDomain1.Namespace)) }) it("only updates the out-of-date FederationDomain", func() { @@ -284,15 +284,15 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - provider1, err := provider.NewFederationDomain(federationDomain1.Spec.Issuer) + provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer) r.NoError(err) - provider2, err := provider.NewFederationDomain(federationDomain2.Spec.Issuer) + provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) r.ElementsMatch( - []*provider.FederationDomain{ + []*provider.FederationDomainIssuer{ provider1, provider2, }, @@ -322,10 +322,10 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.EqualError(err, "1 error(s):\n- could not update status: some update error") - provider1, err := provider.NewFederationDomain(federationDomain1.Spec.Issuer) + provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer) r.NoError(err) - provider2, err := provider.NewFederationDomain(federationDomain2.Spec.Issuer) + provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) @@ -387,7 +387,7 @@ func TestSync(t *testing.T) { Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer.com"}, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain)) - r.NoError(opcInformerClient.Tracker().Add(federationDomain)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomain)) }) when("there is a conflict while updating an FederationDomain", func() { @@ -521,14 +521,14 @@ func TestSync(t *testing.T) { Spec: v1alpha1.FederationDomainSpec{Issuer: "https://valid-issuer.com"}, } r.NoError(pinnipedAPIClient.Tracker().Add(validFederationDomain)) - r.NoError(opcInformerClient.Tracker().Add(validFederationDomain)) + r.NoError(federationDomainInformerClient.Tracker().Add(validFederationDomain)) invalidFederationDomain = &v1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{Name: "invalid-config", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://invalid-issuer.com?some=query"}, } r.NoError(pinnipedAPIClient.Tracker().Add(invalidFederationDomain)) - r.NoError(opcInformerClient.Tracker().Add(invalidFederationDomain)) + r.NoError(federationDomainInformerClient.Tracker().Add(invalidFederationDomain)) }) it("calls the ProvidersSetter with the valid provider", func() { @@ -536,12 +536,12 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - validProvider, err := provider.NewFederationDomain(validFederationDomain.Spec.Issuer) + validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) r.Equal( - []*provider.FederationDomain{ + []*provider.FederationDomainIssuer{ validProvider, }, providersSetter.FederationDomainsReceived, @@ -593,8 +593,8 @@ func TestSync(t *testing.T) { "federationdomains", func(action coretesting.Action) (bool, runtime.Object, error) { updateAction := action.(coretesting.UpdateActionImpl) - opc := updateAction.Object.(*v1alpha1.FederationDomain) - if opc.Name == validFederationDomain.Name { + federationDomain := updateAction.Object.(*v1alpha1.FederationDomain) + if federationDomain.Name == validFederationDomain.Name { return true, nil, nil } @@ -608,12 +608,12 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.EqualError(err, "1 error(s):\n- could not update status: some update error") - validProvider, err := provider.NewFederationDomain(validFederationDomain.Spec.Issuer) + validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) r.Equal( - []*provider.FederationDomain{ + []*provider.FederationDomainIssuer{ validProvider, }, providersSetter.FederationDomainsReceived, @@ -675,20 +675,20 @@ func TestSync(t *testing.T) { Spec: v1alpha1.FederationDomainSpec{Issuer: "https://iSSueR-duPlicAte.cOm/a"}, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDuplicate1)) - r.NoError(opcInformerClient.Tracker().Add(federationDomainDuplicate1)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainDuplicate1)) federationDomainDuplicate2 = &v1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{Name: "duplicate2", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/a"}, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDuplicate2)) - r.NoError(opcInformerClient.Tracker().Add(federationDomainDuplicate2)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainDuplicate2)) federationDomain = &v1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{Name: "not-duplicate", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/A"}, // different path } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain)) - r.NoError(opcInformerClient.Tracker().Add(federationDomain)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomain)) }) it("calls the ProvidersSetter with the non-duplicate", func() { @@ -696,12 +696,12 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - nonDuplicateProvider, err := provider.NewFederationDomain(federationDomain.Spec.Issuer) + nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) r.Equal( - []*provider.FederationDomain{ + []*provider.FederationDomainIssuer{ nonDuplicateProvider, }, providersSetter.FederationDomainsReceived, @@ -824,7 +824,7 @@ func TestSync(t *testing.T) { }, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress1)) - r.NoError(opcInformerClient.Tracker().Add(federationDomainSameIssuerAddress1)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainSameIssuerAddress1)) federationDomainSameIssuerAddress2 = &v1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{Name: "provider2", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{ @@ -835,7 +835,7 @@ func TestSync(t *testing.T) { }, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress2)) - r.NoError(opcInformerClient.Tracker().Add(federationDomainSameIssuerAddress2)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainSameIssuerAddress2)) federationDomainDifferentIssuerAddress = &v1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressProvider", Namespace: namespace}, @@ -845,7 +845,7 @@ func TestSync(t *testing.T) { }, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDifferentIssuerAddress)) - r.NoError(opcInformerClient.Tracker().Add(federationDomainDifferentIssuerAddress)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainDifferentIssuerAddress)) // Also add one with a URL that cannot be parsed to make sure that the error handling // for the duplicate issuers and secret names are not confused by invalid URLs. @@ -860,7 +860,7 @@ func TestSync(t *testing.T) { }, } r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainWithInvalidIssuerURL)) - r.NoError(opcInformerClient.Tracker().Add(federationDomainWithInvalidIssuerURL)) + r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainWithInvalidIssuerURL)) }) it("calls the ProvidersSetter with the non-duplicate", func() { @@ -868,12 +868,12 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - nonDuplicateProvider, err := provider.NewFederationDomain(federationDomainDifferentIssuerAddress.Spec.Issuer) + nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomainDifferentIssuerAddress.Spec.Issuer) r.NoError(err) r.True(providersSetter.SetProvidersWasCalled) r.Equal( - []*provider.FederationDomain{ + []*provider.FederationDomainIssuer{ nonDuplicateProvider, }, providersSetter.FederationDomainsReceived, diff --git a/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go b/internal/controller/supervisorconfig/generator/federation_domain_secrets.go similarity index 60% rename from internal/controller/supervisorconfig/generator/oidc_provider_secrets.go rename to internal/controller/supervisorconfig/generator/federation_domain_secrets.go index 146e4541..e9e169d1 100644 --- a/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go +++ b/internal/controller/supervisorconfig/generator/federation_domain_secrets.go @@ -25,12 +25,12 @@ import ( ) type federationDomainSecretsController struct { - secretHelper SecretHelper - secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference - kubeClient kubernetes.Interface - pinnipedClient pinnipedclientset.Interface - opcInformer configinformers.FederationDomainInformer - secretInformer corev1informers.SecretInformer + secretHelper SecretHelper + secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference + kubeClient kubernetes.Interface + pinnipedClient pinnipedclientset.Interface + federationDomainInformer configinformers.FederationDomainInformer + secretInformer corev1informers.SecretInformer } // NewFederationDomainSecretsController returns a controllerlib.Controller that ensures a child Secret @@ -42,27 +42,27 @@ func NewFederationDomainSecretsController( kubeClient kubernetes.Interface, pinnipedClient pinnipedclientset.Interface, secretInformer corev1informers.SecretInformer, - opcInformer configinformers.FederationDomainInformer, + federationDomainInformer configinformers.FederationDomainInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: fmt.Sprintf("%s%s", secretHelper.NamePrefix(), "controller"), Syncer: &federationDomainSecretsController{ - secretHelper: secretHelper, - secretRefFunc: secretRefFunc, - kubeClient: kubeClient, - pinnipedClient: pinnipedClient, - secretInformer: secretInformer, - opcInformer: opcInformer, + secretHelper: secretHelper, + secretRefFunc: secretRefFunc, + kubeClient: kubeClient, + pinnipedClient: pinnipedClient, + secretInformer: secretInformer, + federationDomainInformer: federationDomainInformer, }, }, - // We want to be notified when a OPC's secret gets updated or deleted. When this happens, we - // should get notified via the corresponding OPC key. + // We want to be notified when a FederationDomain's secret gets updated or deleted. When this happens, we + // should get notified via the corresponding FederationDomain key. withInformer( secretInformer, - pinnipedcontroller.SimpleFilter(isOPControllee, func(obj metav1.Object) controllerlib.Key { - if isOPControllee(obj) { + pinnipedcontroller.SimpleFilter(isFederationDomainControllee, func(obj metav1.Object) controllerlib.Key { + if isFederationDomainControllee(obj) { controller := metav1.GetControllerOf(obj) return controllerlib.Key{ Name: controller.Name, @@ -73,9 +73,9 @@ func NewFederationDomainSecretsController( }), controllerlib.InformerOption{}, ), - // We want to be notified when anything happens to an OPC. + // We want to be notified when anything happens to an FederationDomain. withInformer( - opcInformer, + federationDomainInformer, pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct controllerlib.InformerOption{}, ), @@ -83,7 +83,7 @@ func NewFederationDomainSecretsController( } func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) error { - op, err := c.opcInformer.Lister().FederationDomains(ctx.Key.Namespace).Get(ctx.Key.Name) + federationDomain, err := c.federationDomainInformer.Lister().FederationDomains(ctx.Key.Namespace).Get(ctx.Key.Name) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return fmt.Errorf( @@ -95,8 +95,8 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro } if notFound { - // The corresponding secret to this OP should have been garbage collected since it should have - // had this OP as its owner. + // The corresponding secret to this FederationDomain should have been garbage collected since it should have + // had this FederationDomain as its owner. plog.Debug( "federationdomain deleted", "federationdomain", @@ -105,13 +105,13 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro return nil } - op = op.DeepCopy() - newSecret, err := c.secretHelper.Generate(op) + federationDomain = federationDomain.DeepCopy() + newSecret, err := c.secretHelper.Generate(federationDomain) if err != nil { return fmt.Errorf("failed to generate secret: %w", err) } - secretNeedsUpdate, existingSecret, err := c.secretNeedsUpdate(op, newSecret.Name) + secretNeedsUpdate, existingSecret, err := c.secretNeedsUpdate(federationDomain, newSecret.Name) if err != nil { return fmt.Errorf("failed to determine secret status: %w", err) } @@ -120,44 +120,44 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro plog.Debug( "secret is up to date", "federationdomain", - klog.KObj(op), + klog.KObj(federationDomain), "secret", klog.KObj(existingSecret), ) - op = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(op, existingSecret) - if err := c.updateFederationDomain(ctx.Context, op); err != nil { + federationDomain = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(federationDomain, existingSecret) + if err := c.updateFederationDomain(ctx.Context, federationDomain); err != nil { return fmt.Errorf("failed to update federationdomain: %w", err) } - plog.Debug("updated federationdomain", "federationdomain", klog.KObj(op), "secret", klog.KObj(newSecret)) + plog.Debug("updated federationdomain", "federationdomain", klog.KObj(federationDomain), "secret", klog.KObj(newSecret)) return nil } - // If the OP does not have a secret associated with it, that secret does not exist, or the secret + // If the FederationDomain does not have a secret associated with it, that secret does not exist, or the secret // is invalid, we will create a new secret. - if err := c.createOrUpdateSecret(ctx.Context, op, &newSecret); err != nil { + if err := c.createOrUpdateSecret(ctx.Context, federationDomain, &newSecret); err != nil { return fmt.Errorf("failed to create or update secret: %w", err) } - plog.Debug("created/updated secret", "federationdomain", klog.KObj(op), "secret", klog.KObj(newSecret)) + plog.Debug("created/updated secret", "federationdomain", klog.KObj(federationDomain), "secret", klog.KObj(newSecret)) - op = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(op, newSecret) - if err := c.updateFederationDomain(ctx.Context, op); err != nil { + federationDomain = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(federationDomain, newSecret) + if err := c.updateFederationDomain(ctx.Context, federationDomain); err != nil { return fmt.Errorf("failed to update federationdomain: %w", err) } - plog.Debug("updated federationdomain", "federationdomain", klog.KObj(op), "secret", klog.KObj(newSecret)) + plog.Debug("updated federationdomain", "federationdomain", klog.KObj(federationDomain), "secret", klog.KObj(newSecret)) return nil } -// secretNeedsUpdate returns whether or not the Secret, with name secretName, for FederationDomain op +// secretNeedsUpdate returns whether or not the Secret, with name secretName, for the federationDomain param // needs to be updated. It returns the existing secret as its second argument. func (c *federationDomainSecretsController) secretNeedsUpdate( - op *configv1alpha1.FederationDomain, + federationDomain *configv1alpha1.FederationDomain, secretName string, ) (bool, *corev1.Secret, error) { - // 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(op.Namespace).Get(secretName) + // This FederationDomain says it has a secret associated with it. Let's try to get it from the cache. + secret, err := c.secretInformer.Lister().Secrets(federationDomain.Namespace).Get(secretName) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return false, nil, fmt.Errorf("cannot get secret: %w", err) @@ -167,7 +167,7 @@ func (c *federationDomainSecretsController) secretNeedsUpdate( return true, nil, nil } - if !c.secretHelper.IsValid(op, secret) { + if !c.secretHelper.IsValid(federationDomain, secret) { // If this secret is invalid, we need to generate a new one. return true, secret, nil } @@ -177,7 +177,7 @@ func (c *federationDomainSecretsController) secretNeedsUpdate( func (c *federationDomainSecretsController) createOrUpdateSecret( ctx context.Context, - op *configv1alpha1.FederationDomain, + federationDomain *configv1alpha1.FederationDomain, newSecret **corev1.Secret, ) error { secretClient := c.kubeClient.CoreV1().Secrets((*newSecret).Namespace) @@ -198,7 +198,7 @@ func (c *federationDomainSecretsController) createOrUpdateSecret( } // New secret already exists, so ensure it is up to date. - if c.secretHelper.IsValid(op, oldSecret) { + if c.secretHelper.IsValid(federationDomain, oldSecret) { // If the secret already has valid a valid Secret, then we are good to go and we don't need an // update. *newSecret = oldSecret @@ -216,23 +216,23 @@ func (c *federationDomainSecretsController) createOrUpdateSecret( func (c *federationDomainSecretsController) updateFederationDomain( ctx context.Context, - newOP *configv1alpha1.FederationDomain, + newFederationDomain *configv1alpha1.FederationDomain, ) error { - opcClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newOP.Namespace) + federationDomainClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newFederationDomain.Namespace) return retry.RetryOnConflict(retry.DefaultRetry, func() error { - oldOP, err := opcClient.Get(ctx, newOP.Name, metav1.GetOptions{}) + oldFederationDomain, err := federationDomainClient.Get(ctx, newFederationDomain.Name, metav1.GetOptions{}) if err != nil { - return fmt.Errorf("failed to get federationdomain %s/%s: %w", newOP.Namespace, newOP.Name, err) + return fmt.Errorf("failed to get federationdomain %s/%s: %w", newFederationDomain.Namespace, newFederationDomain.Name, err) } - oldOPSecretRef := c.secretRefFunc(oldOP) - newOPSecretRef := c.secretRefFunc(newOP) - if reflect.DeepEqual(oldOPSecretRef, newOPSecretRef) { + oldFederationDomainSecretRef := c.secretRefFunc(oldFederationDomain) + newFederationDomainSecretRef := c.secretRefFunc(newFederationDomain) + if reflect.DeepEqual(oldFederationDomainSecretRef, newFederationDomainSecretRef) { return nil } - *oldOPSecretRef = *newOPSecretRef - _, err = opcClient.Update(ctx, oldOP, metav1.UpdateOptions{}) + *oldFederationDomainSecretRef = *newFederationDomainSecretRef + _, err = federationDomainClient.Update(ctx, oldFederationDomain, metav1.UpdateOptions{}) return err }) } diff --git a/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go b/internal/controller/supervisorconfig/generator/federation_domain_secrets_test.go similarity index 65% rename from internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go rename to internal/controller/supervisorconfig/generator/federation_domain_secrets_test.go index e56125a1..2d5c688c 100644 --- a/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go +++ b/internal/controller/supervisorconfig/generator/federation_domain_secrets_test.go @@ -150,7 +150,7 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) { kubernetesfake.NewSimpleClientset(), 0, ).Core().V1().Secrets() - opcInformer := pinnipedinformers.NewSharedInformerFactory( + federationDomainInformer := pinnipedinformers.NewSharedInformerFactory( pinnipedfake.NewSimpleClientset(), 0, ).Config().V1alpha1().FederationDomains() @@ -161,7 +161,7 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) { nil, // kubeClient, not needed nil, // pinnipedClient, not needed secretInformer, - opcInformer, + federationDomainInformer, withInformer.WithInformer, ) @@ -176,24 +176,24 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) { } } -func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) { +func TestNewFederationDomainSecretsControllerFilterFederationDomain(t *testing.T) { t.Parallel() tests := []struct { - name string - opc configv1alpha1.FederationDomain - wantAdd bool - wantUpdate bool - wantDelete bool - wantParent controllerlib.Key + name string + federationDomain configv1alpha1.FederationDomain + wantAdd bool + wantUpdate bool + wantDelete bool + wantParent controllerlib.Key }{ { - name: "anything goes", - opc: configv1alpha1.FederationDomain{}, - wantAdd: true, - wantUpdate: true, - wantDelete: true, - wantParent: controllerlib.Key{}, + name: "anything goes", + federationDomain: configv1alpha1.FederationDomain{}, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + wantParent: controllerlib.Key{}, }, } for _, test := range tests { @@ -210,7 +210,7 @@ func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) { kubernetesfake.NewSimpleClientset(), 0, ).Core().V1().Secrets() - opcInformer := pinnipedinformers.NewSharedInformerFactory( + federationDomainInformer := pinnipedinformers.NewSharedInformerFactory( pinnipedfake.NewSimpleClientset(), 0, ).Config().V1alpha1().FederationDomains() @@ -221,17 +221,17 @@ func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) { nil, // kubeClient, not needed nil, // pinnipedClient, not needed secretInformer, - opcInformer, + federationDomainInformer, withInformer.WithInformer, ) unrelated := configv1alpha1.FederationDomain{} - filter := withInformer.GetFilterForInformer(opcInformer) - require.Equal(t, test.wantAdd, filter.Add(&test.opc)) - require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.opc)) - require.Equal(t, test.wantUpdate, filter.Update(&test.opc, &unrelated)) - require.Equal(t, test.wantDelete, filter.Delete(&test.opc)) - require.Equal(t, test.wantParent, filter.Parent(&test.opc)) + filter := withInformer.GetFilterForInformer(federationDomainInformer) + require.Equal(t, test.wantAdd, filter.Add(&test.federationDomain)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.federationDomain)) + require.Equal(t, test.wantUpdate, filter.Update(&test.federationDomain, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(&test.federationDomain)) + require.Equal(t, test.wantParent, filter.Parent(&test.federationDomain)) }) } } @@ -242,14 +242,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { const ( namespace = "some-namespace" - opName = "op-name" - opUID = "op-uid" + federationDomainName = "federationDomain-name" + federationDomainUID = "federationDomain-uid" secretName = "secret-name" secretUID = "secret-uid" ) - opGVR := schema.GroupVersionResource{ + federationDomainGVR := schema.GroupVersionResource{ Group: configv1alpha1.SchemeGroupVersion.Group, Version: configv1alpha1.SchemeGroupVersion.Version, Resource: "federationdomains", @@ -261,11 +261,11 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { Resource: "secrets", } - goodOP := &configv1alpha1.FederationDomain{ + goodFederationDomain := &configv1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{ - Name: opName, + Name: federationDomainName, Namespace: namespace, - UID: opUID, + UID: federationDomainUID, }, } @@ -276,10 +276,10 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { UID: secretUID, OwnerReferences: []metav1.OwnerReference{ { - APIVersion: opGVR.GroupVersion().String(), + APIVersion: federationDomainGVR.GroupVersion().String(), Kind: "FederationDomain", - Name: opName, - UID: opUID, + Name: federationDomainName, + UID: federationDomainUID, BlockOwnerDeletion: boolPtr(true), Controller: boolPtr(true), }, @@ -295,14 +295,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, } - goodOPWithTokenSigningKey := goodOP.DeepCopy() - goodOPWithTokenSigningKey.Status.Secrets.TokenSigningKey.Name = goodSecret.Name + goodFederationDomainWithTokenSigningKey := goodFederationDomain.DeepCopy() + goodFederationDomainWithTokenSigningKey.Status.Secrets.TokenSigningKey.Name = goodSecret.Name - goodOPWithJWKS := goodOP.DeepCopy() - goodOPWithJWKS.Status.Secrets.JWKS.Name = "some-jwks-key" + goodFederationDomainWithJWKS := goodFederationDomain.DeepCopy() + goodFederationDomainWithJWKS.Status.Secrets.JWKS.Name = "some-jwks-key" - goodOPWithJWKSAndTokenSigningKey := goodOPWithJWKS.DeepCopy() - goodOPWithJWKSAndTokenSigningKey.Status.Secrets.TokenSigningKey = goodOPWithTokenSigningKey.Status.Secrets.TokenSigningKey + goodFederationDomainWithJWKSAndTokenSigningKey := goodFederationDomainWithJWKS.DeepCopy() + goodFederationDomainWithJWKSAndTokenSigningKey.Status.Secrets.TokenSigningKey = goodFederationDomainWithTokenSigningKey.Status.Secrets.TokenSigningKey invalidSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -311,10 +311,10 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { UID: secretUID, OwnerReferences: []metav1.OwnerReference{ { - APIVersion: opGVR.GroupVersion().String(), + APIVersion: federationDomainGVR.GroupVersion().String(), Kind: "FederationDomain", - Name: opName, - UID: opUID, + Name: federationDomainName, + UID: federationDomainUID, BlockOwnerDeletion: boolPtr(true), Controller: boolPtr(true), }, @@ -323,39 +323,39 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { } tests := []struct { - name string - storage func(**configv1alpha1.FederationDomain, **corev1.Secret) - client func(*pinnipedfake.Clientset, *kubernetesfake.Clientset) - secretHelper func(*mocksecrethelper.MockSecretHelper) - wantOPActions []kubetesting.Action - wantSecretActions []kubetesting.Action - wantError string + name string + storage func(**configv1alpha1.FederationDomain, **corev1.Secret) + client func(*pinnipedfake.Clientset, *kubernetesfake.Clientset) + secretHelper func(*mocksecrethelper.MockSecretHelper) + wantFederationDomainActions []kubetesting.Action + wantSecretActions []kubetesting.Action + wantError string }{ { name: "FederationDomain does not exist and secret does not exist", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { - *op = nil + storage: func(federationDomain **configv1alpha1.FederationDomain, s **corev1.Secret) { + *federationDomain = nil *s = nil }, }, { name: "FederationDomain does not exist and secret exists", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { - *op = nil + storage: func(federationDomain **configv1alpha1.FederationDomain, s **corev1.Secret) { + *federationDomain = nil }, }, { name: "FederationDomain exists and secret does not exist", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + storage: func(federationDomain **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) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -364,21 +364,21 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, { 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) { + storage: func(federationDomain **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) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { - return true, goodOPWithJWKS, nil + return true, goodFederationDomainWithJWKS, nil }) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithJWKSAndTokenSigningKey), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithJWKSAndTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -387,20 +387,20 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, { 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) { + storage: func(federationDomain **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) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { - return true, goodOPWithJWKSAndTokenSigningKey, nil + return true, goodFederationDomainWithJWKSAndTokenSigningKey, nil }) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -409,17 +409,17 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, { name: "FederationDomain exists and invalid secret exists", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + storage: func(federationDomain **configv1alpha1.FederationDomain, 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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(2).Return(false) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -429,7 +429,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { { name: "FederationDomain exists and generating a secret fails", secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { - secretHelper.EXPECT().Generate(goodOP).Times(1).Return(nil, errors.New("some generate error")) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(nil, errors.New("some generate error")) }, wantError: "failed to generate secret: some generate error", }, @@ -439,14 +439,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { otherSecret := goodSecret.DeepCopy() otherSecret.UID = "other-secret-uid" - 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(goodOPWithTokenSigningKey) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(otherSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(1).Return(false) + secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(1).Return(true) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -455,8 +455,8 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { { name: "FederationDomain 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) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(1).Return(false) }, client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { c.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { @@ -470,11 +470,11 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, { name: "FederationDomain exists and secret does not exist and creating secret fails", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + storage: func(federationDomain **configv1alpha1.FederationDomain, s **corev1.Secret) { *s = nil }, secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { - secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) }, client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { c.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { @@ -490,8 +490,8 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { { name: "FederationDomain 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) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(2).Return(false) }, client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { c.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { @@ -506,13 +506,13 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, { name: "FederationDomain exists and invalid secret exists and updating secret fails due to conflict", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + storage: func(federationDomain **configv1alpha1.FederationDomain, 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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(3).Return(false) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { once := sync.Once{} @@ -522,9 +522,9 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { return true, nil, err }) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -535,37 +535,37 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { }, { name: "FederationDomain exists and invalid secret exists and getting FederationDomain fails", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + storage: func(federationDomain **configv1alpha1.FederationDomain, 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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(2).Return(false) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("some get error") }) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, wantSecretActions: []kubetesting.Action{ 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", goodOPWithTokenSigningKey.Namespace, goodOPWithTokenSigningKey.Name), + wantError: fmt.Sprintf("failed to update federationdomain: failed to get federationdomain %s/%s: some get error", goodFederationDomainWithTokenSigningKey.Namespace, goodFederationDomainWithTokenSigningKey.Name), }, { name: "FederationDomain exists and invalid secret exists and updating FederationDomain fails due to conflict", - storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { + storage: func(federationDomain **configv1alpha1.FederationDomain, 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().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) + secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(2).Return(false) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey) }, client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { once := sync.Once{} @@ -575,11 +575,11 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { return true, nil, err }) }, - wantOPActions: []kubetesting.Action{ - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), - kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), - kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey), + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -601,14 +601,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { kubeAPIClient := kubernetesfake.NewSimpleClientset() kubeInformerClient := kubernetesfake.NewSimpleClientset() - op := goodOP.DeepCopy() + federationDomain := goodFederationDomain.DeepCopy() secret := goodSecret.DeepCopy() if test.storage != nil { - test.storage(&op, &secret) + test.storage(&federationDomain, &secret) } - if op != nil { - require.NoError(t, pinnipedAPIClient.Tracker().Add(op)) - require.NoError(t, pinnipedInformerClient.Tracker().Add(op)) + if federationDomain != nil { + require.NoError(t, pinnipedAPIClient.Tracker().Add(federationDomain)) + require.NoError(t, pinnipedInformerClient.Tracker().Add(federationDomain)) } if secret != nil { require.NoError(t, kubeAPIClient.Tracker().Add(secret)) @@ -657,7 +657,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { Context: ctx, Key: controllerlib.Key{ Namespace: namespace, - Name: opName, + Name: federationDomainName, }, }) if test.wantError != "" { @@ -666,10 +666,10 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) { } require.NoError(t, err) - if test.wantOPActions == nil { - test.wantOPActions = []kubetesting.Action{} + if test.wantFederationDomainActions == nil { + test.wantFederationDomainActions = []kubetesting.Action{} } - require.Equal(t, test.wantOPActions, pinnipedAPIClient.Actions()) + require.Equal(t, test.wantFederationDomainActions, pinnipedAPIClient.Actions()) if test.wantSecretActions == nil { test.wantSecretActions = []kubetesting.Action{} } diff --git a/internal/controller/supervisorconfig/generator/generator.go b/internal/controller/supervisorconfig/generator/generator.go index 0cbaeaf8..df4b73c5 100644 --- a/internal/controller/supervisorconfig/generator/generator.go +++ b/internal/controller/supervisorconfig/generator/generator.go @@ -95,8 +95,8 @@ func generateSecret(namespace, name string, labels map[string]string, secretData }, nil } -// isOPCControlle returns whether the provided obj is controlled by an OPC. -func isOPControllee(obj metav1.Object) bool { +// 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() && diff --git a/internal/controller/supervisorconfig/jwks_writer.go b/internal/controller/supervisorconfig/jwks_writer.go index 179f65e4..559b1afa 100644 --- a/internal/controller/supervisorconfig/jwks_writer.go +++ b/internal/controller/supervisorconfig/jwks_writer.go @@ -30,7 +30,7 @@ import ( "go.pinniped.dev/internal/plog" ) -// These constants are the keys in an OPC's Secret's Data map. +// These constants are the keys in a FederationDomain's Secret's Data map. const ( // activeJWKKey points to the current private key used for signing tokens. // @@ -43,7 +43,7 @@ const ( ) const ( - opcKind = "FederationDomain" + federationDomainKind = "FederationDomain" ) // generateKey is stubbed out for the purpose of testing. The default behavior is to generate an EC key. @@ -54,44 +54,44 @@ func generateECKey(r io.Reader) (interface{}, error) { return ecdsa.GenerateKey(elliptic.P256(), r) } -// jwkController holds the fields necessary for the JWKS controller to communicate with OPC's and +// jwkController holds the fields necessary for the JWKS controller to communicate with FederationDomains and // secrets, both via a cache and via the API. type jwksWriterController struct { - jwksSecretLabels map[string]string - pinnipedClient pinnipedclientset.Interface - kubeClient kubernetes.Interface - opcInformer configinformers.FederationDomainInformer - secretInformer corev1informers.SecretInformer + jwksSecretLabels map[string]string + pinnipedClient pinnipedclientset.Interface + kubeClient kubernetes.Interface + federationDomainInformer configinformers.FederationDomainInformer + secretInformer corev1informers.SecretInformer } -// NewJWKSWriterController returns a controllerlib.Controller that ensures an OPC has a corresponding +// NewJWKSWriterController returns a controllerlib.Controller that ensures a FederationDomain has a corresponding // Secret that contains a valid active JWK and JWKS. func NewJWKSWriterController( jwksSecretLabels map[string]string, kubeClient kubernetes.Interface, pinnipedClient pinnipedclientset.Interface, secretInformer corev1informers.SecretInformer, - opcInformer configinformers.FederationDomainInformer, + federationDomainInformer configinformers.FederationDomainInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: "JWKSController", Syncer: &jwksWriterController{ - jwksSecretLabels: jwksSecretLabels, - kubeClient: kubeClient, - pinnipedClient: pinnipedClient, - secretInformer: secretInformer, - opcInformer: opcInformer, + jwksSecretLabels: jwksSecretLabels, + kubeClient: kubeClient, + pinnipedClient: pinnipedClient, + secretInformer: secretInformer, + federationDomainInformer: federationDomainInformer, }, }, - // We want to be notified when a OPC's secret gets updated or deleted. When this happens, we - // should get notified via the corresponding OPC key. + // We want to be notified when a FederationDomain's secret gets updated or deleted. When this happens, we + // should get notified via the corresponding FederationDomain key. withInformer( secretInformer, controllerlib.FilterFuncs{ ParentFunc: func(obj metav1.Object) controllerlib.Key { - if isOPCControllee(obj) { + if isFederationDomainControllee(obj) { controller := metav1.GetControllerOf(obj) return controllerlib.Key{ Name: controller.Name, @@ -100,17 +100,17 @@ func NewJWKSWriterController( } return controllerlib.Key{} }, - AddFunc: isOPCControllee, + AddFunc: isFederationDomainControllee, UpdateFunc: func(oldObj, newObj metav1.Object) bool { - return isOPCControllee(oldObj) || isOPCControllee(newObj) + return isFederationDomainControllee(oldObj) || isFederationDomainControllee(newObj) }, - DeleteFunc: isOPCControllee, + DeleteFunc: isFederationDomainControllee, }, controllerlib.InformerOption{}, ), - // We want to be notified when anything happens to an OPC. + // We want to be notified when anything happens to an FederationDomain. withInformer( - opcInformer, + federationDomainInformer, pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct controllerlib.InformerOption{}, ), @@ -119,7 +119,7 @@ func NewJWKSWriterController( // Sync implements controllerlib.Syncer. func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { - opc, err := c.opcInformer.Lister().FederationDomains(ctx.Key.Namespace).Get(ctx.Key.Name) + federationDomain, err := c.federationDomainInformer.Lister().FederationDomains(ctx.Key.Namespace).Get(ctx.Key.Name) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return fmt.Errorf( @@ -131,17 +131,17 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { } if notFound { - // The corresponding secret to this OPC should have been garbage collected since it should have - // had this OPC as its owner. + // The corresponding secret to this FederationDomain should have been garbage collected since it should have + // had this FederationDomain as its owner. plog.Debug( - "federationdomain deleted", + "FederationDomain deleted", "federationdomain", klog.KRef(ctx.Key.Namespace, ctx.Key.Name), ) return nil } - secretNeedsUpdate, err := c.secretNeedsUpdate(opc) + secretNeedsUpdate, err := c.secretNeedsUpdate(federationDomain) if err != nil { return fmt.Errorf("cannot determine secret status: %w", err) } @@ -155,9 +155,9 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { return nil } - // If the OPC does not have a secret associated with it, that secret does not exist, or the secret + // If the FederationDomain does not have a secret associated with it, that secret does not exist, or the secret // is invalid, we will generate a new secret (i.e., a JWKS). - secret, err := c.generateSecret(opc) + secret, err := c.generateSecret(federationDomain) if err != nil { return fmt.Errorf("cannot generate secret: %w", err) } @@ -167,25 +167,25 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { } plog.Debug("created/updated secret", "secret", klog.KObj(secret)) - // Ensure that the OPC points to the secret. - newOPC := opc.DeepCopy() - newOPC.Status.Secrets.JWKS.Name = secret.Name - if err := c.updateOPC(ctx.Context, newOPC); err != nil { - return fmt.Errorf("cannot update opc: %w", err) + // Ensure that the FederationDomain points to the secret. + newFederationDomain := federationDomain.DeepCopy() + newFederationDomain.Status.Secrets.JWKS.Name = secret.Name + if err := c.updateFederationDomain(ctx.Context, newFederationDomain); err != nil { + return fmt.Errorf("cannot update FederationDomain: %w", err) } - plog.Debug("updated federationdomain", "federationdomain", klog.KObj(newOPC)) + plog.Debug("updated FederationDomain", "federationdomain", klog.KObj(newFederationDomain)) return nil } -func (c *jwksWriterController) secretNeedsUpdate(opc *configv1alpha1.FederationDomain) (bool, error) { - if opc.Status.Secrets.JWKS.Name == "" { - // If the OPC says it doesn't have a secret associated with it, then let's create one. +func (c *jwksWriterController) secretNeedsUpdate(federationDomain *configv1alpha1.FederationDomain) (bool, error) { + if federationDomain.Status.Secrets.JWKS.Name == "" { + // If the FederationDomain 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.Secrets.JWKS.Name) + // This FederationDomain says it has a secret associated with it. Let's try to get it from the cache. + secret, err := c.secretInformer.Lister().Secrets(federationDomain.Namespace).Get(federationDomain.Status.Secrets.JWKS.Name) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return false, fmt.Errorf("cannot get secret: %w", err) @@ -203,9 +203,9 @@ func (c *jwksWriterController) secretNeedsUpdate(opc *configv1alpha1.FederationD return false, nil } -func (c *jwksWriterController) generateSecret(opc *configv1alpha1.FederationDomain) (*corev1.Secret, error) { - // Note! This is where we could potentially add more handling of OPC spec fields which tell us how - // this OIDC provider should sign and verify ID tokens (e.g., hardcoded token secret, gRPC +func (c *jwksWriterController) generateSecret(federationDomain *configv1alpha1.FederationDomain) (*corev1.Secret, error) { + // Note! This is where we could potentially add more handling of FederationDomain spec fields which tell us how + // this FederationDomain should sign and verify ID tokens (e.g., hardcoded token secret, gRPC // connection to KMS, etc). // // For now, we just generate an new RSA keypair and put that in the secret. @@ -236,14 +236,14 @@ func (c *jwksWriterController) generateSecret(opc *configv1alpha1.FederationDoma s := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: opc.Name + "-jwks", - Namespace: opc.Namespace, + Name: federationDomain.Name + "-jwks", + Namespace: federationDomain.Namespace, Labels: c.jwksSecretLabels, OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(opc, schema.GroupVersionKind{ + *metav1.NewControllerRef(federationDomain, schema.GroupVersionKind{ Group: configv1alpha1.SchemeGroupVersion.Group, Version: configv1alpha1.SchemeGroupVersion.Version, - Kind: opcKind, + Kind: federationDomainKind, }), }, }, @@ -290,34 +290,34 @@ func (c *jwksWriterController) createOrUpdateSecret( }) } -func (c *jwksWriterController) updateOPC( +func (c *jwksWriterController) updateFederationDomain( ctx context.Context, - newOPC *configv1alpha1.FederationDomain, + newFederationDomain *configv1alpha1.FederationDomain, ) error { - opcClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newOPC.Namespace) + federationDomainClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newFederationDomain.Namespace) return retry.RetryOnConflict(retry.DefaultRetry, func() error { - oldOPC, err := opcClient.Get(ctx, newOPC.Name, metav1.GetOptions{}) + oldFederationDomain, err := federationDomainClient.Get(ctx, newFederationDomain.Name, metav1.GetOptions{}) if err != nil { - return fmt.Errorf("cannot get opc: %w", err) + return fmt.Errorf("cannot get FederationDomain: %w", err) } - 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. + if newFederationDomain.Status.Secrets.JWKS.Name == oldFederationDomain.Status.Secrets.JWKS.Name { + // If the existing FederationDomain is up to date, we don't need to update it. return nil } - oldOPC.Status.Secrets.JWKS.Name = newOPC.Status.Secrets.JWKS.Name - _, err = opcClient.Update(ctx, oldOPC, metav1.UpdateOptions{}) + oldFederationDomain.Status.Secrets.JWKS.Name = newFederationDomain.Status.Secrets.JWKS.Name + _, err = federationDomainClient.Update(ctx, oldFederationDomain, metav1.UpdateOptions{}) return err }) } -// isOPCControlle returns whether the provided obj is controlled by an OPC. -func isOPCControllee(obj metav1.Object) bool { +// isFederationDomainControlle returns whether the provided obj is controlled by a FederationDomain. +func isFederationDomainControllee(obj metav1.Object) bool { controller := metav1.GetControllerOf(obj) return controller != nil && controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && - controller.Kind == opcKind + controller.Kind == federationDomainKind } // isValid returns whether the provided secret contains a valid active JWK and verification JWKS. diff --git a/internal/controller/supervisorconfig/jwks_writer_test.go b/internal/controller/supervisorconfig/jwks_writer_test.go index ededdc16..1c3d6565 100644 --- a/internal/controller/supervisorconfig/jwks_writer_test.go +++ b/internal/controller/supervisorconfig/jwks_writer_test.go @@ -145,7 +145,7 @@ func TestJWKSWriterControllerFilterSecret(t *testing.T) { kubernetesfake.NewSimpleClientset(), 0, ).Core().V1().Secrets() - opcInformer := pinnipedinformers.NewSharedInformerFactory( + federationDomainInformer := pinnipedinformers.NewSharedInformerFactory( pinnipedfake.NewSimpleClientset(), 0, ).Config().V1alpha1().FederationDomains() @@ -155,7 +155,7 @@ func TestJWKSWriterControllerFilterSecret(t *testing.T) { nil, // kubeClient, not needed nil, // pinnipedClient, not needed secretInformer, - opcInformer, + federationDomainInformer, withInformer.WithInformer, ) @@ -170,24 +170,24 @@ func TestJWKSWriterControllerFilterSecret(t *testing.T) { } } -func TestJWKSWriterControllerFilterOPC(t *testing.T) { +func TestJWKSWriterControllerFilterFederationDomain(t *testing.T) { t.Parallel() tests := []struct { - name string - opc configv1alpha1.FederationDomain - wantAdd bool - wantUpdate bool - wantDelete bool - wantParent controllerlib.Key + name string + federationDomain configv1alpha1.FederationDomain + wantAdd bool + wantUpdate bool + wantDelete bool + wantParent controllerlib.Key }{ { - name: "anything goes", - opc: configv1alpha1.FederationDomain{}, - wantAdd: true, - wantUpdate: true, - wantDelete: true, - wantParent: controllerlib.Key{}, + name: "anything goes", + federationDomain: configv1alpha1.FederationDomain{}, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + wantParent: controllerlib.Key{}, }, } for _, test := range tests { @@ -199,7 +199,7 @@ func TestJWKSWriterControllerFilterOPC(t *testing.T) { kubernetesfake.NewSimpleClientset(), 0, ).Core().V1().Secrets() - opcInformer := pinnipedinformers.NewSharedInformerFactory( + federationDomainInformer := pinnipedinformers.NewSharedInformerFactory( pinnipedfake.NewSimpleClientset(), 0, ).Config().V1alpha1().FederationDomains() @@ -209,17 +209,17 @@ func TestJWKSWriterControllerFilterOPC(t *testing.T) { nil, // kubeClient, not needed nil, // pinnipedClient, not needed secretInformer, - opcInformer, + federationDomainInformer, withInformer.WithInformer, ) unrelated := configv1alpha1.FederationDomain{} - filter := withInformer.GetFilterForInformer(opcInformer) - require.Equal(t, test.wantAdd, filter.Add(&test.opc)) - require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.opc)) - require.Equal(t, test.wantUpdate, filter.Update(&test.opc, &unrelated)) - require.Equal(t, test.wantDelete, filter.Delete(&test.opc)) - require.Equal(t, test.wantParent, filter.Parent(&test.opc)) + filter := withInformer.GetFilterForInformer(federationDomainInformer) + require.Equal(t, test.wantAdd, filter.Add(&test.federationDomain)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.federationDomain)) + require.Equal(t, test.wantUpdate, filter.Update(&test.federationDomain, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(&test.federationDomain)) + require.Equal(t, test.wantParent, filter.Parent(&test.federationDomain)) }) } } @@ -236,24 +236,24 @@ func TestJWKSWriterControllerSync(t *testing.T) { goodKey, err := x509.ParseECPrivateKey(block.Bytes) require.NoError(t, err) - opcGVR := schema.GroupVersionResource{ + federationDomainGVR := schema.GroupVersionResource{ Group: configv1alpha1.SchemeGroupVersion.Group, Version: configv1alpha1.SchemeGroupVersion.Version, Resource: "federationdomains", } - goodOPC := &configv1alpha1.FederationDomain{ + goodFederationDomain := &configv1alpha1.FederationDomain{ ObjectMeta: metav1.ObjectMeta{ - Name: "good-opc", + Name: "good-federationDomain", Namespace: namespace, - UID: "good-opc-uid", + UID: "good-federationDomain-uid", }, Spec: configv1alpha1.FederationDomainSpec{ Issuer: "https://some-issuer.com", }, } - goodOPCWithStatus := goodOPC.DeepCopy() - goodOPCWithStatus.Status.Secrets.JWKS.Name = goodOPCWithStatus.Name + "-jwks" + goodFederationDomainWithStatus := goodFederationDomain.DeepCopy() + goodFederationDomainWithStatus.Status.Secrets.JWKS.Name = goodFederationDomainWithStatus.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.Secrets.JWKS.Name, + Name: goodFederationDomainWithStatus.Status.Secrets.JWKS.Name, Namespace: namespace, Labels: map[string]string{ "myLabelKey1": "myLabelValue1", @@ -272,10 +272,10 @@ func TestJWKSWriterControllerSync(t *testing.T) { }, OwnerReferences: []metav1.OwnerReference{ { - APIVersion: opcGVR.GroupVersion().String(), + APIVersion: federationDomainGVR.GroupVersion().String(), Kind: "FederationDomain", - Name: goodOPC.Name, - UID: goodOPC.UID, + Name: goodFederationDomain.Name, + UID: goodFederationDomain.UID, BlockOwnerDeletion: boolPtr(true), Controller: boolPtr(true), }, @@ -295,39 +295,39 @@ func TestJWKSWriterControllerSync(t *testing.T) { goodSecret := newSecret("testdata/good-jwk.json", "testdata/good-jwks.json") tests := []struct { - name string - key controllerlib.Key - secrets []*corev1.Secret - configKubeClient func(*kubernetesfake.Clientset) - configPinnipedClient func(*pinnipedfake.Clientset) - opcs []*configv1alpha1.FederationDomain - generateKeyErr error - wantGenerateKeyCount int - wantSecretActions []kubetesting.Action - wantOPCActions []kubetesting.Action - wantError string + name string + key controllerlib.Key + secrets []*corev1.Secret + configKubeClient func(*kubernetesfake.Clientset) + configPinnipedClient func(*pinnipedfake.Clientset) + federationDomains []*configv1alpha1.FederationDomain + generateKeyErr error + wantGenerateKeyCount int + wantSecretActions []kubetesting.Action + wantFederationDomainActions []kubetesting.Action + wantError string }{ { - name: "new opc with no secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + name: "new federationDomain with no secret", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, wantGenerateKeyCount: 1, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), - kubetesting.NewUpdateAction(opcGVR, namespace, goodOPCWithStatus), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithStatus), }, }, { - name: "opc without status with existing secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + name: "federationDomain without status with existing secret", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, secrets: []*corev1.Secret{ goodSecret, @@ -336,46 +336,46 @@ func TestJWKSWriterControllerSync(t *testing.T) { wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), - kubetesting.NewUpdateAction(opcGVR, namespace, goodOPCWithStatus), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), + kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithStatus), }, }, { - name: "existing opc with no secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + name: "existing federationDomain with no secret", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, wantGenerateKeyCount: 1, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { - name: "existing opc with existing secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + name: "existing federationDomain with existing secret", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ goodSecret, }, }, { - name: "deleted opc", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + name: "deleted federationDomain", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, // Nothing to do here since Kube will garbage collect our child secret via its OwnerReference. }, { name: "missing jwk in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("", "testdata/good-jwks.json"), @@ -385,15 +385,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "missing jwks in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/good-jwk.json", ""), @@ -403,15 +403,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "invalid jwk JSON in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/not-json.txt", "testdata/good-jwks.json"), @@ -421,15 +421,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "invalid jwks JSON in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/good-jwk.json", "testdata/not-json.txt"), @@ -439,15 +439,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "public jwk in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/public-jwk.json", "testdata/good-jwks.json"), @@ -457,15 +457,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "private jwks in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/good-jwk.json", "testdata/private-jwks.json"), @@ -475,15 +475,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "invalid jwk key in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/invalid-key-jwk.json", "testdata/good-jwks.json"), @@ -493,15 +493,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "invalid jwks key in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/good-jwk.json", "testdata/invalid-key-jwks.json"), @@ -511,15 +511,15 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "missing active jwks in secret", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, secrets: []*corev1.Secret{ newSecret("testdata/good-jwk.json", "testdata/missing-active-jwks.json"), @@ -529,24 +529,24 @@ func TestJWKSWriterControllerSync(t *testing.T) { kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, - wantOPCActions: []kubetesting.Action{ - kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + wantFederationDomainActions: []kubetesting.Action{ + kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name), }, }, { name: "generate key fails", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPCWithStatus, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomainWithStatus, }, generateKeyErr: errors.New("some generate error"), wantError: "cannot generate secret: cannot generate key: some generate error", }, { name: "get secret fails", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, configKubeClient: func(client *kubernetesfake.Clientset) { client.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { @@ -557,9 +557,9 @@ func TestJWKSWriterControllerSync(t *testing.T) { }, { name: "create secret fails", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, configKubeClient: func(client *kubernetesfake.Clientset) { client.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { @@ -570,9 +570,9 @@ func TestJWKSWriterControllerSync(t *testing.T) { }, { name: "update secret fails", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, secrets: []*corev1.Secret{ newSecret("", ""), @@ -585,30 +585,30 @@ func TestJWKSWriterControllerSync(t *testing.T) { wantError: "cannot create or update secret: some update error", }, { - name: "get opc fails", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + name: "get FederationDomain fails", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, configPinnipedClient: func(client *pinnipedfake.Clientset) { client.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("some get error") }) }, - wantError: "cannot update opc: cannot get opc: some get error", + wantError: "cannot update FederationDomain: cannot get FederationDomain: some get error", }, { - name: "update opc fails", - key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, - opcs: []*configv1alpha1.FederationDomain{ - goodOPC, + name: "update federationDomain fails", + key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name}, + federationDomains: []*configv1alpha1.FederationDomain{ + goodFederationDomain, }, configPinnipedClient: func(client *pinnipedfake.Clientset) { client.PrependReactor("update", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("some update error") }) }, - wantError: "cannot update opc: some update error", + wantError: "cannot update FederationDomain: some update error", }, } for _, test := range tests { @@ -636,9 +636,9 @@ func TestJWKSWriterControllerSync(t *testing.T) { pinnipedAPIClient := pinnipedfake.NewSimpleClientset() pinnipedInformerClient := pinnipedfake.NewSimpleClientset() - for _, opc := range test.opcs { - require.NoError(t, pinnipedAPIClient.Tracker().Add(opc)) - require.NoError(t, pinnipedInformerClient.Tracker().Add(opc)) + for _, federationDomain := range test.federationDomains { + require.NoError(t, pinnipedAPIClient.Tracker().Add(federationDomain)) + require.NoError(t, pinnipedInformerClient.Tracker().Add(federationDomain)) } if test.configPinnipedClient != nil { test.configPinnipedClient(pinnipedAPIClient) @@ -685,8 +685,8 @@ func TestJWKSWriterControllerSync(t *testing.T) { if test.wantSecretActions != nil { require.Equal(t, test.wantSecretActions, kubeAPIClient.Actions()) } - if test.wantOPCActions != nil { - require.Equal(t, test.wantOPCActions, pinnipedAPIClient.Actions()) + if test.wantFederationDomainActions != nil { + require.Equal(t, test.wantFederationDomainActions, pinnipedAPIClient.Actions()) } }) } diff --git a/internal/oidc/provider/oidcprovider.go b/internal/oidc/provider/federation_domain_issuer.go similarity index 66% rename from internal/oidc/provider/oidcprovider.go rename to internal/oidc/provider/federation_domain_issuer.go index 0c1a6151..be683c69 100644 --- a/internal/oidc/provider/oidcprovider.go +++ b/internal/oidc/provider/federation_domain_issuer.go @@ -11,15 +11,16 @@ import ( "go.pinniped.dev/internal/constable" ) -// FederationDomain represents all of the settings and state for an OIDC provider. -type FederationDomain struct { +// FederationDomainIssuer represents all of the settings and state for a downstream OIDC provider +// as defined by a FederationDomain. +type FederationDomainIssuer struct { issuer string issuerHost string issuerPath string } -func NewFederationDomain(issuer string) (*FederationDomain, error) { - p := FederationDomain{issuer: issuer} +func NewFederationDomainIssuer(issuer string) (*FederationDomainIssuer, error) { + p := FederationDomainIssuer{issuer: issuer} err := p.validate() if err != nil { return nil, err @@ -27,9 +28,9 @@ func NewFederationDomain(issuer string) (*FederationDomain, error) { return &p, nil } -func (p *FederationDomain) validate() error { +func (p *FederationDomainIssuer) validate() error { if p.issuer == "" { - return constable.Error("provider must have an issuer") + return constable.Error("federation domain must have an issuer") } issuerURL, err := url.Parse(p.issuer) @@ -63,14 +64,14 @@ func (p *FederationDomain) validate() error { return nil } -func (p *FederationDomain) Issuer() string { +func (p *FederationDomainIssuer) Issuer() string { return p.issuer } -func (p *FederationDomain) IssuerHost() string { +func (p *FederationDomainIssuer) IssuerHost() string { return p.issuerHost } -func (p *FederationDomain) IssuerPath() string { +func (p *FederationDomainIssuer) IssuerPath() string { return p.issuerPath } diff --git a/internal/oidc/provider/oidcprovider_test.go b/internal/oidc/provider/federation_domain_issuer_test.go similarity index 89% rename from internal/oidc/provider/oidcprovider_test.go rename to internal/oidc/provider/federation_domain_issuer_test.go index db24384a..4f7c06e9 100644 --- a/internal/oidc/provider/oidcprovider_test.go +++ b/internal/oidc/provider/federation_domain_issuer_test.go @@ -9,16 +9,16 @@ import ( "github.com/stretchr/testify/require" ) -func TestFederationDomainValidations(t *testing.T) { +func TestFederationDomainIssuerValidations(t *testing.T) { tests := []struct { name string issuer string wantError string }{ { - name: "provider must have an issuer", + name: "must have an issuer", issuer: "", - wantError: "provider must have an issuer", + wantError: "federation domain must have an issuer", }, { name: "no scheme", @@ -72,7 +72,7 @@ func TestFederationDomainValidations(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - _, err := NewFederationDomain(tt.issuer) + _, err := NewFederationDomainIssuer(tt.issuer) if tt.wantError != "" { require.EqualError(t, err, tt.wantError) } else { diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index e5533374..52227ce5 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -32,7 +32,7 @@ import ( // It is thread-safe. type Manager struct { mu sync.RWMutex - providers []*provider.FederationDomain + providers []*provider.FederationDomainIssuer providerHandlers map[string]http.Handler // map of all routes for all providers nextHandler http.Handler // the next handler in a chain, called when this manager didn't know how to handle a request dynamicJWKSProvider jwks.DynamicJWKSProvider // in-memory cache of per-issuer JWKS data @@ -68,9 +68,9 @@ func NewManager( // It also removes any providerHandlers that were previously added but were not passed in to // the current invocation. // -// This method assumes that all of the FederationDomain arguments have already been validated +// This method assumes that all of the FederationDomainIssuer arguments have already been validated // by someone else before they are passed to this method. -func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomain) { +func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIssuer) { m.mu.Lock() defer m.mu.Unlock() diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index 21b8b608..64b7ab4a 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -348,9 +348,9 @@ func TestManager(t *testing.T) { when("given some valid providers via SetProviders()", func() { it.Before(func() { - p1, err := provider.NewFederationDomain(issuer1) + p1, err := provider.NewFederationDomainIssuer(issuer1) r.NoError(err) - p2, err := provider.NewFederationDomain(issuer2) + p2, err := provider.NewFederationDomainIssuer(issuer2) r.NoError(err) subject.SetProviders(p1, p2) @@ -391,9 +391,9 @@ func TestManager(t *testing.T) { when("given the same valid providers as arguments to SetProviders() in reverse order", func() { it.Before(func() { - p1, err := provider.NewFederationDomain(issuer1) + p1, err := provider.NewFederationDomainIssuer(issuer1) r.NoError(err) - p2, err := provider.NewFederationDomain(issuer2) + p2, err := provider.NewFederationDomainIssuer(issuer2) r.NoError(err) subject.SetProviders(p2, p1) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index b45cf70a..ddf2bd6a 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -595,17 +595,17 @@ func requireStatus(t *testing.T, client pinnipedclientset.Interface, ns, name st ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - var opc *v1alpha1.FederationDomain + var federationDomain *v1alpha1.FederationDomain var err error assert.Eventually(t, func() bool { - opc, err = client.ConfigV1alpha1().FederationDomains(ns).Get(ctx, name, metav1.GetOptions{}) + federationDomain, err = client.ConfigV1alpha1().FederationDomains(ns).Get(ctx, name, metav1.GetOptions{}) if err != nil { t.Logf("error trying to get FederationDomain: %s", err.Error()) } - return err == nil && opc.Status.Status == status + return err == nil && federationDomain.Status.Status == status }, time.Minute, 200*time.Millisecond) require.NoError(t, err) - require.Equalf(t, status, opc.Status.Status, "unexpected status (message = '%s')", opc.Status.Message) + require.Equalf(t, status, federationDomain.Status.Status, "unexpected status (message = '%s')", federationDomain.Status.Message) } func newHTTPClient(t *testing.T, caBundle string, dnsOverrides map[string]string) *http.Client { diff --git a/test/library/client.go b/test/library/client.go index af63b67f..7f8f3ea1 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -249,8 +249,8 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string issuer = fmt.Sprintf("http://test-issuer-%s.pinniped.dev", RandHex(t, 8)) } - opcs := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) - opc, err := opcs.Create(createContext, &configv1alpha1.FederationDomain{ + federationDomains := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) + federationDomain, err := federationDomains.Create(createContext, &configv1alpha1.FederationDomain{ ObjectMeta: testObjectMeta(t, "oidc-provider"), Spec: configv1alpha1.FederationDomainSpec{ Issuer: issuer, @@ -258,31 +258,31 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string }, }, metav1.CreateOptions{}) require.NoError(t, err, "could not create test FederationDomain") - t.Logf("created test FederationDomain %s/%s", opc.Namespace, opc.Name) + t.Logf("created test FederationDomain %s/%s", federationDomain.Namespace, federationDomain.Name) t.Cleanup(func() { t.Helper() - t.Logf("cleaning up test FederationDomain %s/%s", opc.Namespace, opc.Name) + t.Logf("cleaning up test FederationDomain %s/%s", federationDomain.Namespace, federationDomain.Name) deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - err := opcs.Delete(deleteCtx, opc.Name, metav1.DeleteOptions{}) + err := federationDomains.Delete(deleteCtx, federationDomain.Name, metav1.DeleteOptions{}) notFound := k8serrors.IsNotFound(err) // It's okay if it is not found, because it might have been deleted by another part of this test. if !notFound { - require.NoErrorf(t, err, "could not cleanup test FederationDomain %s/%s", opc.Namespace, opc.Name) + require.NoErrorf(t, err, "could not cleanup test FederationDomain %s/%s", federationDomain.Namespace, federationDomain.Name) } }) // If we're not expecting any particular status, just return the new FederationDomain immediately. if expectStatus == "" { - return opc + return federationDomain } // Wait for the FederationDomain to enter the expected phase (or time out). var result *configv1alpha1.FederationDomain assert.Eventuallyf(t, func() bool { var err error - result, err = opcs.Get(ctx, opc.Name, metav1.GetOptions{}) + result, err = federationDomains.Get(ctx, federationDomain.Name, metav1.GetOptions{}) require.NoError(t, err) return result.Status.Status == expectStatus }, 60*time.Second, 1*time.Second, "expected the FederationDomain to have status %q", expectStatus) @@ -292,7 +292,7 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string if result.Status.Status == configv1alpha1.SuccessFederationDomainStatusCondition { assert.Eventually(t, func() bool { var err error - result, err = opcs.Get(ctx, opc.Name, metav1.GetOptions{}) + result, err = federationDomains.Get(ctx, federationDomain.Name, metav1.GetOptions{}) require.NoError(t, err) return result.Status.Secrets.JWKS.Name != "" && result.Status.Secrets.TokenSigningKey.Name != "" && @@ -304,7 +304,7 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string require.NotEmpty(t, result.Status.Secrets.StateSigningKey.Name) require.NotEmpty(t, result.Status.Secrets.StateEncryptionKey.Name) } - return opc + return federationDomain } func RandHex(t *testing.T, numBytes int) string {