diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index 81366746..0663469f 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -30,17 +30,17 @@ import ( "go.pinniped.dev/internal/plog" ) -// ProvidersSetter can be notified of all known valid providers with its SetIssuer function. +// FederationDomainsSetter can be notified of all known valid providers with its SetIssuer function. // 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.FederationDomainIssuer) +type FederationDomainsSetter interface { + SetFederationDomains(federationDomains ...*provider.FederationDomainIssuer) } type federationDomainWatcherController struct { - providerSetter ProvidersSetter - clock clock.Clock - client pinnipedclientset.Interface + federationDomainsSetter FederationDomainsSetter + clock clock.Clock + client pinnipedclientset.Interface federationDomainInformer configinformers.FederationDomainInformer oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer @@ -51,7 +51,7 @@ type federationDomainWatcherController struct { // NewFederationDomainWatcherController creates a controllerlib.Controller that watches // FederationDomain objects and notifies a callback object of the collection of provider configs. func NewFederationDomainWatcherController( - providerSetter ProvidersSetter, + federationDomainsSetter FederationDomainsSetter, clock clock.Clock, client pinnipedclientset.Interface, federationDomainInformer configinformers.FederationDomainInformer, @@ -64,7 +64,7 @@ func NewFederationDomainWatcherController( controllerlib.Config{ Name: "FederationDomainWatcherController", Syncer: &federationDomainWatcherController{ - providerSetter: providerSetter, + federationDomainsSetter: federationDomainsSetter, clock: clock, client: client, federationDomainInformer: federationDomainInformer, @@ -438,7 +438,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer) } - c.providerSetter.SetProviders(federationDomainIssuers...) + c.federationDomainsSetter.SetFederationDomains(federationDomainIssuers...) return errors.NewAggregate(errs) } diff --git a/internal/controller/supervisorconfig/federation_domain_watcher_test.go b/internal/controller/supervisorconfig/federation_domain_watcher_test.go index 980ecac6..66eb6e00 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher_test.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher_test.go @@ -89,13 +89,13 @@ func TestInformerFilters(t *testing.T) { }, spec.Parallel(), spec.Report(report.Terminal{})) } -type fakeProvidersSetter struct { - SetProvidersWasCalled bool - FederationDomainsReceived []*provider.FederationDomainIssuer +type fakeFederationDomainsSetter struct { + SetFederationDomainsWasCalled bool + FederationDomainsReceived []*provider.FederationDomainIssuer } -func (f *fakeProvidersSetter) SetProviders(federationDomains ...*provider.FederationDomainIssuer) { - f.SetProvidersWasCalled = true +func (f *fakeFederationDomainsSetter) SetFederationDomains(federationDomains ...*provider.FederationDomainIssuer) { + f.SetFederationDomainsWasCalled = true f.FederationDomainsReceived = federationDomains } @@ -113,7 +113,7 @@ func TestSync(t *testing.T) { var cancelContextCancelFunc context.CancelFunc var syncContext *controllerlib.Context var frozenNow time.Time - var providersSetter *fakeProvidersSetter + var federationDomainsSetter *fakeFederationDomainsSetter var federationDomainGVR schema.GroupVersionResource // Defer starting the informers until the last possible moment so that the @@ -121,7 +121,7 @@ func TestSync(t *testing.T) { var startInformersAndController = func() { // Set this at the last second to allow for injection of server override. subject = NewFederationDomainWatcherController( - providersSetter, + federationDomainsSetter, clocktesting.NewFakeClock(frozenNow), pinnipedAPIClient, pinnipedInformers.Config().V1alpha1().FederationDomains(), @@ -149,7 +149,7 @@ func TestSync(t *testing.T) { it.Before(func() { r = require.New(t) - providersSetter = &fakeProvidersSetter{} + federationDomainsSetter = &fakeFederationDomainsSetter{} frozenNow = time.Date(2020, time.September, 23, 7, 42, 0, 0, time.Local) cancelContext, cancelContextCancelFunc = context.WithCancel(context.Background()) @@ -191,24 +191,24 @@ func TestSync(t *testing.T) { r.NoError(pinnipedInformerClient.Tracker().Add(federationDomain2)) }) - it("calls the ProvidersSetter", func() { + it("calls the FederationDomainsSetter", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + fd1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + fd2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) r.ElementsMatch( []*provider.FederationDomainIssuer{ - provider1, - provider2, + fd1, + fd2, }, - providersSetter.FederationDomainsReceived, + federationDomainsSetter.FederationDomainsReceived, ) }) @@ -292,24 +292,24 @@ func TestSync(t *testing.T) { r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) }) - it("calls the ProvidersSetter with both FederationDomain's", func() { + it("calls the FederationDomainsSetter with both FederationDomain's", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + fd1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + fd2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) r.ElementsMatch( []*provider.FederationDomainIssuer{ - provider1, - provider2, + fd1, + fd2, }, - providersSetter.FederationDomainsReceived, + federationDomainsSetter.FederationDomainsReceived, ) }) }) @@ -330,22 +330,22 @@ func TestSync(t *testing.T) { ) }) - it("sets the provider that it could actually update in the API", func() { + it("sets the FederationDomain that it could actually update in the API", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.EqualError(err, "could not update status: some update error") - provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + fd1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + fd2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) - r.Len(providersSetter.FederationDomainsReceived, 1) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) + r.Len(federationDomainsSetter.FederationDomainsReceived, 1) r.True( - reflect.DeepEqual(providersSetter.FederationDomainsReceived[0], provider1) || - reflect.DeepEqual(providersSetter.FederationDomainsReceived[0], provider2), + reflect.DeepEqual(federationDomainsSetter.FederationDomainsReceived[0], fd1) || + reflect.DeepEqual(federationDomainsSetter.FederationDomainsReceived[0], fd2), ) }) @@ -549,20 +549,20 @@ func TestSync(t *testing.T) { r.NoError(pinnipedInformerClient.Tracker().Add(invalidFederationDomain)) }) - it("calls the ProvidersSetter with the valid provider", func() { + it("calls the FederationDomainsSetter with the valid FederationDomain", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + validFederationDomain, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) r.Equal( []*provider.FederationDomainIssuer{ - validProvider, + validFederationDomain, }, - providersSetter.FederationDomainsReceived, + federationDomainsSetter.FederationDomainsReceived, ) }) @@ -623,20 +623,20 @@ func TestSync(t *testing.T) { ) }) - it("sets the provider that it could actually update in the API", func() { + it("sets the FederationDomain that it could actually update in the API", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.EqualError(err, "could not update status: some update error") - validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + validFederationDomain, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) r.Equal( []*provider.FederationDomainIssuer{ - validProvider, + validFederationDomain, }, - providersSetter.FederationDomainsReceived, + federationDomainsSetter.FederationDomainsReceived, ) }) @@ -713,20 +713,20 @@ func TestSync(t *testing.T) { r.NoError(pinnipedInformerClient.Tracker().Add(federationDomain)) }) - it("calls the ProvidersSetter with the non-duplicate", func() { + it("calls the FederationDomainsSetter with the non-duplicate", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + nonDuplicateFederationDomain, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) r.Equal( []*provider.FederationDomainIssuer{ - nonDuplicateProvider, + nonDuplicateFederationDomain, }, - providersSetter.FederationDomainsReceived, + federationDomainsSetter.FederationDomainsReceived, ) }) @@ -840,7 +840,7 @@ func TestSync(t *testing.T) { it.Before(func() { federationDomainSameIssuerAddress1 = &v1alpha1.FederationDomain{ - ObjectMeta: metav1.ObjectMeta{Name: "provider1", Namespace: namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "fd1", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{ Issuer: "https://iSSueR-duPlicAte-adDress.cOm/path1", TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: "secret1"}, @@ -849,7 +849,7 @@ func TestSync(t *testing.T) { r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress1)) r.NoError(pinnipedInformerClient.Tracker().Add(federationDomainSameIssuerAddress1)) federationDomainSameIssuerAddress2 = &v1alpha1.FederationDomain{ - ObjectMeta: metav1.ObjectMeta{Name: "provider2", Namespace: namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "fd2", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{ // Validation treats these as the same DNS hostname even though they have different port numbers, // because SNI information on the incoming requests is not going to include port numbers. @@ -861,7 +861,7 @@ func TestSync(t *testing.T) { r.NoError(pinnipedInformerClient.Tracker().Add(federationDomainSameIssuerAddress2)) federationDomainDifferentIssuerAddress = &v1alpha1.FederationDomain{ - ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressProvider", Namespace: namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressFederationDomain", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{ Issuer: "https://issuer-not-duplicate.com", TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: "secret1"}, @@ -876,7 +876,7 @@ func TestSync(t *testing.T) { _, err := url.Parse(invalidIssuerURL) //nolint:staticcheck // Yes, this URL is intentionally invalid. r.Error(err) federationDomainWithInvalidIssuerURL = &v1alpha1.FederationDomain{ - ObjectMeta: metav1.ObjectMeta{Name: "invalidIssuerURLProvider", Namespace: namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "invalidIssuerURLFederationDomain", Namespace: namespace}, Spec: v1alpha1.FederationDomainSpec{ Issuer: invalidIssuerURL, TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: "secret1"}, @@ -886,20 +886,20 @@ func TestSync(t *testing.T) { r.NoError(pinnipedInformerClient.Tracker().Add(federationDomainWithInvalidIssuerURL)) }) - it("calls the ProvidersSetter with the non-duplicate", func() { + it("calls the FederationDomainsSetter with the non-duplicate", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomainDifferentIssuerAddress.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) + nonDuplicateFederationDomain, err := provider.NewFederationDomainIssuer(federationDomainDifferentIssuerAddress.Spec.Issuer, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) r.Equal( []*provider.FederationDomainIssuer{ - nonDuplicateProvider, + nonDuplicateFederationDomain, }, - providersSetter.FederationDomainsReceived, + federationDomainsSetter.FederationDomainsReceived, ) }) @@ -1029,8 +1029,8 @@ func TestSync(t *testing.T) { err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) r.Empty(pinnipedAPIClient.Actions()) - r.True(providersSetter.SetProvidersWasCalled) - r.Empty(providersSetter.FederationDomainsReceived) + r.True(federationDomainsSetter.SetFederationDomainsWasCalled) + r.Empty(federationDomainsSetter.FederationDomainsReceived) }) }) }, spec.Parallel(), spec.Report(report.Terminal{})) diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index 5793765d..2c8df7a0 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -69,7 +69,7 @@ func NewManager( } } -// SetProviders adds or updates all the given providerHandlers using each provider's issuer string +// SetFederationDomains adds or updates all the given providerHandlers using each provider's issuer string // as the name of the provider to decide if it is an add or update operation. // // It also removes any providerHandlers that were previously added but were not passed in to @@ -77,7 +77,7 @@ func NewManager( // // 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.FederationDomainIssuer) { +func (m *Manager) SetFederationDomains(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 45319220..8cff064d 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -277,7 +277,7 @@ func TestManager(t *testing.T) { subject = NewManager(nextHandler, dynamicJWKSProvider, idpLister, &cache, secretsClient, oidcClientsClient) }) - when("given no providers via SetProviders()", func() { + when("given no providers via SetFederationDomains()", func() { it("sends all requests to the nextHandler", func() { r.False(fallbackHandlerWasCalled) subject.ServeHTTP(httptest.NewRecorder(), newGetRequest("/anything")) @@ -375,13 +375,13 @@ func TestManager(t *testing.T) { requireTokenRequestToBeHandled(issuer2DifferentCaseHostname, downstreamAuthCode4, issuer2JWKS, issuer2) } - when("given some valid providers via SetProviders()", func() { + when("given some valid providers via SetFederationDomains()", func() { it.Before(func() { - p1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{}) + fd1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - p2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{}) + fd2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - subject.SetProviders(p1, p2) + subject.SetFederationDomains(fd1, fd2) jwksMap := map[string]*jose.JSONWebKeySet{ issuer1: {Keys: []jose.JSONWebKey{*newTestJWK(issuer1KeyID)}}, @@ -418,13 +418,13 @@ func TestManager(t *testing.T) { }) }) - when("given the same valid providers as arguments to SetProviders() in reverse order", func() { + when("given the same valid providers as arguments to SetFederationDomains() in reverse order", func() { it.Before(func() { - p1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{}) + fd1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - p2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{}) + fd2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{}) r.NoError(err) - subject.SetProviders(p2, p1) + subject.SetFederationDomains(fd2, fd1) jwksMap := map[string]*jose.JSONWebKeySet{ issuer1: {Keys: []jose.JSONWebKey{*newTestJWK(issuer1KeyID)}},