djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #298 from vmware-tanzu/more-opc-rename

Browse Source 
Rename all "op" and "opc" usages
This commit is contained in:
Ryan Richard 2020-12-17 12:31:52 -08:00 committed by GitHub
commit 32602f579b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 496 additions and 494 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/pinniped-supervisor/main.go
View File

@ -102,7 +102,7 @@ func startControllers(
supervisorstorage.GarbageCollectorController( supervisorstorage.GarbageCollectorController(
clock.RealClock{}, clock.RealClock{},
kubeClient, kubeClient,
kubeInformers.Core().V1().Secrets(), secretInformer,
controllerlib.WithInformer, controllerlib.WithInformer,
), ),
singletonWorker, singletonWorker,

77
internal/controller/supervisorconfig/oidcproviderconfig_watcher.go → internal/controller/supervisorconfig/federation_domain_watcher.go
View File

@ -29,14 +29,14 @@ import (
// If there are no longer any valid issuers, then it can be called with no arguments. // 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. // Implementations of this type should be thread-safe to support calls from multiple goroutines.
type ProvidersSetter interface { type ProvidersSetter interface {
SetProviders(federationDomains ...*provider.FederationDomain) SetProviders(federationDomains ...*provider.FederationDomainIssuer)
} }
type federationDomainWatcherController struct { type federationDomainWatcherController struct {
providerSetter ProvidersSetter providerSetter ProvidersSetter
clock clock.Clock clock clock.Clock
client pinnipedclientset.Interface client pinnipedclientset.Interface
opcInformer configinformers.FederationDomainInformer federationDomainInformer configinformers.FederationDomainInformer
} }
// NewFederationDomainWatcherController creates a controllerlib.Controller that watches // NewFederationDomainWatcherController creates a controllerlib.Controller that watches
@ -45,21 +45,21 @@ func NewFederationDomainWatcherController(
providerSetter ProvidersSetter, providerSetter ProvidersSetter,
clock clock.Clock, clock clock.Clock,
client pinnipedclientset.Interface, client pinnipedclientset.Interface,
opcInformer configinformers.FederationDomainInformer, federationDomainInformer configinformers.FederationDomainInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc, withInformer pinnipedcontroller.WithInformerOptionFunc,
) controllerlib.Controller { ) controllerlib.Controller {
return controllerlib.New( return controllerlib.New(
controllerlib.Config{ controllerlib.Config{
Name: "FederationDomainWatcherController", Name: "FederationDomainWatcherController",
Syncer: &federationDomainWatcherController{ Syncer: &federationDomainWatcherController{
providerSetter: providerSetter, providerSetter: providerSetter,
clock: clock, clock: clock,
client: client, client: client,
opcInformer: opcInformer, federationDomainInformer: federationDomainInformer,
}, },
}, },
withInformer( withInformer(
opcInformer, federationDomainInformer,
pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()),
controllerlib.InformerOption{}, controllerlib.InformerOption{},
), ),
@ -68,7 +68,7 @@ func NewFederationDomainWatcherController(
// Sync implements controllerlib.Syncer. // Sync implements controllerlib.Syncer.
func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) error { 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 { if err != nil {
return err return err
} }
@ -89,8 +89,8 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
uniqueSecretNamesPerIssuerAddress := make(map[string]map[string]bool) uniqueSecretNamesPerIssuerAddress := make(map[string]map[string]bool)
issuerURLToHostnameKey := lowercaseHostWithoutPort issuerURLToHostnameKey := lowercaseHostWithoutPort
for _, opc := range all { for _, federationDomain := range federationDomains {
issuerURL, err := url.Parse(opc.Spec.Issuer) issuerURL, err := url.Parse(federationDomain.Spec.Issuer)
if err != nil { if err != nil {
continue // Skip url parse errors because they will be validated again below. 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) setOfSecretNames = make(map[string]bool)
uniqueSecretNamesPerIssuerAddress[issuerURLToHostnameKey(issuerURL)] = setOfSecretNames uniqueSecretNamesPerIssuerAddress[issuerURLToHostnameKey(issuerURL)] = setOfSecretNames
} }
if opc.Spec.TLS != nil { if federationDomain.Spec.TLS != nil {
setOfSecretNames[opc.Spec.TLS.SecretName] = true setOfSecretNames[federationDomain.Spec.TLS.SecretName] = true
} }
} }
errs := multierror.New() errs := multierror.New()
federationDomains := make([]*provider.FederationDomain, 0) federationDomainIssuers := make([]*provider.FederationDomainIssuer, 0)
for _, opc := range all { for _, federationDomain := range federationDomains {
issuerURL, urlParseErr := url.Parse(opc.Spec.Issuer) issuerURL, urlParseErr := url.Parse(federationDomain.Spec.Issuer)
// Skip url parse errors because they will be validated below. // Skip url parse errors because they will be validated below.
if urlParseErr == nil { if urlParseErr == nil {
if issuerCount := issuerCounts[issuerURLToIssuerKey(issuerURL)]; issuerCount > 1 { if issuerCount := issuerCounts[issuerURLToIssuerKey(issuerURL)]; issuerCount > 1 {
if err := c.updateStatus( if err := c.updateStatus(
ctx.Context, ctx.Context,
opc.Namespace, federationDomain.Namespace,
opc.Name, federationDomain.Name,
configv1alpha1.DuplicateFederationDomainStatusCondition, configv1alpha1.DuplicateFederationDomainStatusCondition,
"Duplicate issuer: "+opc.Spec.Issuer, "Duplicate issuer: "+federationDomain.Spec.Issuer,
); err != nil { ); err != nil {
errs.Add(fmt.Errorf("could not update status: %w", err)) 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 urlParseErr == nil && len(uniqueSecretNamesPerIssuerAddress[issuerURLToHostnameKey(issuerURL)]) > 1 {
if err := c.updateStatus( if err := c.updateStatus(
ctx.Context, ctx.Context,
opc.Namespace, federationDomain.Namespace,
opc.Name, federationDomain.Name,
configv1alpha1.SameIssuerHostMustUseSameSecretFederationDomainStatusCondition, configv1alpha1.SameIssuerHostMustUseSameSecretFederationDomainStatusCondition,
"Issuers with the same DNS hostname (address not including port) must use the same secretName: "+issuerURLToHostnameKey(issuerURL), "Issuers with the same DNS hostname (address not including port) must use the same secretName: "+issuerURLToHostnameKey(issuerURL),
); err != nil { ); err != nil {
@ -143,12 +143,12 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
continue 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 != nil {
if err := c.updateStatus( if err := c.updateStatus(
ctx.Context, ctx.Context,
opc.Namespace, federationDomain.Namespace,
opc.Name, federationDomain.Name,
configv1alpha1.InvalidFederationDomainStatusCondition, configv1alpha1.InvalidFederationDomainStatusCondition,
"Invalid: "+err.Error(), "Invalid: "+err.Error(),
); err != nil { ); err != nil {
@ -159,18 +159,19 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
if err := c.updateStatus( if err := c.updateStatus(
ctx.Context, ctx.Context,
opc.Namespace, federationDomain.Namespace,
opc.Name, federationDomain.Name,
configv1alpha1.SuccessFederationDomainStatusCondition, configv1alpha1.SuccessFederationDomainStatusCondition,
"Provider successfully created", "Provider successfully created",
); err != nil { ); err != nil {
errs.Add(fmt.Errorf("could not update status: %w", err)) errs.Add(fmt.Errorf("could not update status: %w", err))
continue continue
} }
federationDomains = append(federationDomains, federationDomain)
federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer)
} }
c.providerSetter.SetProviders(federationDomains...) c.providerSetter.SetProviders(federationDomainIssuers...)
return errs.ErrOrNil() return errs.ErrOrNil()
} }
@ -182,28 +183,28 @@ func (c *federationDomainWatcherController) updateStatus(
message string, message string,
) error { ) error {
return retry.RetryOnConflict(retry.DefaultRetry, func() 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 { if err != nil {
return fmt.Errorf("get failed: %w", err) 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 return nil
} }
plog.Debug( plog.Debug(
"attempting status update", "attempting status update",
"federationdomainconfig", "federationdomain",
klog.KRef(namespace, name), klog.KRef(namespace, name),
"status", "status",
status, status,
"message", "message",
message, message,
) )
opc.Status.Status = status federationDomain.Status.Status = status
opc.Status.Message = message federationDomain.Status.Message = message
opc.Status.LastUpdateTime = timePtr(metav1.NewTime(c.clock.Now())) federationDomain.Status.LastUpdateTime = timePtr(metav1.NewTime(c.clock.Now()))
_, err = c.client.ConfigV1alpha1().FederationDomains(namespace).Update(ctx, opc, metav1.UpdateOptions{}) _, err = c.client.ConfigV1alpha1().FederationDomains(namespace).Update(ctx, federationDomain, metav1.UpdateOptions{})
return err return err
}) })
} }

84
internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go → internal/controller/supervisorconfig/federation_domain_watcher_test.go
View File

@ -40,15 +40,15 @@ func TestInformerFilters(t *testing.T) {
it.Before(func() { it.Before(func() {
r = require.New(t) r = require.New(t)
observableWithInformerOption = testutil.NewObservableWithInformerOption() observableWithInformerOption = testutil.NewObservableWithInformerOption()
opcInformer := pinnipedinformers.NewSharedInformerFactoryWithOptions(nil, 0).Config().V1alpha1().FederationDomains() federationDomainInformer := pinnipedinformers.NewSharedInformerFactoryWithOptions(nil, 0).Config().V1alpha1().FederationDomains()
_ = NewFederationDomainWatcherController( _ = NewFederationDomainWatcherController(
nil, nil,
nil, nil,
nil, nil,
opcInformer, federationDomainInformer,
observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters 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() { when("watching FederationDomain objects", func() {
@ -84,10 +84,10 @@ func TestInformerFilters(t *testing.T) {
type fakeProvidersSetter struct { type fakeProvidersSetter struct {
SetProvidersWasCalled bool 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.SetProvidersWasCalled = true
f.FederationDomainsReceived = federationDomains f.FederationDomainsReceived = federationDomains
} }
@ -99,8 +99,8 @@ func TestSync(t *testing.T) {
var r *require.Assertions var r *require.Assertions
var subject controllerlib.Controller var subject controllerlib.Controller
var opcInformerClient *pinnipedfake.Clientset var federationDomainInformerClient *pinnipedfake.Clientset
var opcInformers pinnipedinformers.SharedInformerFactory var federationDomainInformers pinnipedinformers.SharedInformerFactory
var pinnipedAPIClient *pinnipedfake.Clientset var pinnipedAPIClient *pinnipedfake.Clientset
var timeoutContext context.Context var timeoutContext context.Context
var timeoutContextCancel context.CancelFunc var timeoutContextCancel context.CancelFunc
@ -117,7 +117,7 @@ func TestSync(t *testing.T) {
providersSetter, providersSetter,
clock.NewFakeClock(frozenNow), clock.NewFakeClock(frozenNow),
pinnipedAPIClient, pinnipedAPIClient,
opcInformers.Config().V1alpha1().FederationDomains(), federationDomainInformers.Config().V1alpha1().FederationDomains(),
controllerlib.WithInformer, controllerlib.WithInformer,
) )
@ -132,7 +132,7 @@ func TestSync(t *testing.T) {
} }
// Must start informers before calling TestRunSynchronously() // Must start informers before calling TestRunSynchronously()
opcInformers.Start(timeoutContext.Done()) federationDomainInformers.Start(timeoutContext.Done())
controllerlib.TestRunSynchronously(t, subject) controllerlib.TestRunSynchronously(t, subject)
} }
@ -144,8 +144,8 @@ func TestSync(t *testing.T) {
timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3)
opcInformerClient = pinnipedfake.NewSimpleClientset() federationDomainInformerClient = pinnipedfake.NewSimpleClientset()
opcInformers = pinnipedinformers.NewSharedInformerFactory(opcInformerClient, 0) federationDomainInformers = pinnipedinformers.NewSharedInformerFactory(federationDomainInformerClient, 0)
pinnipedAPIClient = pinnipedfake.NewSimpleClientset() pinnipedAPIClient = pinnipedfake.NewSimpleClientset()
federationDomainGVR = schema.GroupVersionResource{ federationDomainGVR = schema.GroupVersionResource{
@ -171,14 +171,14 @@ func TestSync(t *testing.T) {
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer1.com"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer1.com"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain1)) r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain1))
r.NoError(opcInformerClient.Tracker().Add(federationDomain1)) r.NoError(federationDomainInformerClient.Tracker().Add(federationDomain1))
federationDomain2 = &v1alpha1.FederationDomain{ federationDomain2 = &v1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace}, ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace},
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer2.com"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer2.com"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain2)) r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain2))
r.NoError(opcInformerClient.Tracker().Add(federationDomain2)) r.NoError(federationDomainInformerClient.Tracker().Add(federationDomain2))
}) })
it("calls the ProvidersSetter", func() { it("calls the ProvidersSetter", func() {
@ -186,15 +186,15 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.NoError(err) r.NoError(err)
provider1, err := provider.NewFederationDomain(federationDomain1.Spec.Issuer) provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer)
r.NoError(err) r.NoError(err)
provider2, err := provider.NewFederationDomain(federationDomain2.Spec.Issuer) provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer)
r.NoError(err) r.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
r.ElementsMatch( r.ElementsMatch(
[]*provider.FederationDomain{ []*provider.FederationDomainIssuer{
provider1, provider1,
provider2, provider2,
}, },
@ -247,7 +247,7 @@ func TestSync(t *testing.T) {
federationDomain1.Status.LastUpdateTime = timePtr(metav1.NewTime(frozenNow)) federationDomain1.Status.LastUpdateTime = timePtr(metav1.NewTime(frozenNow))
r.NoError(pinnipedAPIClient.Tracker().Update(federationDomainGVR, federationDomain1, federationDomain1.Namespace)) 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() { it("only updates the out-of-date FederationDomain", func() {
@ -284,15 +284,15 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.NoError(err) r.NoError(err)
provider1, err := provider.NewFederationDomain(federationDomain1.Spec.Issuer) provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer)
r.NoError(err) r.NoError(err)
provider2, err := provider.NewFederationDomain(federationDomain2.Spec.Issuer) provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer)
r.NoError(err) r.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
r.ElementsMatch( r.ElementsMatch(
[]*provider.FederationDomain{ []*provider.FederationDomainIssuer{
provider1, provider1,
provider2, provider2,
}, },
@ -322,10 +322,10 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error") 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) r.NoError(err)
provider2, err := provider.NewFederationDomain(federationDomain2.Spec.Issuer) provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer)
r.NoError(err) r.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
@ -387,7 +387,7 @@ func TestSync(t *testing.T) {
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer.com"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer.com"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain)) 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() { 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"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://valid-issuer.com"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(validFederationDomain)) r.NoError(pinnipedAPIClient.Tracker().Add(validFederationDomain))
r.NoError(opcInformerClient.Tracker().Add(validFederationDomain)) r.NoError(federationDomainInformerClient.Tracker().Add(validFederationDomain))
invalidFederationDomain = &v1alpha1.FederationDomain{ invalidFederationDomain = &v1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "invalid-config", Namespace: namespace}, ObjectMeta: metav1.ObjectMeta{Name: "invalid-config", Namespace: namespace},
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://invalid-issuer.com?some=query"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://invalid-issuer.com?some=query"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(invalidFederationDomain)) 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() { it("calls the ProvidersSetter with the valid provider", func() {
@ -536,12 +536,12 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.NoError(err) r.NoError(err)
validProvider, err := provider.NewFederationDomain(validFederationDomain.Spec.Issuer) validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer)
r.NoError(err) r.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
r.Equal( r.Equal(
[]*provider.FederationDomain{ []*provider.FederationDomainIssuer{
validProvider, validProvider,
}, },
providersSetter.FederationDomainsReceived, providersSetter.FederationDomainsReceived,
@ -593,8 +593,8 @@ func TestSync(t *testing.T) {
"federationdomains", "federationdomains",
func(action coretesting.Action) (bool, runtime.Object, error) { func(action coretesting.Action) (bool, runtime.Object, error) {
updateAction := action.(coretesting.UpdateActionImpl) updateAction := action.(coretesting.UpdateActionImpl)
opc := updateAction.Object.(*v1alpha1.FederationDomain) federationDomain := updateAction.Object.(*v1alpha1.FederationDomain)
if opc.Name == validFederationDomain.Name { if federationDomain.Name == validFederationDomain.Name {
return true, nil, nil return true, nil, nil
} }
@ -608,12 +608,12 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error") 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.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
r.Equal( r.Equal(
[]*provider.FederationDomain{ []*provider.FederationDomainIssuer{
validProvider, validProvider,
}, },
providersSetter.FederationDomainsReceived, providersSetter.FederationDomainsReceived,
@ -675,20 +675,20 @@ func TestSync(t *testing.T) {
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://iSSueR-duPlicAte.cOm/a"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://iSSueR-duPlicAte.cOm/a"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDuplicate1)) r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDuplicate1))
r.NoError(opcInformerClient.Tracker().Add(federationDomainDuplicate1)) r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainDuplicate1))
federationDomainDuplicate2 = &v1alpha1.FederationDomain{ federationDomainDuplicate2 = &v1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "duplicate2", Namespace: namespace}, ObjectMeta: metav1.ObjectMeta{Name: "duplicate2", Namespace: namespace},
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/a"}, Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/a"},
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDuplicate2)) r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDuplicate2))
r.NoError(opcInformerClient.Tracker().Add(federationDomainDuplicate2)) r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainDuplicate2))
federationDomain = &v1alpha1.FederationDomain{ federationDomain = &v1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "not-duplicate", Namespace: namespace}, ObjectMeta: metav1.ObjectMeta{Name: "not-duplicate", Namespace: namespace},
Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/A"}, // different path Spec: v1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/A"}, // different path
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomain)) 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() { it("calls the ProvidersSetter with the non-duplicate", func() {
@ -696,12 +696,12 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.NoError(err) r.NoError(err)
nonDuplicateProvider, err := provider.NewFederationDomain(federationDomain.Spec.Issuer) nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer)
r.NoError(err) r.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
r.Equal( r.Equal(
[]*provider.FederationDomain{ []*provider.FederationDomainIssuer{
nonDuplicateProvider, nonDuplicateProvider,
}, },
providersSetter.FederationDomainsReceived, providersSetter.FederationDomainsReceived,
@ -824,7 +824,7 @@ func TestSync(t *testing.T) {
}, },
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress1)) r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress1))
r.NoError(opcInformerClient.Tracker().Add(federationDomainSameIssuerAddress1)) r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainSameIssuerAddress1))
federationDomainSameIssuerAddress2 = &v1alpha1.FederationDomain{ federationDomainSameIssuerAddress2 = &v1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "provider2", Namespace: namespace}, ObjectMeta: metav1.ObjectMeta{Name: "provider2", Namespace: namespace},
Spec: v1alpha1.FederationDomainSpec{ Spec: v1alpha1.FederationDomainSpec{
@ -835,7 +835,7 @@ func TestSync(t *testing.T) {
}, },
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress2)) r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress2))
r.NoError(opcInformerClient.Tracker().Add(federationDomainSameIssuerAddress2)) r.NoError(federationDomainInformerClient.Tracker().Add(federationDomainSameIssuerAddress2))
federationDomainDifferentIssuerAddress = &v1alpha1.FederationDomain{ federationDomainDifferentIssuerAddress = &v1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressProvider", Namespace: namespace}, ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressProvider", Namespace: namespace},
@ -845,7 +845,7 @@ func TestSync(t *testing.T) {
}, },
} }
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainDifferentIssuerAddress)) 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 // 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. // 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(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() { it("calls the ProvidersSetter with the non-duplicate", func() {
@ -868,12 +868,12 @@ func TestSync(t *testing.T) {
err := controllerlib.TestSync(t, subject, *syncContext) err := controllerlib.TestSync(t, subject, *syncContext)
r.NoError(err) r.NoError(err)
nonDuplicateProvider, err := provider.NewFederationDomain(federationDomainDifferentIssuerAddress.Spec.Issuer) nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomainDifferentIssuerAddress.Spec.Issuer)
r.NoError(err) r.NoError(err)
r.True(providersSetter.SetProvidersWasCalled) r.True(providersSetter.SetProvidersWasCalled)
r.Equal( r.Equal(
[]*provider.FederationDomain{ []*provider.FederationDomainIssuer{
nonDuplicateProvider, nonDuplicateProvider,
}, },
providersSetter.FederationDomainsReceived, providersSetter.FederationDomainsReceived,

102
internal/controller/supervisorconfig/generator/oidc_provider_secrets.go → internal/controller/supervisorconfig/generator/federation_domain_secrets.go
View File

@ -25,12 +25,12 @@ import (
) )
type federationDomainSecretsController struct { type federationDomainSecretsController struct {
secretHelper SecretHelper secretHelper SecretHelper
secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference
kubeClient kubernetes.Interface kubeClient kubernetes.Interface
pinnipedClient pinnipedclientset.Interface pinnipedClient pinnipedclientset.Interface
opcInformer configinformers.FederationDomainInformer federationDomainInformer configinformers.FederationDomainInformer
secretInformer corev1informers.SecretInformer secretInformer corev1informers.SecretInformer
} }
// NewFederationDomainSecretsController returns a controllerlib.Controller that ensures a child Secret // NewFederationDomainSecretsController returns a controllerlib.Controller that ensures a child Secret
@ -42,27 +42,27 @@ func NewFederationDomainSecretsController(
kubeClient kubernetes.Interface, kubeClient kubernetes.Interface,
pinnipedClient pinnipedclientset.Interface, pinnipedClient pinnipedclientset.Interface,
secretInformer corev1informers.SecretInformer, secretInformer corev1informers.SecretInformer,
opcInformer configinformers.FederationDomainInformer, federationDomainInformer configinformers.FederationDomainInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc, withInformer pinnipedcontroller.WithInformerOptionFunc,
) controllerlib.Controller { ) controllerlib.Controller {
return controllerlib.New( return controllerlib.New(
controllerlib.Config{ controllerlib.Config{
Name: fmt.Sprintf("%s%s", secretHelper.NamePrefix(), "controller"), Name: fmt.Sprintf("%s%s", secretHelper.NamePrefix(), "controller"),
Syncer: &federationDomainSecretsController{ Syncer: &federationDomainSecretsController{
secretHelper: secretHelper, secretHelper: secretHelper,
secretRefFunc: secretRefFunc, secretRefFunc: secretRefFunc,
kubeClient: kubeClient, kubeClient: kubeClient,
pinnipedClient: pinnipedClient, pinnipedClient: pinnipedClient,
secretInformer: secretInformer, secretInformer: secretInformer,
opcInformer: opcInformer, federationDomainInformer: federationDomainInformer,
}, },
}, },
// We want to be notified when a OPC's secret gets updated or deleted. When this happens, we // We want to be notified when a FederationDomain's secret gets updated or deleted. When this happens, we
// should get notified via the corresponding OPC key. // should get notified via the corresponding FederationDomain key.
withInformer( withInformer(
secretInformer, secretInformer,
pinnipedcontroller.SimpleFilter(isOPControllee, func(obj metav1.Object) controllerlib.Key { pinnipedcontroller.SimpleFilter(isFederationDomainControllee, func(obj metav1.Object) controllerlib.Key {
if isOPControllee(obj) { if isFederationDomainControllee(obj) {
controller := metav1.GetControllerOf(obj) controller := metav1.GetControllerOf(obj)
return controllerlib.Key{ return controllerlib.Key{
Name: controller.Name, Name: controller.Name,
@ -73,9 +73,9 @@ func NewFederationDomainSecretsController(
}), }),
controllerlib.InformerOption{}, 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( withInformer(
opcInformer, federationDomainInformer,
pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct
controllerlib.InformerOption{}, controllerlib.InformerOption{},
), ),
@ -83,7 +83,7 @@ func NewFederationDomainSecretsController(
} }
func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) error { 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) notFound := k8serrors.IsNotFound(err)
if err != nil && !notFound { if err != nil && !notFound {
return fmt.Errorf( return fmt.Errorf(
@ -95,8 +95,8 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro
} }
if notFound { if notFound {
// The corresponding secret to this OP should have been garbage collected since it should have // The corresponding secret to this FederationDomain should have been garbage collected since it should have
// had this OP as its owner. // had this FederationDomain as its owner.
plog.Debug( plog.Debug(
"federationdomain deleted", "federationdomain deleted",
"federationdomain", "federationdomain",
@ -105,13 +105,13 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro
return nil return nil
} }
op = op.DeepCopy() federationDomain = federationDomain.DeepCopy()
newSecret, err := c.secretHelper.Generate(op) newSecret, err := c.secretHelper.Generate(federationDomain)
if err != nil { if err != nil {
return fmt.Errorf("failed to generate secret: %w", err) 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 { if err != nil {
return fmt.Errorf("failed to determine secret status: %w", err) return fmt.Errorf("failed to determine secret status: %w", err)
} }
@ -120,44 +120,44 @@ func (c *federationDomainSecretsController) Sync(ctx controllerlib.Context) erro
plog.Debug( plog.Debug(
"secret is up to date", "secret is up to date",
"federationdomain", "federationdomain",
klog.KObj(op), klog.KObj(federationDomain),
"secret", "secret",
klog.KObj(existingSecret), klog.KObj(existingSecret),
) )
op = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(op, existingSecret) federationDomain = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(federationDomain, existingSecret)
if err := c.updateFederationDomain(ctx.Context, op); err != nil { if err := c.updateFederationDomain(ctx.Context, federationDomain); err != nil {
return fmt.Errorf("failed to update federationdomain: %w", err) 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 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. // 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) 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) federationDomain = c.secretHelper.ObserveActiveSecretAndUpdateParentFederationDomain(federationDomain, newSecret)
if err := c.updateFederationDomain(ctx.Context, op); err != nil { if err := c.updateFederationDomain(ctx.Context, federationDomain); err != nil {
return fmt.Errorf("failed to update federationdomain: %w", err) 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 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. // needs to be updated. It returns the existing secret as its second argument.
func (c *federationDomainSecretsController) secretNeedsUpdate( func (c *federationDomainSecretsController) secretNeedsUpdate(
op *configv1alpha1.FederationDomain, federationDomain *configv1alpha1.FederationDomain,
secretName string, secretName string,
) (bool, *corev1.Secret, error) { ) (bool, *corev1.Secret, error) {
// This OPC says it has a secret associated with it. Let's try to get it from the cache. // 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(op.Namespace).Get(secretName) secret, err := c.secretInformer.Lister().Secrets(federationDomain.Namespace).Get(secretName)
notFound := k8serrors.IsNotFound(err) notFound := k8serrors.IsNotFound(err)
if err != nil && !notFound { if err != nil && !notFound {
return false, nil, fmt.Errorf("cannot get secret: %w", err) return false, nil, fmt.Errorf("cannot get secret: %w", err)
@ -167,7 +167,7 @@ func (c *federationDomainSecretsController) secretNeedsUpdate(
return true, nil, nil 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. // If this secret is invalid, we need to generate a new one.
return true, secret, nil return true, secret, nil
} }
@ -177,7 +177,7 @@ func (c *federationDomainSecretsController) secretNeedsUpdate(
func (c *federationDomainSecretsController) createOrUpdateSecret( func (c *federationDomainSecretsController) createOrUpdateSecret(
ctx context.Context, ctx context.Context,
op *configv1alpha1.FederationDomain, federationDomain *configv1alpha1.FederationDomain,
newSecret **corev1.Secret, newSecret **corev1.Secret,
) error { ) error {
secretClient := c.kubeClient.CoreV1().Secrets((*newSecret).Namespace) 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. // 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 // If the secret already has valid a valid Secret, then we are good to go and we don't need an
// update. // update.
*newSecret = oldSecret *newSecret = oldSecret
@ -216,23 +216,23 @@ func (c *federationDomainSecretsController) createOrUpdateSecret(
func (c *federationDomainSecretsController) updateFederationDomain( func (c *federationDomainSecretsController) updateFederationDomain(
ctx context.Context, ctx context.Context,
newOP *configv1alpha1.FederationDomain, newFederationDomain *configv1alpha1.FederationDomain,
) error { ) error {
opcClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newOP.Namespace) federationDomainClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newFederationDomain.Namespace)
return retry.RetryOnConflict(retry.DefaultRetry, func() error { 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 { 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) oldFederationDomainSecretRef := c.secretRefFunc(oldFederationDomain)
newOPSecretRef := c.secretRefFunc(newOP) newFederationDomainSecretRef := c.secretRefFunc(newFederationDomain)
if reflect.DeepEqual(oldOPSecretRef, newOPSecretRef) { if reflect.DeepEqual(oldFederationDomainSecretRef, newFederationDomainSecretRef) {
return nil return nil
} }
*oldOPSecretRef = *newOPSecretRef *oldFederationDomainSecretRef = *newFederationDomainSecretRef
_, err = opcClient.Update(ctx, oldOP, metav1.UpdateOptions{}) _, err = federationDomainClient.Update(ctx, oldFederationDomain, metav1.UpdateOptions{})
return err return err
}) })
} }

248
internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go → internal/controller/supervisorconfig/generator/federation_domain_secrets_test.go
View File

@ -150,7 +150,7 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) {
kubernetesfake.NewSimpleClientset(), kubernetesfake.NewSimpleClientset(),
0, 0,
).Core().V1().Secrets() ).Core().V1().Secrets()
opcInformer := pinnipedinformers.NewSharedInformerFactory( federationDomainInformer := pinnipedinformers.NewSharedInformerFactory(
pinnipedfake.NewSimpleClientset(), pinnipedfake.NewSimpleClientset(),
0, 0,
).Config().V1alpha1().FederationDomains() ).Config().V1alpha1().FederationDomains()
@ -161,7 +161,7 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) {
nil, // kubeClient, not needed nil, // kubeClient, not needed
nil, // pinnipedClient, not needed nil, // pinnipedClient, not needed
secretInformer, secretInformer,
opcInformer, federationDomainInformer,
withInformer.WithInformer, withInformer.WithInformer,
) )
@ -176,24 +176,24 @@ func TestFederationDomainControllerFilterSecret(t *testing.T) {
} }
} }
func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) { func TestNewFederationDomainSecretsControllerFilterFederationDomain(t *testing.T) {
t.Parallel() t.Parallel()
tests := []struct { tests := []struct {
name string name string
opc configv1alpha1.FederationDomain federationDomain configv1alpha1.FederationDomain
wantAdd bool wantAdd bool
wantUpdate bool wantUpdate bool
wantDelete bool wantDelete bool
wantParent controllerlib.Key wantParent controllerlib.Key
}{ }{
{ {
name: "anything goes", name: "anything goes",
opc: configv1alpha1.FederationDomain{}, federationDomain: configv1alpha1.FederationDomain{},
wantAdd: true, wantAdd: true,
wantUpdate: true, wantUpdate: true,
wantDelete: true, wantDelete: true,
wantParent: controllerlib.Key{}, wantParent: controllerlib.Key{},
}, },
} }
for _, test := range tests { for _, test := range tests {
@ -210,7 +210,7 @@ func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) {
kubernetesfake.NewSimpleClientset(), kubernetesfake.NewSimpleClientset(),
0, 0,
).Core().V1().Secrets() ).Core().V1().Secrets()
opcInformer := pinnipedinformers.NewSharedInformerFactory( federationDomainInformer := pinnipedinformers.NewSharedInformerFactory(
pinnipedfake.NewSimpleClientset(), pinnipedfake.NewSimpleClientset(),
0, 0,
).Config().V1alpha1().FederationDomains() ).Config().V1alpha1().FederationDomains()
@ -221,17 +221,17 @@ func TestNewFederationDomainSecretsControllerFilterOPC(t *testing.T) {
nil, // kubeClient, not needed nil, // kubeClient, not needed
nil, // pinnipedClient, not needed nil, // pinnipedClient, not needed
secretInformer, secretInformer,
opcInformer, federationDomainInformer,
withInformer.WithInformer, withInformer.WithInformer,
) )
unrelated := configv1alpha1.FederationDomain{} unrelated := configv1alpha1.FederationDomain{}
filter := withInformer.GetFilterForInformer(opcInformer) filter := withInformer.GetFilterForInformer(federationDomainInformer)
require.Equal(t, test.wantAdd, filter.Add(&test.opc)) require.Equal(t, test.wantAdd, filter.Add(&test.federationDomain))
require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.opc)) require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.federationDomain))
require.Equal(t, test.wantUpdate, filter.Update(&test.opc, &unrelated)) require.Equal(t, test.wantUpdate, filter.Update(&test.federationDomain, &unrelated))
require.Equal(t, test.wantDelete, filter.Delete(&test.opc)) require.Equal(t, test.wantDelete, filter.Delete(&test.federationDomain))
require.Equal(t, test.wantParent, filter.Parent(&test.opc)) require.Equal(t, test.wantParent, filter.Parent(&test.federationDomain))
}) })
} }
} }
@ -242,14 +242,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
const ( const (
namespace = "some-namespace" namespace = "some-namespace"
opName = "op-name" federationDomainName = "federationDomain-name"
opUID = "op-uid" federationDomainUID = "federationDomain-uid"
secretName = "secret-name" secretName = "secret-name"
secretUID = "secret-uid" secretUID = "secret-uid"
) )
opGVR := schema.GroupVersionResource{ federationDomainGVR := schema.GroupVersionResource{
Group: configv1alpha1.SchemeGroupVersion.Group, Group: configv1alpha1.SchemeGroupVersion.Group,
Version: configv1alpha1.SchemeGroupVersion.Version, Version: configv1alpha1.SchemeGroupVersion.Version,
Resource: "federationdomains", Resource: "federationdomains",
@ -261,11 +261,11 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
Resource: "secrets", Resource: "secrets",
} }
goodOP := &configv1alpha1.FederationDomain{ goodFederationDomain := &configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: opName, Name: federationDomainName,
Namespace: namespace, Namespace: namespace,
UID: opUID, UID: federationDomainUID,
}, },
} }
@ -276,10 +276,10 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
UID: secretUID, UID: secretUID,
OwnerReferences: []metav1.OwnerReference{ OwnerReferences: []metav1.OwnerReference{
{ {
APIVersion: opGVR.GroupVersion().String(), APIVersion: federationDomainGVR.GroupVersion().String(),
Kind: "FederationDomain", Kind: "FederationDomain",
Name: opName, Name: federationDomainName,
UID: opUID, UID: federationDomainUID,
BlockOwnerDeletion: boolPtr(true), BlockOwnerDeletion: boolPtr(true),
Controller: boolPtr(true), Controller: boolPtr(true),
}, },
@ -295,14 +295,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
}, },
} }
goodOPWithTokenSigningKey := goodOP.DeepCopy() goodFederationDomainWithTokenSigningKey := goodFederationDomain.DeepCopy()
goodOPWithTokenSigningKey.Status.Secrets.TokenSigningKey.Name = goodSecret.Name goodFederationDomainWithTokenSigningKey.Status.Secrets.TokenSigningKey.Name = goodSecret.Name
goodOPWithJWKS := goodOP.DeepCopy() goodFederationDomainWithJWKS := goodFederationDomain.DeepCopy()
goodOPWithJWKS.Status.Secrets.JWKS.Name = "some-jwks-key" goodFederationDomainWithJWKS.Status.Secrets.JWKS.Name = "some-jwks-key"
goodOPWithJWKSAndTokenSigningKey := goodOPWithJWKS.DeepCopy() goodFederationDomainWithJWKSAndTokenSigningKey := goodFederationDomainWithJWKS.DeepCopy()
goodOPWithJWKSAndTokenSigningKey.Status.Secrets.TokenSigningKey = goodOPWithTokenSigningKey.Status.Secrets.TokenSigningKey goodFederationDomainWithJWKSAndTokenSigningKey.Status.Secrets.TokenSigningKey = goodFederationDomainWithTokenSigningKey.Status.Secrets.TokenSigningKey
invalidSecret := &corev1.Secret{ invalidSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
@ -311,10 +311,10 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
UID: secretUID, UID: secretUID,
OwnerReferences: []metav1.OwnerReference{ OwnerReferences: []metav1.OwnerReference{
{ {
APIVersion: opGVR.GroupVersion().String(), APIVersion: federationDomainGVR.GroupVersion().String(),
Kind: "FederationDomain", Kind: "FederationDomain",
Name: opName, Name: federationDomainName,
UID: opUID, UID: federationDomainUID,
BlockOwnerDeletion: boolPtr(true), BlockOwnerDeletion: boolPtr(true),
Controller: boolPtr(true), Controller: boolPtr(true),
}, },
@ -323,39 +323,39 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
} }
tests := []struct { tests := []struct {
name string name string
storage func(**configv1alpha1.FederationDomain, **corev1.Secret) storage func(**configv1alpha1.FederationDomain, **corev1.Secret)
client func(*pinnipedfake.Clientset, *kubernetesfake.Clientset) client func(*pinnipedfake.Clientset, *kubernetesfake.Clientset)
secretHelper func(*mocksecrethelper.MockSecretHelper) secretHelper func(*mocksecrethelper.MockSecretHelper)
wantOPActions []kubetesting.Action wantFederationDomainActions []kubetesting.Action
wantSecretActions []kubetesting.Action wantSecretActions []kubetesting.Action
wantError string wantError string
}{ }{
{ {
name: "FederationDomain does not exist and secret does not exist", name: "FederationDomain does not exist and secret does not exist",
storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { storage: func(federationDomain **configv1alpha1.FederationDomain, s **corev1.Secret) {
*op = nil *federationDomain = nil
*s = nil *s = nil
}, },
}, },
{ {
name: "FederationDomain does not exist and secret exists", name: "FederationDomain does not exist and secret exists",
storage: func(op **configv1alpha1.FederationDomain, s **corev1.Secret) { storage: func(federationDomain **configv1alpha1.FederationDomain, s **corev1.Secret) {
*op = nil *federationDomain = nil
}, },
}, },
{ {
name: "FederationDomain exists and secret does not exist", 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 *s = nil
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), 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", 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 *s = nil
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) {
c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, goodOPWithJWKS, nil return true, goodFederationDomainWithJWKS, nil
}) })
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithJWKSAndTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithJWKSAndTokenSigningKey),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), 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", 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 *s = nil
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) {
c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, goodOPWithJWKSAndTokenSigningKey, nil return true, goodFederationDomainWithJWKSAndTokenSigningKey, nil
}) })
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
@ -409,17 +409,17 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
}, },
{ {
name: "FederationDomain exists and invalid secret exists", 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() *s = invalidSecret.DeepCopy()
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(2).Return(false)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
@ -429,7 +429,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
{ {
name: "FederationDomain exists and generating a secret fails", name: "FederationDomain exists and generating a secret fails",
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { 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", wantError: "failed to generate secret: some generate error",
}, },
@ -439,14 +439,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
otherSecret := goodSecret.DeepCopy() otherSecret := goodSecret.DeepCopy()
otherSecret.UID = "other-secret-uid" otherSecret.UID = "other-secret-uid"
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(otherSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(otherSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(1).Return(false)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(true) secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(1).Return(true)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), 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", name: "FederationDomain exists and invalid secret exists and getting secret fails",
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(1).Return(false)
}, },
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
c.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { 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", 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 *s = nil
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { 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) { client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
c.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { 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", name: "FederationDomain exists and invalid secret exists and updating secret fails",
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(2).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, goodSecret).Times(2).Return(false)
}, },
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
c.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { 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", 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() *s = invalidSecret.DeepCopy()
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(3).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(3).Return(false)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) {
once := sync.Once{} once := sync.Once{}
@ -522,9 +522,9 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
return true, nil, err return true, nil, err
}) })
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), 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", 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() *s = invalidSecret.DeepCopy()
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(2).Return(false)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) {
c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { c.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some get error") return true, nil, errors.New("some get error")
}) })
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), 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", 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() *s = invalidSecret.DeepCopy()
}, },
secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) {
secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().Generate(goodFederationDomain).Times(1).Return(goodSecret, nil)
secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false) secretHelper.EXPECT().IsValid(goodFederationDomain, invalidSecret).Times(2).Return(false)
secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodOP, goodSecret).Times(1).Return(goodOPWithTokenSigningKey) secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentFederationDomain(goodFederationDomain, goodSecret).Times(1).Return(goodFederationDomainWithTokenSigningKey)
}, },
client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) {
once := sync.Once{} once := sync.Once{}
@ -575,11 +575,11 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
return true, nil, err return true, nil, err
}) })
}, },
wantOPActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey),
kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithTokenSigningKey), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithTokenSigningKey),
}, },
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
@ -601,14 +601,14 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
kubeAPIClient := kubernetesfake.NewSimpleClientset() kubeAPIClient := kubernetesfake.NewSimpleClientset()
kubeInformerClient := kubernetesfake.NewSimpleClientset() kubeInformerClient := kubernetesfake.NewSimpleClientset()
op := goodOP.DeepCopy() federationDomain := goodFederationDomain.DeepCopy()
secret := goodSecret.DeepCopy() secret := goodSecret.DeepCopy()
if test.storage != nil { if test.storage != nil {
test.storage(&op, &secret) test.storage(&federationDomain, &secret)
} }
if op != nil { if federationDomain != nil {
require.NoError(t, pinnipedAPIClient.Tracker().Add(op)) require.NoError(t, pinnipedAPIClient.Tracker().Add(federationDomain))
require.NoError(t, pinnipedInformerClient.Tracker().Add(op)) require.NoError(t, pinnipedInformerClient.Tracker().Add(federationDomain))
} }
if secret != nil { if secret != nil {
require.NoError(t, kubeAPIClient.Tracker().Add(secret)) require.NoError(t, kubeAPIClient.Tracker().Add(secret))
@ -657,7 +657,7 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
Context: ctx, Context: ctx,
Key: controllerlib.Key{ Key: controllerlib.Key{
Namespace: namespace, Namespace: namespace,
Name: opName, Name: federationDomainName,
}, },
}) })
if test.wantError != "" { if test.wantError != "" {
@ -666,10 +666,10 @@ func TestFederationDomainSecretsControllerSync(t *testing.T) {
} }
require.NoError(t, err) require.NoError(t, err)
if test.wantOPActions == nil { if test.wantFederationDomainActions == nil {
test.wantOPActions = []kubetesting.Action{} test.wantFederationDomainActions = []kubetesting.Action{}
} }
require.Equal(t, test.wantOPActions, pinnipedAPIClient.Actions()) require.Equal(t, test.wantFederationDomainActions, pinnipedAPIClient.Actions())
if test.wantSecretActions == nil { if test.wantSecretActions == nil {
test.wantSecretActions = []kubetesting.Action{} test.wantSecretActions = []kubetesting.Action{}
} }

4
internal/controller/supervisorconfig/generator/generator.go
View File

@ -95,8 +95,8 @@ func generateSecret(namespace, name string, labels map[string]string, secretData
}, nil }, nil
} }
// isOPCControlle returns whether the provided obj is controlled by an OPC. // isFederationDomainControllee returns whether the provided obj is controlled by an FederationDomain.
func isOPControllee(obj metav1.Object) bool { func isFederationDomainControllee(obj metav1.Object) bool {
controller := metav1.GetControllerOf(obj) controller := metav1.GetControllerOf(obj)
return controller != nil && return controller != nil &&
controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() &&

120
internal/controller/supervisorconfig/jwks_writer.go
View File

@ -30,7 +30,7 @@ import (
"go.pinniped.dev/internal/plog" "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 ( const (
// activeJWKKey points to the current private key used for signing tokens. // activeJWKKey points to the current private key used for signing tokens.
// //
@ -43,7 +43,7 @@ const (
) )
const ( const (
opcKind = "FederationDomain" federationDomainKind = "FederationDomain"
) )
// generateKey is stubbed out for the purpose of testing. The default behavior is to generate an EC key. // 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) 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. // secrets, both via a cache and via the API.
type jwksWriterController struct { type jwksWriterController struct {
jwksSecretLabels map[string]string jwksSecretLabels map[string]string
pinnipedClient pinnipedclientset.Interface pinnipedClient pinnipedclientset.Interface
kubeClient kubernetes.Interface kubeClient kubernetes.Interface
opcInformer configinformers.FederationDomainInformer federationDomainInformer configinformers.FederationDomainInformer
secretInformer corev1informers.SecretInformer 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. // Secret that contains a valid active JWK and JWKS.
func NewJWKSWriterController( func NewJWKSWriterController(
jwksSecretLabels map[string]string, jwksSecretLabels map[string]string,
kubeClient kubernetes.Interface, kubeClient kubernetes.Interface,
pinnipedClient pinnipedclientset.Interface, pinnipedClient pinnipedclientset.Interface,
secretInformer corev1informers.SecretInformer, secretInformer corev1informers.SecretInformer,
opcInformer configinformers.FederationDomainInformer, federationDomainInformer configinformers.FederationDomainInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc, withInformer pinnipedcontroller.WithInformerOptionFunc,
) controllerlib.Controller { ) controllerlib.Controller {
return controllerlib.New( return controllerlib.New(
controllerlib.Config{ controllerlib.Config{
Name: "JWKSController", Name: "JWKSController",
Syncer: &jwksWriterController{ Syncer: &jwksWriterController{
jwksSecretLabels: jwksSecretLabels, jwksSecretLabels: jwksSecretLabels,
kubeClient: kubeClient, kubeClient: kubeClient,
pinnipedClient: pinnipedClient, pinnipedClient: pinnipedClient,
secretInformer: secretInformer, secretInformer: secretInformer,
opcInformer: opcInformer, federationDomainInformer: federationDomainInformer,
}, },
}, },
// We want to be notified when a OPC's secret gets updated or deleted. When this happens, we // We want to be notified when a FederationDomain's secret gets updated or deleted. When this happens, we
// should get notified via the corresponding OPC key. // should get notified via the corresponding FederationDomain key.
withInformer( withInformer(
secretInformer, secretInformer,
controllerlib.FilterFuncs{ controllerlib.FilterFuncs{
ParentFunc: func(obj metav1.Object) controllerlib.Key { ParentFunc: func(obj metav1.Object) controllerlib.Key {
if isOPCControllee(obj) { if isFederationDomainControllee(obj) {
controller := metav1.GetControllerOf(obj) controller := metav1.GetControllerOf(obj)
return controllerlib.Key{ return controllerlib.Key{
Name: controller.Name, Name: controller.Name,
@ -100,17 +100,17 @@ func NewJWKSWriterController(
} }
return controllerlib.Key{} return controllerlib.Key{}
}, },
AddFunc: isOPCControllee, AddFunc: isFederationDomainControllee,
UpdateFunc: func(oldObj, newObj metav1.Object) bool { UpdateFunc: func(oldObj, newObj metav1.Object) bool {
return isOPCControllee(oldObj) || isOPCControllee(newObj) return isFederationDomainControllee(oldObj) || isFederationDomainControllee(newObj)
}, },
DeleteFunc: isOPCControllee, DeleteFunc: isFederationDomainControllee,
}, },
controllerlib.InformerOption{}, 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( withInformer(
opcInformer, federationDomainInformer,
pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct
controllerlib.InformerOption{}, controllerlib.InformerOption{},
), ),
@ -119,7 +119,7 @@ func NewJWKSWriterController(
// Sync implements controllerlib.Syncer. // Sync implements controllerlib.Syncer.
func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { 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) notFound := k8serrors.IsNotFound(err)
if err != nil && !notFound { if err != nil && !notFound {
return fmt.Errorf( return fmt.Errorf(
@ -131,17 +131,17 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error {
} }
if notFound { if notFound {
// The corresponding secret to this OPC should have been garbage collected since it should have // The corresponding secret to this FederationDomain should have been garbage collected since it should have
// had this OPC as its owner. // had this FederationDomain as its owner.
plog.Debug( plog.Debug(
"federationdomain deleted", "FederationDomain deleted",
"federationdomain", "federationdomain",
klog.KRef(ctx.Key.Namespace, ctx.Key.Name), klog.KRef(ctx.Key.Namespace, ctx.Key.Name),
) )
return nil return nil
} }
secretNeedsUpdate, err := c.secretNeedsUpdate(opc) secretNeedsUpdate, err := c.secretNeedsUpdate(federationDomain)
if err != nil { if err != nil {
return fmt.Errorf("cannot determine secret status: %w", err) return fmt.Errorf("cannot determine secret status: %w", err)
} }
@ -155,9 +155,9 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error {
return nil 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). // 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 { if err != nil {
return fmt.Errorf("cannot generate secret: %w", err) 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)) plog.Debug("created/updated secret", "secret", klog.KObj(secret))
// Ensure that the OPC points to the secret. // Ensure that the FederationDomain points to the secret.
newOPC := opc.DeepCopy() newFederationDomain := federationDomain.DeepCopy()
newOPC.Status.Secrets.JWKS.Name = secret.Name newFederationDomain.Status.Secrets.JWKS.Name = secret.Name
if err := c.updateOPC(ctx.Context, newOPC); err != nil { if err := c.updateFederationDomain(ctx.Context, newFederationDomain); err != nil {
return fmt.Errorf("cannot update opc: %w", err) 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 return nil
} }
func (c *jwksWriterController) secretNeedsUpdate(opc *configv1alpha1.FederationDomain) (bool, error) { func (c *jwksWriterController) secretNeedsUpdate(federationDomain *configv1alpha1.FederationDomain) (bool, error) {
if opc.Status.Secrets.JWKS.Name == "" { if federationDomain.Status.Secrets.JWKS.Name == "" {
// If the OPC says it doesn't have a secret associated with it, then let's create one. // If the FederationDomain says it doesn't have a secret associated with it, then let's create one.
return true, nil return true, nil
} }
// This OPC says it has a secret associated with it. Let's try to get it from the cache. // 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(opc.Namespace).Get(opc.Status.Secrets.JWKS.Name) secret, err := c.secretInformer.Lister().Secrets(federationDomain.Namespace).Get(federationDomain.Status.Secrets.JWKS.Name)
notFound := k8serrors.IsNotFound(err) notFound := k8serrors.IsNotFound(err)
if err != nil && !notFound { if err != nil && !notFound {
return false, fmt.Errorf("cannot get secret: %w", err) return false, fmt.Errorf("cannot get secret: %w", err)
@ -203,9 +203,9 @@ func (c *jwksWriterController) secretNeedsUpdate(opc *configv1alpha1.FederationD
return false, nil return false, nil
} }
func (c *jwksWriterController) generateSecret(opc *configv1alpha1.FederationDomain) (*corev1.Secret, error) { func (c *jwksWriterController) generateSecret(federationDomain *configv1alpha1.FederationDomain) (*corev1.Secret, error) {
// Note! This is where we could potentially add more handling of OPC spec fields which tell us how // Note! This is where we could potentially add more handling of FederationDomain spec fields which tell us how
// this OIDC provider should sign and verify ID tokens (e.g., hardcoded token secret, gRPC // this FederationDomain should sign and verify ID tokens (e.g., hardcoded token secret, gRPC
// connection to KMS, etc). // connection to KMS, etc).
// //
// For now, we just generate an new RSA keypair and put that in the secret. // 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{ s := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: opc.Name + "-jwks", Name: federationDomain.Name + "-jwks",
Namespace: opc.Namespace, Namespace: federationDomain.Namespace,
Labels: c.jwksSecretLabels, Labels: c.jwksSecretLabels,
OwnerReferences: []metav1.OwnerReference{ OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(opc, schema.GroupVersionKind{ *metav1.NewControllerRef(federationDomain, schema.GroupVersionKind{
Group: configv1alpha1.SchemeGroupVersion.Group, Group: configv1alpha1.SchemeGroupVersion.Group,
Version: configv1alpha1.SchemeGroupVersion.Version, 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, ctx context.Context,
newOPC *configv1alpha1.FederationDomain, newFederationDomain *configv1alpha1.FederationDomain,
) error { ) error {
opcClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newOPC.Namespace) federationDomainClient := c.pinnipedClient.ConfigV1alpha1().FederationDomains(newFederationDomain.Namespace)
return retry.RetryOnConflict(retry.DefaultRetry, func() error { 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 { 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 newFederationDomain.Status.Secrets.JWKS.Name == oldFederationDomain.Status.Secrets.JWKS.Name {
// If the existing OPC is up to date, we don't need to update it. // If the existing FederationDomain is up to date, we don't need to update it.
return nil return nil
} }
oldOPC.Status.Secrets.JWKS.Name = newOPC.Status.Secrets.JWKS.Name oldFederationDomain.Status.Secrets.JWKS.Name = newFederationDomain.Status.Secrets.JWKS.Name
_, err = opcClient.Update(ctx, oldOPC, metav1.UpdateOptions{}) _, err = federationDomainClient.Update(ctx, oldFederationDomain, metav1.UpdateOptions{})
return err return err
}) })
} }
// isOPCControlle returns whether the provided obj is controlled by an OPC. // isFederationDomainControlle returns whether the provided obj is controlled by a FederationDomain.
func isOPCControllee(obj metav1.Object) bool { func isFederationDomainControllee(obj metav1.Object) bool {
controller := metav1.GetControllerOf(obj) controller := metav1.GetControllerOf(obj)
return controller != nil && return controller != nil &&
controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && 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. // isValid returns whether the provided secret contains a valid active JWK and verification JWKS.

284
internal/controller/supervisorconfig/jwks_writer_test.go
View File

@ -145,7 +145,7 @@ func TestJWKSWriterControllerFilterSecret(t *testing.T) {
kubernetesfake.NewSimpleClientset(), kubernetesfake.NewSimpleClientset(),
0, 0,
).Core().V1().Secrets() ).Core().V1().Secrets()
opcInformer := pinnipedinformers.NewSharedInformerFactory( federationDomainInformer := pinnipedinformers.NewSharedInformerFactory(
pinnipedfake.NewSimpleClientset(), pinnipedfake.NewSimpleClientset(),
0, 0,
).Config().V1alpha1().FederationDomains() ).Config().V1alpha1().FederationDomains()
@ -155,7 +155,7 @@ func TestJWKSWriterControllerFilterSecret(t *testing.T) {
nil, // kubeClient, not needed nil, // kubeClient, not needed
nil, // pinnipedClient, not needed nil, // pinnipedClient, not needed
secretInformer, secretInformer,
opcInformer, federationDomainInformer,
withInformer.WithInformer, withInformer.WithInformer,
) )
@ -170,24 +170,24 @@ func TestJWKSWriterControllerFilterSecret(t *testing.T) {
} }
} }
func TestJWKSWriterControllerFilterOPC(t *testing.T) { func TestJWKSWriterControllerFilterFederationDomain(t *testing.T) {
t.Parallel() t.Parallel()
tests := []struct { tests := []struct {
name string name string
opc configv1alpha1.FederationDomain federationDomain configv1alpha1.FederationDomain
wantAdd bool wantAdd bool
wantUpdate bool wantUpdate bool
wantDelete bool wantDelete bool
wantParent controllerlib.Key wantParent controllerlib.Key
}{ }{
{ {
name: "anything goes", name: "anything goes",
opc: configv1alpha1.FederationDomain{}, federationDomain: configv1alpha1.FederationDomain{},
wantAdd: true, wantAdd: true,
wantUpdate: true, wantUpdate: true,
wantDelete: true, wantDelete: true,
wantParent: controllerlib.Key{}, wantParent: controllerlib.Key{},
}, },
} }
for _, test := range tests { for _, test := range tests {
@ -199,7 +199,7 @@ func TestJWKSWriterControllerFilterOPC(t *testing.T) {
kubernetesfake.NewSimpleClientset(), kubernetesfake.NewSimpleClientset(),
0, 0,
).Core().V1().Secrets() ).Core().V1().Secrets()
opcInformer := pinnipedinformers.NewSharedInformerFactory( federationDomainInformer := pinnipedinformers.NewSharedInformerFactory(
pinnipedfake.NewSimpleClientset(), pinnipedfake.NewSimpleClientset(),
0, 0,
).Config().V1alpha1().FederationDomains() ).Config().V1alpha1().FederationDomains()
@ -209,17 +209,17 @@ func TestJWKSWriterControllerFilterOPC(t *testing.T) {
nil, // kubeClient, not needed nil, // kubeClient, not needed
nil, // pinnipedClient, not needed nil, // pinnipedClient, not needed
secretInformer, secretInformer,
opcInformer, federationDomainInformer,
withInformer.WithInformer, withInformer.WithInformer,
) )
unrelated := configv1alpha1.FederationDomain{} unrelated := configv1alpha1.FederationDomain{}
filter := withInformer.GetFilterForInformer(opcInformer) filter := withInformer.GetFilterForInformer(federationDomainInformer)
require.Equal(t, test.wantAdd, filter.Add(&test.opc)) require.Equal(t, test.wantAdd, filter.Add(&test.federationDomain))
require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.opc)) require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.federationDomain))
require.Equal(t, test.wantUpdate, filter.Update(&test.opc, &unrelated)) require.Equal(t, test.wantUpdate, filter.Update(&test.federationDomain, &unrelated))
require.Equal(t, test.wantDelete, filter.Delete(&test.opc)) require.Equal(t, test.wantDelete, filter.Delete(&test.federationDomain))
require.Equal(t, test.wantParent, filter.Parent(&test.opc)) require.Equal(t, test.wantParent, filter.Parent(&test.federationDomain))
}) })
} }
} }
@ -236,24 +236,24 @@ func TestJWKSWriterControllerSync(t *testing.T) {
goodKey, err := x509.ParseECPrivateKey(block.Bytes) goodKey, err := x509.ParseECPrivateKey(block.Bytes)
require.NoError(t, err) require.NoError(t, err)
opcGVR := schema.GroupVersionResource{ federationDomainGVR := schema.GroupVersionResource{
Group: configv1alpha1.SchemeGroupVersion.Group, Group: configv1alpha1.SchemeGroupVersion.Group,
Version: configv1alpha1.SchemeGroupVersion.Version, Version: configv1alpha1.SchemeGroupVersion.Version,
Resource: "federationdomains", Resource: "federationdomains",
} }
goodOPC := &configv1alpha1.FederationDomain{ goodFederationDomain := &configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "good-opc", Name: "good-federationDomain",
Namespace: namespace, Namespace: namespace,
UID: "good-opc-uid", UID: "good-federationDomain-uid",
}, },
Spec: configv1alpha1.FederationDomainSpec{ Spec: configv1alpha1.FederationDomainSpec{
Issuer: "https://some-issuer.com", Issuer: "https://some-issuer.com",
}, },
} }
goodOPCWithStatus := goodOPC.DeepCopy() goodFederationDomainWithStatus := goodFederationDomain.DeepCopy()
goodOPCWithStatus.Status.Secrets.JWKS.Name = goodOPCWithStatus.Name + "-jwks" goodFederationDomainWithStatus.Status.Secrets.JWKS.Name = goodFederationDomainWithStatus.Name + "-jwks"
secretGVR := schema.GroupVersionResource{ secretGVR := schema.GroupVersionResource{
Group: corev1.SchemeGroupVersion.Group, Group: corev1.SchemeGroupVersion.Group,
@ -264,7 +264,7 @@ func TestJWKSWriterControllerSync(t *testing.T) {
newSecret := func(activeJWKPath, jwksPath string) *corev1.Secret { newSecret := func(activeJWKPath, jwksPath string) *corev1.Secret {
s := corev1.Secret{ s := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: goodOPCWithStatus.Status.Secrets.JWKS.Name, Name: goodFederationDomainWithStatus.Status.Secrets.JWKS.Name,
Namespace: namespace, Namespace: namespace,
Labels: map[string]string{ Labels: map[string]string{
"myLabelKey1": "myLabelValue1", "myLabelKey1": "myLabelValue1",
@ -272,10 +272,10 @@ func TestJWKSWriterControllerSync(t *testing.T) {
}, },
OwnerReferences: []metav1.OwnerReference{ OwnerReferences: []metav1.OwnerReference{
{ {
APIVersion: opcGVR.GroupVersion().String(), APIVersion: federationDomainGVR.GroupVersion().String(),
Kind: "FederationDomain", Kind: "FederationDomain",
Name: goodOPC.Name, Name: goodFederationDomain.Name,
UID: goodOPC.UID, UID: goodFederationDomain.UID,
BlockOwnerDeletion: boolPtr(true), BlockOwnerDeletion: boolPtr(true),
Controller: boolPtr(true), Controller: boolPtr(true),
}, },
@ -295,39 +295,39 @@ func TestJWKSWriterControllerSync(t *testing.T) {
goodSecret := newSecret("testdata/good-jwk.json", "testdata/good-jwks.json") goodSecret := newSecret("testdata/good-jwk.json", "testdata/good-jwks.json")
tests := []struct { tests := []struct {
name string name string
key controllerlib.Key key controllerlib.Key
secrets []*corev1.Secret secrets []*corev1.Secret
configKubeClient func(*kubernetesfake.Clientset) configKubeClient func(*kubernetesfake.Clientset)
configPinnipedClient func(*pinnipedfake.Clientset) configPinnipedClient func(*pinnipedfake.Clientset)
opcs []*configv1alpha1.FederationDomain federationDomains []*configv1alpha1.FederationDomain
generateKeyErr error generateKeyErr error
wantGenerateKeyCount int wantGenerateKeyCount int
wantSecretActions []kubetesting.Action wantSecretActions []kubetesting.Action
wantOPCActions []kubetesting.Action wantFederationDomainActions []kubetesting.Action
wantError string wantError string
}{ }{
{ {
name: "new opc with no secret", name: "new federationDomain with no secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
wantGenerateKeyCount: 1, wantGenerateKeyCount: 1,
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), kubetesting.NewCreateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opcGVR, namespace, goodOPCWithStatus), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithStatus),
}, },
}, },
{ {
name: "opc without status with existing secret", name: "federationDomain without status with existing secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
goodSecret, goodSecret,
@ -336,46 +336,46 @@ func TestJWKSWriterControllerSync(t *testing.T) {
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
kubetesting.NewUpdateAction(opcGVR, namespace, goodOPCWithStatus), kubetesting.NewUpdateAction(federationDomainGVR, namespace, goodFederationDomainWithStatus),
}, },
}, },
{ {
name: "existing opc with no secret", name: "existing federationDomain with no secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
wantGenerateKeyCount: 1, wantGenerateKeyCount: 1,
wantSecretActions: []kubetesting.Action{ wantSecretActions: []kubetesting.Action{
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), kubetesting.NewCreateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "existing opc with existing secret", name: "existing federationDomain with existing secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
goodSecret, goodSecret,
}, },
}, },
{ {
name: "deleted opc", name: "deleted federationDomain",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
// Nothing to do here since Kube will garbage collect our child secret via its OwnerReference. // Nothing to do here since Kube will garbage collect our child secret via its OwnerReference.
}, },
{ {
name: "missing jwk in secret", name: "missing jwk in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("", "testdata/good-jwks.json"), newSecret("", "testdata/good-jwks.json"),
@ -385,15 +385,15 @@ func TestJWKSWriterControllerSync(t *testing.T) {
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "missing jwks in secret", name: "missing jwks in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/good-jwk.json", ""), newSecret("testdata/good-jwk.json", ""),
@ -403,15 +403,15 @@ func TestJWKSWriterControllerSync(t *testing.T) {
kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "invalid jwk JSON in secret", name: "invalid jwk JSON in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/not-json.txt", "testdata/good-jwks.json"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "invalid jwks JSON in secret", name: "invalid jwks JSON in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/good-jwk.json", "testdata/not-json.txt"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "public jwk in secret", name: "public jwk in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/public-jwk.json", "testdata/good-jwks.json"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "private jwks in secret", name: "private jwks in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/good-jwk.json", "testdata/private-jwks.json"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "invalid jwk key in secret", name: "invalid jwk key in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/invalid-key-jwk.json", "testdata/good-jwks.json"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "invalid jwks key in secret", name: "invalid jwks key in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/good-jwk.json", "testdata/invalid-key-jwks.json"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "missing active jwks in secret", name: "missing active jwks in secret",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("testdata/good-jwk.json", "testdata/missing-active-jwks.json"), 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.NewGetAction(secretGVR, namespace, goodSecret.Name),
kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret),
}, },
wantOPCActions: []kubetesting.Action{ wantFederationDomainActions: []kubetesting.Action{
kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), kubetesting.NewGetAction(federationDomainGVR, namespace, goodFederationDomain.Name),
}, },
}, },
{ {
name: "generate key fails", name: "generate key fails",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPCWithStatus, goodFederationDomainWithStatus,
}, },
generateKeyErr: errors.New("some generate error"), generateKeyErr: errors.New("some generate error"),
wantError: "cannot generate secret: cannot generate key: some generate error", wantError: "cannot generate secret: cannot generate key: some generate error",
}, },
{ {
name: "get secret fails", name: "get secret fails",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
configKubeClient: func(client *kubernetesfake.Clientset) { configKubeClient: func(client *kubernetesfake.Clientset) {
client.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { client.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
@ -557,9 +557,9 @@ func TestJWKSWriterControllerSync(t *testing.T) {
}, },
{ {
name: "create secret fails", name: "create secret fails",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
configKubeClient: func(client *kubernetesfake.Clientset) { configKubeClient: func(client *kubernetesfake.Clientset) {
client.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { client.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
@ -570,9 +570,9 @@ func TestJWKSWriterControllerSync(t *testing.T) {
}, },
{ {
name: "update secret fails", name: "update secret fails",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
secrets: []*corev1.Secret{ secrets: []*corev1.Secret{
newSecret("", ""), newSecret("", ""),
@ -585,30 +585,30 @@ func TestJWKSWriterControllerSync(t *testing.T) {
wantError: "cannot create or update secret: some update error", wantError: "cannot create or update secret: some update error",
}, },
{ {
name: "get opc fails", name: "get FederationDomain fails",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
configPinnipedClient: func(client *pinnipedfake.Clientset) { configPinnipedClient: func(client *pinnipedfake.Clientset) {
client.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { client.PrependReactor("get", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some get 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", name: "update federationDomain fails",
key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, key: controllerlib.Key{Namespace: goodFederationDomain.Namespace, Name: goodFederationDomain.Name},
opcs: []*configv1alpha1.FederationDomain{ federationDomains: []*configv1alpha1.FederationDomain{
goodOPC, goodFederationDomain,
}, },
configPinnipedClient: func(client *pinnipedfake.Clientset) { configPinnipedClient: func(client *pinnipedfake.Clientset) {
client.PrependReactor("update", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) { client.PrependReactor("update", "federationdomains", func(_ kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some update 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 { for _, test := range tests {
@ -636,9 +636,9 @@ func TestJWKSWriterControllerSync(t *testing.T) {
pinnipedAPIClient := pinnipedfake.NewSimpleClientset() pinnipedAPIClient := pinnipedfake.NewSimpleClientset()
pinnipedInformerClient := pinnipedfake.NewSimpleClientset() pinnipedInformerClient := pinnipedfake.NewSimpleClientset()
for _, opc := range test.opcs { for _, federationDomain := range test.federationDomains {
require.NoError(t, pinnipedAPIClient.Tracker().Add(opc)) require.NoError(t, pinnipedAPIClient.Tracker().Add(federationDomain))
require.NoError(t, pinnipedInformerClient.Tracker().Add(opc)) require.NoError(t, pinnipedInformerClient.Tracker().Add(federationDomain))
} }
if test.configPinnipedClient != nil { if test.configPinnipedClient != nil {
test.configPinnipedClient(pinnipedAPIClient) test.configPinnipedClient(pinnipedAPIClient)
@ -685,8 +685,8 @@ func TestJWKSWriterControllerSync(t *testing.T) {
if test.wantSecretActions != nil { if test.wantSecretActions != nil {
require.Equal(t, test.wantSecretActions, kubeAPIClient.Actions()) require.Equal(t, test.wantSecretActions, kubeAPIClient.Actions())
} }
if test.wantOPCActions != nil { if test.wantFederationDomainActions != nil {
require.Equal(t, test.wantOPCActions, pinnipedAPIClient.Actions()) require.Equal(t, test.wantFederationDomainActions, pinnipedAPIClient.Actions())
} }
}) })
} }

19
internal/oidc/provider/oidcprovider.go → internal/oidc/provider/federation_domain_issuer.go
View File

@ -11,15 +11,16 @@ import (
"go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/constable"
) )
// FederationDomain represents all of the settings and state for an OIDC provider. // FederationDomainIssuer represents all of the settings and state for a downstream OIDC provider
type FederationDomain struct { // as defined by a FederationDomain.
type FederationDomainIssuer struct {
issuer string issuer string
issuerHost string issuerHost string
issuerPath string issuerPath string
} }
func NewFederationDomain(issuer string) (*FederationDomain, error) { func NewFederationDomainIssuer(issuer string) (*FederationDomainIssuer, error) {
p := FederationDomain{issuer: issuer} p := FederationDomainIssuer{issuer: issuer}
err := p.validate() err := p.validate()
if err != nil { if err != nil {
return nil, err return nil, err
@ -27,9 +28,9 @@ func NewFederationDomain(issuer string) (*FederationDomain, error) {
return &p, nil return &p, nil
} }
func (p *FederationDomain) validate() error { func (p *FederationDomainIssuer) validate() error {
if p.issuer == "" { 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) issuerURL, err := url.Parse(p.issuer)
@ -63,14 +64,14 @@ func (p *FederationDomain) validate() error {
return nil return nil
} }
func (p *FederationDomain) Issuer() string { func (p *FederationDomainIssuer) Issuer() string {
return p.issuer return p.issuer
} }
func (p *FederationDomain) IssuerHost() string { func (p *FederationDomainIssuer) IssuerHost() string {
return p.issuerHost return p.issuerHost
} }
func (p *FederationDomain) IssuerPath() string { func (p *FederationDomainIssuer) IssuerPath() string {
return p.issuerPath return p.issuerPath
} }

8
internal/oidc/provider/oidcprovider_test.go → internal/oidc/provider/federation_domain_issuer_test.go
View File

@ -9,16 +9,16 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestFederationDomainValidations(t *testing.T) { func TestFederationDomainIssuerValidations(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
issuer string issuer string
wantError string wantError string
}{ }{
{ {
name: "provider must have an issuer", name: "must have an issuer",
issuer: "", issuer: "",
wantError: "provider must have an issuer", wantError: "federation domain must have an issuer",
}, },
{ {
name: "no scheme", name: "no scheme",
@ -72,7 +72,7 @@ func TestFederationDomainValidations(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
tt := tt tt := tt
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
_, err := NewFederationDomain(tt.issuer) _, err := NewFederationDomainIssuer(tt.issuer)
if tt.wantError != "" { if tt.wantError != "" {
require.EqualError(t, err, tt.wantError) require.EqualError(t, err, tt.wantError)
} else { } else {

6
internal/oidc/provider/manager/manager.go
View File

@ -32,7 +32,7 @@ import (
// It is thread-safe. // It is thread-safe.
type Manager struct { type Manager struct {
mu sync.RWMutex mu sync.RWMutex
providers []*provider.FederationDomain providers []*provider.FederationDomainIssuer
providerHandlers map[string]http.Handler // map of all routes for all providers 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 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 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 // It also removes any providerHandlers that were previously added but were not passed in to
// the current invocation. // 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. // 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() m.mu.Lock()
defer m.mu.Unlock() defer m.mu.Unlock()

8
internal/oidc/provider/manager/manager_test.go
View File

@ -348,9 +348,9 @@ func TestManager(t *testing.T) {
when("given some valid providers via SetProviders()", func() { when("given some valid providers via SetProviders()", func() {
it.Before(func() { it.Before(func() {
p1, err := provider.NewFederationDomain(issuer1) p1, err := provider.NewFederationDomainIssuer(issuer1)
r.NoError(err) r.NoError(err)
p2, err := provider.NewFederationDomain(issuer2) p2, err := provider.NewFederationDomainIssuer(issuer2)
r.NoError(err) r.NoError(err)
subject.SetProviders(p1, p2) 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() { when("given the same valid providers as arguments to SetProviders() in reverse order", func() {
it.Before(func() { it.Before(func() {
p1, err := provider.NewFederationDomain(issuer1) p1, err := provider.NewFederationDomainIssuer(issuer1)
r.NoError(err) r.NoError(err)
p2, err := provider.NewFederationDomain(issuer2) p2, err := provider.NewFederationDomainIssuer(issuer2)
r.NoError(err) r.NoError(err)
subject.SetProviders(p2, p1) subject.SetProviders(p2, p1)

8
test/integration/supervisor_discovery_test.go
View File

@ -595,17 +595,17 @@ func requireStatus(t *testing.T, client pinnipedclientset.Interface, ns, name st
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel() defer cancel()
var opc *v1alpha1.FederationDomain var federationDomain *v1alpha1.FederationDomain
var err error var err error
assert.Eventually(t, func() bool { 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 { if err != nil {
t.Logf("error trying to get FederationDomain: %s", err.Error()) 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) }, time.Minute, 200*time.Millisecond)
require.NoError(t, err) 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 { func newHTTPClient(t *testing.T, caBundle string, dnsOverrides map[string]string) *http.Client {

20
test/library/client.go
View File

@ -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)) issuer = fmt.Sprintf("http://test-issuer-%s.pinniped.dev", RandHex(t, 8))
} }
opcs := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) federationDomains := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace)
opc, err := opcs.Create(createContext, &configv1alpha1.FederationDomain{ federationDomain, err := federationDomains.Create(createContext, &configv1alpha1.FederationDomain{
ObjectMeta: testObjectMeta(t, "oidc-provider"), ObjectMeta: testObjectMeta(t, "oidc-provider"),
Spec: configv1alpha1.FederationDomainSpec{ Spec: configv1alpha1.FederationDomainSpec{
Issuer: issuer, Issuer: issuer,
@ -258,31 +258,31 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string
}, },
}, metav1.CreateOptions{}) }, metav1.CreateOptions{})
require.NoError(t, err, "could not create test FederationDomain") 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.Cleanup(func() {
t.Helper() 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) deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel() defer cancel()
err := opcs.Delete(deleteCtx, opc.Name, metav1.DeleteOptions{}) err := federationDomains.Delete(deleteCtx, federationDomain.Name, metav1.DeleteOptions{})
notFound := k8serrors.IsNotFound(err) notFound := k8serrors.IsNotFound(err)
// It's okay if it is not found, because it might have been deleted by another part of this test. // It's okay if it is not found, because it might have been deleted by another part of this test.
if !notFound { 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 we're not expecting any particular status, just return the new FederationDomain immediately.
if expectStatus == "" { if expectStatus == "" {
return opc return federationDomain
} }
// Wait for the FederationDomain to enter the expected phase (or time out). // Wait for the FederationDomain to enter the expected phase (or time out).
var result *configv1alpha1.FederationDomain var result *configv1alpha1.FederationDomain
assert.Eventuallyf(t, func() bool { assert.Eventuallyf(t, func() bool {
var err error 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) require.NoError(t, err)
return result.Status.Status == expectStatus return result.Status.Status == expectStatus
}, 60*time.Second, 1*time.Second, "expected the FederationDomain to have status %q", 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 { if result.Status.Status == configv1alpha1.SuccessFederationDomainStatusCondition {
assert.Eventually(t, func() bool { assert.Eventually(t, func() bool {
var err error 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) require.NoError(t, err)
return result.Status.Secrets.JWKS.Name != "" && return result.Status.Secrets.JWKS.Name != "" &&
result.Status.Secrets.TokenSigningKey.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.StateSigningKey.Name)
require.NotEmpty(t, result.Status.Secrets.StateEncryptionKey.Name) require.NotEmpty(t, result.Status.Secrets.StateEncryptionKey.Name)
} }
return opc return federationDomain
} }
func RandHex(t *testing.T, numBytes int) string { func RandHex(t *testing.T, numBytes int) string {