diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index b7813a9fb..1a4865eed 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -14,6 +14,7 @@ import ( "github.com/go-ldap/ldap/v3" "github.com/google/uuid" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -290,7 +291,18 @@ func newInternal( ), withInformer( secretInformer, - pinnipedcontroller.MatchAnySecretOfTypeFilter(upstreamwatchers.LDAPBindAccountSecretType, pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.MatchAnySecretOfTypesFilter( + []corev1.SecretType{ + upstreamwatchers.LDAPBindAccountSecretType, + corev1.SecretTypeOpaque, + corev1.SecretTypeTLS, + }, + pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + configMapInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), ) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index 78fd5380c..e6007c65e 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -47,7 +47,7 @@ func TestActiveDirectoryUpstreamWatcherControllerFilterSecrets(t *testing.T) { wantDelete bool }{ { - name: "a secret of the right type", + name: "should return true for a secret of type BasicAuth", secret: &corev1.Secret{ Type: corev1.SecretTypeBasicAuth, ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, @@ -57,14 +57,34 @@ func TestActiveDirectoryUpstreamWatcherControllerFilterSecrets(t *testing.T) { wantDelete: true, }, { - name: "a secret of the wrong type", + name: "should return true for a secret of type TLS", + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return true for a secret of type Opaque", + secret: &corev1.Secret{ + Type: corev1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return false for a secret of the wrong type", secret: &corev1.Secret{ Type: "this-is-the-wrong-type", ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, }, { - name: "resource of a data type which is not watched by this controller", + name: "should return false for a resource of a data type which is not watched by this controller", secret: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, @@ -95,6 +115,51 @@ func TestActiveDirectoryUpstreamWatcherControllerFilterSecrets(t *testing.T) { } } +func TestActiveDirectoryUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cm metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "any configmap", + cm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + fakePinnipedClient := supervisorfake.NewSimpleClientset() + pinnipedInformers := supervisorinformers.NewSharedInformerFactory(fakePinnipedClient, 0) + activeDirectoryIDPInformer := pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders() + fakeKubeClient := fake.NewSimpleClientset() + kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + withInformer := testutil.NewObservableWithInformerOption() + + New(nil, nil, activeDirectoryIDPInformer, secretInformer, configMapInformer, withInformer.WithInformer) + + unrelated := corev1.Secret{} + filter := withInformer.GetFilterForInformer(configMapInformer) + require.Equal(t, test.wantAdd, filter.Add(test.cm)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, test.cm)) + require.Equal(t, test.wantUpdate, filter.Update(test.cm, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(test.cm)) + }) + } +} + func TestActiveDirectoryUpstreamWatcherControllerFilterActiveDirectoryIdentityProviders(t *testing.T) { t.Parallel() diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index c9580d98b..48e812d99 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -31,6 +31,7 @@ import ( pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" @@ -73,6 +74,7 @@ type gitHubWatcherController struct { client supervisorclientset.Interface gitHubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer secretInformer corev1informers.SecretInformer + configMapInformer corev1informers.ConfigMapInformer clock clock.Clock dialFunc func(network, addr string, config *tls.Config) (*tls.Conn, error) } @@ -84,6 +86,7 @@ func New( client supervisorclientset.Interface, gitHubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer, secretInformer corev1informers.SecretInformer, + configMapInformer corev1informers.ConfigMapInformer, log plog.Logger, withInformer pinnipedcontroller.WithInformerOptionFunc, clock clock.Clock, @@ -96,6 +99,7 @@ func New( log: log.WithName(controllerName), gitHubIdentityProviderInformer: gitHubIdentityProviderInformer, secretInformer: secretInformer, + configMapInformer: configMapInformer, clock: clock, dialFunc: dialFunc, } @@ -104,15 +108,24 @@ func New( controllerlib.Config{Name: controllerName, Syncer: &c}, withInformer( gitHubIdentityProviderInformer, - pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { - gitHubIDP, ok := obj.(*idpv1alpha1.GitHubIdentityProvider) - return ok && gitHubIDP.Namespace == namespace - }, pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), withInformer( secretInformer, - pinnipedcontroller.MatchAnySecretOfTypeFilter(gitHubClientSecretType, pinnipedcontroller.SingletonQueue(), namespace), + pinnipedcontroller.MatchAnySecretOfTypesFilter( + []corev1.SecretType{ + gitHubClientSecretType, + corev1.SecretTypeOpaque, + corev1.SecretTypeTLS, + }, + pinnipedcontroller.SingletonQueue(), + ), + controllerlib.InformerOption{}, + ), + withInformer( + configMapInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), ) @@ -201,7 +214,7 @@ func (c *gitHubWatcherController) validateClientSecret(secretName string) (*meta return &metav1.Condition{ Type: ClientCredentialsSecretValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("clientID and clientSecret have been read from spec.client.SecretName (%q)", secretName), }, clientID, clientSecret, nil } @@ -219,7 +232,7 @@ func validateOrganizationsPolicy(organizationsSpec *idpv1alpha1.GitHubOrganizati return &metav1.Condition{ Type: OrganizationsPolicyValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.allowAuthentication.organizations.policy (%q) is valid", policy), } } @@ -354,30 +367,20 @@ func validateHost(gitHubAPIConfig idpv1alpha1.GitHubAPIConfig) (*metav1.Conditio return &metav1.Condition{ Type: HostValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", host), }, &hostPort } func (c *gitHubWatcherController) validateTLSConfiguration(tlsSpec *idpv1alpha1.TLSSpec) (*metav1.Condition, *x509.CertPool) { - certPool, _, buildCertPoolErr := pinnipedcontroller.BuildCertPoolIDP(tlsSpec) - if buildCertPoolErr != nil { - // buildCertPoolErr is not recoverable with a resync. - // It requires user interaction, so do not return the error. - return &metav1.Condition{ - Type: TLSConfigurationValid, - Status: metav1.ConditionFalse, - Reason: "InvalidTLSConfig", - Message: fmt.Sprintf("spec.githubAPI.tls.certificateAuthorityData is not valid: %s", buildCertPoolErr), - }, nil - } + tlsCondition, _, certPool, _ := tlsconfigutil.ValidateTLSConfig( + tlsconfigutil.TLSSpecForSupervisor(tlsSpec), + "spec.githubAPI.tls", + c.namespace, + c.secretInformer, + c.configMapInformer) - return &metav1.Condition{ - Type: TLSConfigurationValid, - Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, - Message: "spec.githubAPI.tls.certificateAuthorityData is valid", - }, certPool + return tlsCondition, certPool } func (c *gitHubWatcherController) validateGitHubConnection( @@ -407,7 +410,7 @@ func (c *gitHubWatcherController) validateGitHubConnection( return &metav1.Condition{ Type: GitHubConnectionValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", hostPort.Endpoint()), }, fmt.Sprintf("https://%s", hostPort.Endpoint()), phttp.Default(certPool), conn.Close() } @@ -471,7 +474,7 @@ func validateUserAndGroupAttributes(upstream *idpv1alpha1.GitHubIdentityProvider return &metav1.Condition{ Type: ClaimsValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "spec.claims are valid", }, groupNameAttribute, usernameAttribute } diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index 5b9df963f..68266001c 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -35,6 +35,7 @@ import ( supervisorinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/certauthority" pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" @@ -167,7 +168,7 @@ func TestController(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", host), } } @@ -193,7 +194,7 @@ func TestController(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "spec.githubAPI.tls.certificateAuthorityData is valid", } } @@ -219,7 +220,7 @@ func TestController(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.allowAuthentication.organizations.policy (%q) is valid", policy), } } @@ -245,7 +246,7 @@ func TestController(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("clientID and clientSecret have been read from spec.client.SecretName (%q)", secretName), } } @@ -276,7 +277,7 @@ func TestController(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "spec.claims are valid", } } @@ -302,7 +303,7 @@ func TestController(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", host), } } @@ -1853,6 +1854,7 @@ func TestController(t *testing.T) { fakeSupervisorClient, gitHubIdentityProviderInformer, kubeInformers.Core().V1().Secrets(), + kubeInformers.Core().V1().ConfigMaps(), logger, controllerlib.WithInformer, frozenClockForLastTransitionTime, @@ -2058,7 +2060,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 333, LastTransitionTime: oneHourAgo, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", goodServerDomain), }, { @@ -2066,7 +2068,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 333, LastTransitionTime: oneHourAgo, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", goodServerDomain), }, { @@ -2074,7 +2076,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 333, LastTransitionTime: oneHourAgo, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, }, { @@ -2082,7 +2084,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 333, LastTransitionTime: oneHourAgo, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "spec.githubAPI.tls.certificateAuthorityData is valid", }, }, @@ -2154,7 +2156,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "spec.claims are valid", }, { @@ -2162,7 +2164,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: `clientID and clientSecret have been read from spec.client.SecretName ("some-secret-name")`, }, { @@ -2170,7 +2172,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", goodServerDomain), }, { @@ -2178,7 +2180,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", goodServerDomain), }, { @@ -2186,7 +2188,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, }, { @@ -2194,7 +2196,7 @@ func TestController_OnlyWantActions(t *testing.T) { Status: metav1.ConditionTrue, ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "spec.githubAPI.tls.certificateAuthorityData is valid", }, }, @@ -2227,6 +2229,7 @@ func TestController_OnlyWantActions(t *testing.T) { fakeSupervisorClient, supervisorInformers.IDP().V1alpha1().GitHubIdentityProviders(), kubeInformers.Core().V1().Secrets(), + kubeInformers.Core().V1().ConfigMaps(), logger, controllerlib.WithInformer, frozenClockForLastTransitionTime, @@ -2273,12 +2276,10 @@ func compareTLSClientConfigWithinHttpClients(t *testing.T, expected *http.Client } func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { - namespace := "some-namespace" goodSecret := &corev1.Secret{ Type: "secrets.pinniped.dev/github-client", ObjectMeta: metav1.ObjectMeta{ - Name: "some-name", - Namespace: namespace, + Name: "some-name", }, } @@ -2290,22 +2291,36 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { wantDelete bool }{ { - name: "a secret of the right type", + name: "should return true for a secret of the type secrets.pinniped.dev/github-client", secret: goodSecret, wantAdd: true, wantUpdate: true, wantDelete: true, }, { - name: "a secret of the right type, but in the wrong namespace", + name: "should return false for a secret of the type Opaque", secret: func() *corev1.Secret { otherSecret := goodSecret.DeepCopy() - otherSecret.Namespace = "other-namespace" + otherSecret.Type = corev1.SecretTypeOpaque return otherSecret }(), + wantAdd: true, + wantUpdate: true, + wantDelete: true, }, { - name: "a secret of the wrong type", + name: "should return false for a secret of the type TLS", + secret: func() *corev1.Secret { + otherSecret := goodSecret.DeepCopy() + otherSecret.Type = corev1.SecretTypeTLS + return otherSecret + }(), + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return false for a secret of the wrong type", secret: func() *corev1.Secret { otherSecret := goodSecret.DeepCopy() otherSecret.Type = "other-type" @@ -2313,7 +2328,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { }(), }, { - name: "resource of wrong data type", + name: "should return false for a resource of wrong data type", secret: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, @@ -2329,14 +2344,16 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { logger := plog.TestLogger(t, &log) secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() observableInformers := testutil.NewObservableWithInformerOption() _ = New( - namespace, + "some-namespace", dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), supervisorfake.NewSimpleClientset(), supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders(), secretInformer, + configMapInformer, logger, observableInformers.WithInformer, clock.RealClock{}, @@ -2353,6 +2370,65 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { } } +// TODO: make this test pass +func TestGitHubUpstreamWatcherControllerFilterConfigMaps(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) + + gitHubIdentityProviderInformer := supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() + observableInformers := testutil.NewObservableWithInformerOption() + + configMapInformer := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().ConfigMaps() + + _ = New( + namespace, + dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), + supervisorfake.NewSimpleClientset(), + gitHubIdentityProviderInformer, + k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().Secrets(), + configMapInformer, + logger, + observableInformers.WithInformer, + clock.RealClock{}, + tls.Dial, + ) + + 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)) + }) + } +} + func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { namespace := "some-namespace" goodIDP := &idpv1alpha1.GitHubIdentityProvider{ @@ -2375,20 +2451,6 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { wantUpdate: true, wantDelete: true, }, - { - name: "IDP in the wrong namespace", - idp: func() metav1.Object { - badIDP := goodIDP.DeepCopy() - badIDP.Namespace = "other-namespace" - return badIDP - }(), - }, - { - name: "resource of wrong data type", - idp: &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{Name: "some-name"}, - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2406,6 +2468,7 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { supervisorfake.NewSimpleClientset(), gitHubIdentityProviderInformer, k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().Secrets(), + k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().ConfigMaps(), logger, observableInformers.WithInformer, clock.RealClock{}, diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index bae4dc8b3..02476cbfb 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -7,6 +7,7 @@ package ldapupstreamwatcher import ( "context" "fmt" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -152,6 +153,7 @@ func New( client supervisorclientset.Interface, ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer, secretInformer corev1informers.SecretInformer, + configMapInformer corev1informers.ConfigMapInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { return newInternal( @@ -163,6 +165,7 @@ func New( client, ldapIdentityProviderInformer, secretInformer, + configMapInformer, withInformer, ) } @@ -175,6 +178,7 @@ func newInternal( client supervisorclientset.Interface, ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer, secretInformer corev1informers.SecretInformer, + configMapInformer corev1informers.ConfigMapInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { c := ldapWatcherController{ @@ -184,6 +188,7 @@ func newInternal( client: client, ldapIdentityProviderInformer: ldapIdentityProviderInformer, secretInformer: secretInformer, + configMapInformer: configMapInformer, } return controllerlib.New( controllerlib.Config{Name: ldapControllerName, Syncer: &c}, @@ -194,7 +199,17 @@ func newInternal( ), withInformer( secretInformer, - pinnipedcontroller.MatchAnySecretOfTypeFilter(upstreamwatchers.LDAPBindAccountSecretType, pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.MatchAnySecretOfTypesFilter( + []corev1.SecretType{ + upstreamwatchers.LDAPBindAccountSecretType, + corev1.SecretTypeOpaque, + corev1.SecretTypeTLS, + }, pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + configMapInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), ) diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index c5f313d4a..05dfdaf62 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -46,7 +46,7 @@ func TestLDAPUpstreamWatcherControllerFilterSecrets(t *testing.T) { wantDelete bool }{ { - name: "a secret of the right type", + name: "should return true for a secret of type BasicAuth", secret: &corev1.Secret{ Type: corev1.SecretTypeBasicAuth, ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, @@ -56,14 +56,34 @@ func TestLDAPUpstreamWatcherControllerFilterSecrets(t *testing.T) { wantDelete: true, }, { - name: "a secret of the wrong type", + name: "should return true for a secret of type Opaque", + secret: &corev1.Secret{ + Type: corev1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return true for a secret of type TLS", + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return false for a secret of the wrong type", secret: &corev1.Secret{ Type: "this-is-the-wrong-type", ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, }, { - name: "resource of a data type which is not watched by this controller", + name: "should return false for a resource of a data type which is not watched by this controller", secret: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, @@ -79,9 +99,10 @@ func TestLDAPUpstreamWatcherControllerFilterSecrets(t *testing.T) { fakeKubeClient := fake.NewSimpleClientset() kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() withInformer := testutil.NewObservableWithInformerOption() - New(nil, nil, ldapIDPInformer, secretInformer, withInformer.WithInformer) + New(nil, nil, ldapIDPInformer, secretInformer, configMapInformer, withInformer.WithInformer) unrelated := corev1.Secret{} filter := withInformer.GetFilterForInformer(secretInformer) @@ -93,6 +114,51 @@ func TestLDAPUpstreamWatcherControllerFilterSecrets(t *testing.T) { } } +func TestLDAPUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cm metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "any configmap", + cm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + fakePinnipedClient := supervisorfake.NewSimpleClientset() + pinnipedInformers := supervisorinformers.NewSharedInformerFactory(fakePinnipedClient, 0) + ldapIDPInformer := pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders() + fakeKubeClient := fake.NewSimpleClientset() + kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + withInformer := testutil.NewObservableWithInformerOption() + + New(nil, nil, ldapIDPInformer, secretInformer, configMapInformer, withInformer.WithInformer) + + unrelated := corev1.ConfigMap{} + filter := withInformer.GetFilterForInformer(configMapInformer) + require.Equal(t, test.wantAdd, filter.Add(test.cm)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, test.cm)) + require.Equal(t, test.wantUpdate, filter.Update(test.cm, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(test.cm)) + }) + } +} + func TestLDAPUpstreamWatcherControllerFilterLDAPIdentityProviders(t *testing.T) { t.Parallel() @@ -123,9 +189,10 @@ func TestLDAPUpstreamWatcherControllerFilterLDAPIdentityProviders(t *testing.T) fakeKubeClient := fake.NewSimpleClientset() kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() withInformer := testutil.NewObservableWithInformerOption() - New(nil, nil, ldapIDPInformer, secretInformer, withInformer.WithInformer) + New(nil, nil, ldapIDPInformer, secretInformer, configMapInformer, withInformer.WithInformer) unrelated := corev1.Secret{} filter := withInformer.GetFilterForInformer(ldapIDPInformer) @@ -1177,6 +1244,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { fakePinnipedClient, pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders(), kubeInformers.Core().V1().Secrets(), + kubeInformers.Core().V1().ConfigMaps(), controllerlib.WithInformer, ) diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index cda75c49f..c711257b5 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -32,6 +32,7 @@ import ( pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/net/phttp" @@ -128,6 +129,7 @@ type oidcWatcherController struct { client supervisorclientset.Interface oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer secretInformer corev1informers.SecretInformer + configMapInformer corev1informers.ConfigMapInformer validatorCache interface { getProvider(*idpv1alpha1.OIDCIdentityProviderSpec) (*coreosoidc.Provider, *http.Client) putProvider(*idpv1alpha1.OIDCIdentityProviderSpec, *coreosoidc.Provider, *http.Client) @@ -140,6 +142,7 @@ func New( client supervisorclientset.Interface, oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer, secretInformer corev1informers.SecretInformer, + configMapInformer corev1informers.ConfigMapInformer, log plog.Logger, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { @@ -149,6 +152,7 @@ func New( client: client, oidcIdentityProviderInformer: oidcIdentityProviderInformer, secretInformer: secretInformer, + configMapInformer: configMapInformer, validatorCache: &lruValidatorCache{cache: cache.NewExpiring()}, } return controllerlib.New( @@ -160,7 +164,18 @@ func New( ), withInformer( secretInformer, - pinnipedcontroller.MatchAnySecretOfTypeFilter(oidcClientSecretType, pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.MatchAnySecretOfTypesFilter( + []corev1.SecretType{ + oidcClientSecretType, + corev1.SecretTypeOpaque, + corev1.SecretTypeTLS, + }, + pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + configMapInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), ) @@ -220,8 +235,9 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst conditions := []*metav1.Condition{ c.validateSecret(upstream, &result), - c.validateIssuer(ctx.Context, upstream, &result), } + conditions = append(conditions, c.validateIssuer(ctx.Context, upstream, &result)...) + if len(rejectedAuthcodeAuthorizeParameters) > 0 { conditions = append(conditions, &metav1.Condition{ Type: typeAdditionalAuthorizeParametersValid, @@ -234,7 +250,7 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst conditions = append(conditions, &metav1.Condition{ Type: typeAdditionalAuthorizeParametersValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: allParamNamesAllowedMsg, }) } @@ -302,32 +318,42 @@ func (c *oidcWatcherController) validateSecret(upstream *idpv1alpha1.OIDCIdentit return &metav1.Condition{ Type: typeClientCredentialsSecretValid, Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, + Reason: conditionsutil.ReasonSuccess, Message: "loaded client credentials", } } // validateIssuer validates the .spec.issuer field, performs OIDC discovery, and returns the appropriate OIDCDiscoverySucceeded condition. -func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *idpv1alpha1.OIDCIdentityProvider, result *upstreamoidc.ProviderConfig) *metav1.Condition { +func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *idpv1alpha1.OIDCIdentityProvider, result *upstreamoidc.ProviderConfig) []*metav1.Condition { // Get the provider and HTTP Client from cache if possible. discoveredProvider, httpClient := c.validatorCache.getProvider(&upstream.Spec) + tlsCondition, _, certPool, _ := tlsconfigutil.ValidateTLSConfig( + tlsconfigutil.TLSSpecForSupervisor(upstream.Spec.TLS), + "oidcIdentityProvider.spec.tls", + upstream.Namespace, + c.secretInformer, + c.configMapInformer) // If the provider does not exist in the cache, do a fresh discovery lookup and save to the cache. if discoveredProvider == nil { var err error - httpClient, err = getClient(upstream) - if err != nil { - return &metav1.Condition{ - Type: typeOIDCDiscoverySucceeded, - Status: metav1.ConditionFalse, - Reason: upstreamwatchers.ReasonInvalidTLSConfig, - Message: err.Error(), + if tlsCondition.Reason != conditionsutil.ReasonSuccess { + return []*metav1.Condition{ + { + Type: typeOIDCDiscoverySucceeded, + Status: metav1.ConditionFalse, + Reason: tlsconfigutil.ReasonInvalidTLSConfig, + Message: tlsCondition.Message, + }, + tlsCondition, } } + httpClient = defaultClientShortTimeout(certPool) + _, issuerURLCondition := validateHTTPSURL(upstream.Spec.Issuer, "issuer", reasonUnreachable) if issuerURLCondition != nil { - return issuerURLCondition + return []*metav1.Condition{issuerURLCondition, tlsCondition} } discoveredProvider, err = coreosoidc.NewProvider(coreosoidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer) @@ -337,11 +363,14 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id "name", upstream.Name, "issuer", upstream.Spec.Issuer, ).Error("failed to perform OIDC discovery", err) - return &metav1.Condition{ - Type: typeOIDCDiscoverySucceeded, - Status: metav1.ConditionFalse, - Reason: reasonUnreachable, - Message: fmt.Sprintf("failed to perform OIDC discovery against %q:\n%s", upstream.Spec.Issuer, pinnipedcontroller.TruncateMostLongErr(err)), + return []*metav1.Condition{ + { + Type: typeOIDCDiscoverySucceeded, + Status: metav1.ConditionFalse, + Reason: reasonUnreachable, + Message: fmt.Sprintf("failed to perform OIDC discovery against %q:\n%s", upstream.Spec.Issuer, pinnipedcontroller.TruncateMostLongErr(err)), + }, + tlsCondition, } } @@ -356,11 +385,14 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id } if err := discoveredProvider.Claims(&additionalDiscoveryClaims); err != nil { // This shouldn't actually happen because the above call to NewProvider() would have already returned this error. - return &metav1.Condition{ - Type: typeOIDCDiscoverySucceeded, - Status: metav1.ConditionFalse, - Reason: reasonInvalidResponse, - Message: fmt.Sprintf("failed to unmarshal OIDC discovery response from %q:\n%s", upstream.Spec.Issuer, pinnipedcontroller.TruncateMostLongErr(err)), + return []*metav1.Condition{ + { + Type: typeOIDCDiscoverySucceeded, + Status: metav1.ConditionFalse, + Reason: reasonInvalidResponse, + Message: fmt.Sprintf("failed to unmarshal OIDC discovery response from %q:\n%s", upstream.Spec.Issuer, pinnipedcontroller.TruncateMostLongErr(err)), + }, + tlsCondition, } } if additionalDiscoveryClaims.RevocationEndpoint != "" { @@ -371,7 +403,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id reasonInvalidResponse, ) if revocationURLCondition != nil { - return revocationURLCondition + return []*metav1.Condition{revocationURLCondition, tlsCondition} } // Remember the URL for later use. result.RevocationURL = revocationURL @@ -383,7 +415,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id reasonInvalidResponse, ) if authorizeURLCondition != nil { - return authorizeURLCondition + return []*metav1.Condition{authorizeURLCondition, tlsCondition} } _, tokenURLCondition := validateHTTPSURL( @@ -392,18 +424,21 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id reasonInvalidResponse, ) if tokenURLCondition != nil { - return tokenURLCondition + return []*metav1.Condition{tokenURLCondition, tlsCondition} } // If everything is valid, update the result and set the condition to true. result.Config.Endpoint = discoveredProvider.Endpoint() result.Provider = discoveredProvider result.Client = httpClient - return &metav1.Condition{ - Type: typeOIDCDiscoverySucceeded, - Status: metav1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, - Message: "discovered issuer configuration", + return []*metav1.Condition{ + { + Type: typeOIDCDiscoverySucceeded, + Status: metav1.ConditionTrue, + Reason: conditionsutil.ReasonSuccess, + Message: "discovered issuer configuration", + }, + tlsCondition, } } @@ -443,7 +478,7 @@ func getClient(upstream *idpv1alpha1.OIDCIdentityProvider) (*http.Client, error) rootCAs := x509.NewCertPool() if !rootCAs.AppendCertsFromPEM(bundle) { - return nil, fmt.Errorf("spec.certificateAuthorityData is invalid: %w", upstreamwatchers.ErrNoCertificates) + return nil, fmt.Errorf("spec.certificateAuthorityData is invalid: %w", tlsconfigutil.ErrNoCertificates) } return defaultClientShortTimeout(rootCAs), nil diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index 1f8889434..376b72fea 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -49,7 +49,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { wantDelete bool }{ { - name: "a secret of the right type", + name: "should return true for a secret of the type secrets.pinniped.dev/oidc-client", secret: &corev1.Secret{ Type: "secrets.pinniped.dev/oidc-client", ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, @@ -59,14 +59,34 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { wantDelete: true, }, { - name: "a secret of the wrong type", + name: "should return true for a secret of the type Opaque", + secret: &corev1.Secret{ + Type: corev1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + 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", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "should return false for a secret of the wrong type", secret: &corev1.Secret{ Type: "secrets.pinniped.dev/not-the-oidc-client-type", ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, }, { - name: "resource of wrong data type", + name: "should return false for resource of wrong data type", secret: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, }, @@ -85,6 +105,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { &upstreamoidc.ProviderConfig{Name: "initial-entry"}, }) secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() withInformer := testutil.NewObservableWithInformerOption() var log bytes.Buffer @@ -95,6 +116,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { nil, pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), secretInformer, + configMapInformer, logger, withInformer.WithInformer, ) @@ -109,6 +131,65 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) { } } +func TestOIDCUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cm metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "any configmap", + cm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + fakePinnipedClient := supervisorfake.NewSimpleClientset() + pinnipedInformers := supervisorinformers.NewSharedInformerFactory(fakePinnipedClient, 0) + fakeKubeClient := fake.NewSimpleClientset() + kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) + cache := dynamicupstreamprovider.NewDynamicUpstreamIDPProvider() + cache.SetOIDCIdentityProviders([]upstreamprovider.UpstreamOIDCIdentityProviderI{ + &upstreamoidc.ProviderConfig{Name: "initial-entry"}, + }) + secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() + withInformer := testutil.NewObservableWithInformerOption() + + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + New( + cache, + nil, + pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), + secretInformer, + configMapInformer, + logger, + withInformer.WithInformer, + ) + + unrelated := corev1.ConfigMap{} + filter := withInformer.GetFilterForInformer(configMapInformer) + require.Equal(t, test.wantAdd, filter.Add(test.cm)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, test.cm)) + require.Equal(t, test.wantUpdate, filter.Update(test.cm, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(test.cm)) + }) + } +} + func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { t.Parallel() now := metav1.NewTime(time.Now().UTC()) @@ -1429,6 +1510,7 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs fakePinnipedClient, pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), kubeInformers.Core().V1().Secrets(), + kubeInformers.Core().V1().ConfigMaps(), logger, controllerlib.WithInformer, ) diff --git a/internal/controller/utils.go b/internal/controller/utils.go index e280cce68..2202c92b5 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -66,6 +66,30 @@ func MatchAnySecretOfTypeFilter(secretType corev1.SecretType, parentFunc control return SimpleFilter(isSecretOfType, parentFunc) } +func containsSecretType(filter []corev1.SecretType, secretType corev1.SecretType) bool { + for _, filter := range filter { + if filter == secretType { + return true + } + } + return false +} + +func MatchAnySecretOfTypesFilter(secretTypes []corev1.SecretType, parentFunc controllerlib.ParentFunc, namespaces ...string) controllerlib.Filter { + isSecretOfType := func(obj metav1.Object) bool { + secret, ok := obj.(*corev1.Secret) + if !ok { + return false + } + // Only match on namespace if namespaces are provided + if len(namespaces) > 0 && !slices.Contains(namespaces, secret.Namespace) { + return false + } + return slices.Contains(secretTypes, secret.Type) + } + return SimpleFilter(isSecretOfType, parentFunc) +} + func SecretIsControlledByParentFunc(matchFunc func(obj metav1.Object) bool) func(obj metav1.Object) controllerlib.Key { return func(obj metav1.Object) controllerlib.Key { if matchFunc(obj) { diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index 4dbf79245..41fa312ae 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -304,6 +304,7 @@ func prepareControllers( pinnipedClient, pinnipedInformers.IDP().V1alpha1().OIDCIdentityProviders(), secretInformer, + configMapInformer, plog.New(), controllerlib.WithInformer, ), @@ -314,6 +315,7 @@ func prepareControllers( pinnipedClient, pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders(), secretInformer, + configMapInformer, controllerlib.WithInformer, ), singletonWorker). @@ -334,6 +336,7 @@ func prepareControllers( pinnipedClient, pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(), secretInformer, + configMapInformer, plog.New(), controllerlib.WithInformer, clock.RealClock{},