diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 870feb0e9..f2e1127db 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -18,6 +18,7 @@ import ( coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/go-jose/go-jose/v4" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -137,6 +138,7 @@ func New( jwtAuthenticators authinformers.JWTAuthenticatorInformer, secretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, clock clock.Clock, log plog.Logger, ) controllerlib.Controller { @@ -154,11 +156,27 @@ func New( log: log.WithName(controllerName), }, }, - controllerlib.WithInformer( + withInformer( jwtAuthenticators, pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct controllerlib.InformerOption{}, ), + withInformer( + secretInformer, + pinnipedcontroller.MatchAnySecretOfTypesFilter( + []corev1.SecretType{ + corev1.SecretTypeOpaque, + corev1.SecretTypeTLS, + }, + pinnipedcontroller.SingletonQueue(), + ), // nil parent func is fine because each event is distinct + controllerlib.InformerOption{}, + ), + withInformer( + configMapInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), ) } diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 0d1a3872a..048270c46 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -26,12 +26,14 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" + k8sinformers "k8s.io/client-go/informers" kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" @@ -1845,6 +1847,7 @@ func TestController(t *testing.T) { } pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(), 0) + observableInformers := testutil.NewObservableWithInformerOption() cache := authncache.New() var log bytes.Buffer @@ -1861,6 +1864,7 @@ func TestController(t *testing.T) { pinnipedInformers.Authentication().V1alpha1().JWTAuthenticators(), kubeInformers.Core().V1().Secrets(), kubeInformers.Core().V1().ConfigMaps(), + observableInformers.WithInformer, frozenClock, logger) @@ -1868,6 +1872,8 @@ func TestController(t *testing.T) { defer cancel() pinnipedInformers.Start(ctx.Done()) + kubeInformers.Start(ctx.Done()) + controllerlib.TestRunSynchronously(t, controller) syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey} @@ -2275,3 +2281,149 @@ func newCacheValue(t *testing.T, spec authenticationv1alpha1.JWTAuthenticatorSpe }, } } + +func TestControllerFilterSecret(t *testing.T) { + tests := []struct { + name string + secret metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "should return true for a secret of the type Opaque", + secret: &corev1.Secret{ + Type: corev1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return true for a secret of the type TLS", + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return false for a secret of the wrong type", + secret: &corev1.Secret{ + Type: "other-type", + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, + }, + { + name: "should return false for a resource of wrong data type", + secret: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) + frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + + kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + pinnipedAPIClient := conciergefake.NewSimpleClientset() + pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) + observableInformers := testutil.NewObservableWithInformerOption() + + _ = New( + "concierge", // namespace for the controller + authncache.New(), + pinnipedAPIClient, + pinnipedInformers.Authentication().V1alpha1().JWTAuthenticators(), + secretInformer, + configMapInformer, + observableInformers.WithInformer, + frozenClock, + logger) + + unrelated := &corev1.Secret{} + filter := observableInformers.GetFilterForInformer(secretInformer) + require.Equal(t, tt.wantAdd, filter.Add(tt.secret)) + require.Equal(t, tt.wantUpdate, filter.Update(unrelated, tt.secret)) + require.Equal(t, tt.wantUpdate, filter.Update(tt.secret, unrelated)) + require.Equal(t, tt.wantDelete, filter.Delete(tt.secret)) + }) + } +} + +func TestControllerFilterConfigMap(t *testing.T) { + namespace := "some-namespace" + goodCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, + } + + tests := []struct { + name string + cm metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "a configMap in the right namespace", + cm: goodCM, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) + frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + + kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + pinnipedAPIClient := conciergefake.NewSimpleClientset() + pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) + observableInformers := testutil.NewObservableWithInformerOption() + + _ = New( + "concierge", // namespace for the controller + authncache.New(), + pinnipedAPIClient, + pinnipedInformers.Authentication().V1alpha1().JWTAuthenticators(), + secretInformer, + configMapInformer, + observableInformers.WithInformer, + frozenClock, + logger) + + unrelated := &corev1.ConfigMap{} + filter := observableInformers.GetFilterForInformer(configMapInformer) + require.Equal(t, tt.wantAdd, filter.Add(tt.cm)) + require.Equal(t, tt.wantUpdate, filter.Update(unrelated, tt.cm)) + require.Equal(t, tt.wantUpdate, filter.Update(tt.cm, unrelated)) + require.Equal(t, tt.wantDelete, filter.Delete(tt.cm)) + }) + } +} diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index f3615eae3..5601b8b85 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -73,6 +73,7 @@ func New( webhooks authinformers.WebhookAuthenticatorInformer, secretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, clock clock.Clock, log plog.Logger, ) controllerlib.Controller { @@ -90,12 +91,12 @@ func New( log: log.WithName(controllerName), }, }, - controllerlib.WithInformer( + withInformer( webhooks, pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct controllerlib.InformerOption{}, ), - controllerlib.WithInformer( + withInformer( secretInformer, pinnipedcontroller.MatchAnySecretOfTypesFilter( []corev1.SecretType{ @@ -106,7 +107,7 @@ func New( ), // nil parent func is fine because each event is distinct controllerlib.InformerOption{}, ), - controllerlib.WithInformer( + withInformer( configMapInformer, pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index 49ab602e3..2c1353861 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -21,10 +21,12 @@ import ( "github.com/stretchr/testify/require" authenticationv1beta1 "k8s.io/api/authentication/v1beta1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/authentication/authenticator" + k8sinformers "k8s.io/client-go/informers" kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" @@ -1491,6 +1493,7 @@ func TestController(t *testing.T) { } informers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(), 0) + observableInformers := testutil.NewObservableWithInformerOption() cache := authncache.New() var log bytes.Buffer @@ -1507,6 +1510,7 @@ func TestController(t *testing.T) { informers.Authentication().V1alpha1().WebhookAuthenticators(), kubeInformers.Core().V1().Secrets(), kubeInformers.Core().V1().ConfigMaps(), + observableInformers.WithInformer, frozenClock, logger) @@ -1514,6 +1518,7 @@ func TestController(t *testing.T) { defer cancel() informers.Start(ctx.Done()) + kubeInformers.Start(ctx.Done()) controllerlib.TestRunSynchronously(t, controller) syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey} @@ -1677,3 +1682,149 @@ func TestNewWebhookAuthenticator(t *testing.T) { }) } } + +func TestControllerFilterSecret(t *testing.T) { + tests := []struct { + name string + secret metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "should return true for a secret of the type Opaque", + secret: &corev1.Secret{ + Type: corev1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return true for a secret of the type TLS", + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return false for a secret of the wrong type", + secret: &corev1.Secret{ + Type: "other-type", + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, + }, + { + name: "should return false for a resource of wrong data type", + secret: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) + frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + + kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + pinnipedAPIClient := conciergefake.NewSimpleClientset() + pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) + observableInformers := testutil.NewObservableWithInformerOption() + + _ = New( + "concierge", // namespace for controller + authncache.New(), + pinnipedAPIClient, + pinnipedInformers.Authentication().V1alpha1().WebhookAuthenticators(), + secretInformer, + configMapInformer, + observableInformers.WithInformer, + frozenClock, + logger) + + unrelated := &corev1.Secret{} + filter := observableInformers.GetFilterForInformer(secretInformer) + require.Equal(t, tt.wantAdd, filter.Add(tt.secret)) + require.Equal(t, tt.wantUpdate, filter.Update(unrelated, tt.secret)) + require.Equal(t, tt.wantUpdate, filter.Update(tt.secret, unrelated)) + require.Equal(t, tt.wantDelete, filter.Delete(tt.secret)) + }) + } +} + +func TestControllerFilterConfigMap(t *testing.T) { + namespace := "some-namespace" + goodCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, + } + + tests := []struct { + name string + cm metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "a configMap in the right namespace", + cm: goodCM, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) + frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + + kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + pinnipedAPIClient := conciergefake.NewSimpleClientset() + pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) + observableInformers := testutil.NewObservableWithInformerOption() + + _ = New( + "concierge", // namespace for the controller + authncache.New(), + pinnipedAPIClient, + pinnipedInformers.Authentication().V1alpha1().WebhookAuthenticators(), + secretInformer, + configMapInformer, + observableInformers.WithInformer, + frozenClock, + logger) + + unrelated := &corev1.ConfigMap{} + filter := observableInformers.GetFilterForInformer(configMapInformer) + require.Equal(t, tt.wantAdd, filter.Add(tt.cm)) + require.Equal(t, tt.wantUpdate, filter.Update(unrelated, tt.cm)) + require.Equal(t, tt.wantUpdate, filter.Update(tt.cm, unrelated)) + require.Equal(t, tt.wantDelete, filter.Delete(tt.cm)) + }) + } +} diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index 250124a2e..0248fa3b0 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -2302,7 +2302,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { wantDelete: true, }, { - name: "should return false for a secret of the type Opaque", + name: "should return true for a secret of the type Opaque", secret: func() *corev1.Secret { otherSecret := goodSecret.DeepCopy() otherSecret.Type = corev1.SecretTypeOpaque @@ -2313,7 +2313,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { wantDelete: true, }, { - name: "should return false for a secret of the type TLS", + name: "should return true for a secret of the type TLS", secret: func() *corev1.Secret { otherSecret := goodSecret.DeepCopy() otherSecret.Type = corev1.SecretTypeTLS diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 7dc14c629..6d5144216 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -241,6 +241,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol informers.pinniped.Authentication().V1alpha1().WebhookAuthenticators(), informers.installationNamespaceK8s.Core().V1().Secrets(), informers.installationNamespaceK8s.Core().V1().ConfigMaps(), + controllerlib.WithInformer, clock.RealClock{}, plog.New(), ), @@ -254,6 +255,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol informers.pinniped.Authentication().V1alpha1().JWTAuthenticators(), informers.installationNamespaceK8s.Core().V1().Secrets(), informers.installationNamespaceK8s.Core().V1().ConfigMaps(), + controllerlib.WithInformer, clock.RealClock{}, plog.New(), ), diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 726d31813..089a3bbe3 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -760,7 +760,6 @@ func TestSupervisorLogin_Browser(t *testing.T) { maybeSkip: skipLDAPTests, createIDP: func(t *testing.T) string { idp, _ := createLDAPIdentityProvider(t, func(spec *idpv1alpha1.LDAPIdentityProviderSpec) { - caConfigMap := testlib.CreateTestConfigMap(t, env.SupervisorNamespace, "ca-cert", map[string]string{ "ca.crt": env.SupervisorUpstreamLDAP.CABundle,