Skip to content

Commit

Permalink
oauthclientscontroller: use a switched informer for oauthclients
Browse files Browse the repository at this point in the history 
So that it can be stopped when auth type is OIDC.
  • Loading branch information
@liouk
liouk committed Nov 22, 2024
1 parent a70732c commit ae635c0
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 19 deletions.
27 changes: 18 additions & 9 deletions pkg/controllers/oauthclientscontroller/oauthclientscontroller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/informers"
corev1listers "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/retry"

configv1 "github.com/openshift/api/config/v1"
oauthv1 "github.com/openshift/api/oauth/v1"
configinformers "github.com/openshift/client-go/config/informers/externalversions"
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned/typed/oauth/v1"
oauthinformers "github.com/openshift/client-go/oauth/informers/externalversions"
oauthv1listers "github.com/openshift/client-go/oauth/listers/oauth/v1"
operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1"
Expand All @@ -35,14 +35,16 @@ import (

"github.com/openshift/cluster-authentication-operator/pkg/controllers/common"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/customroute"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/oauthclientsswitchedinformer"
)

type oauthsClientsController struct {
oauthClientClient oauthclient.OAuthClientInterface

oauthClientLister oauthv1listers.OAuthClientLister
routeLister routev1listers.RouteLister
ingressLister configv1listers.IngressLister
oauthClientInformer cache.SharedIndexInformer
oauthClientLister oauthv1listers.OAuthClientLister
routeLister routev1listers.RouteLister
ingressLister configv1listers.IngressLister

authLister configv1listers.AuthenticationLister
kasLister operatorv1listers.KubeAPIServerLister
Expand All @@ -52,7 +54,7 @@ type oauthsClientsController struct {
func NewOAuthClientsController(
operatorClient v1helpers.OperatorClient,
oauthsClientClient oauthclient.OAuthClientInterface,
oauthInformers oauthinformers.SharedInformerFactory,
oauthClientsSwitchedInformer *oauthclientsswitchedinformer.InformerWithSwitch,
routeInformers routeinformers.SharedInformerFactory,
operatorConfigInformers configinformers.SharedInformerFactory,
kasInformer operatorv1informers.KubeAPIServerInformer,
Expand All @@ -62,9 +64,10 @@ func NewOAuthClientsController(
c := &oauthsClientsController{
oauthClientClient: oauthsClientClient,

oauthClientLister: oauthInformers.Oauth().V1().OAuthClients().Lister(),
routeLister: routeInformers.Route().V1().Routes().Lister(),
ingressLister: operatorConfigInformers.Config().V1().Ingresses().Lister(),
oauthClientInformer: oauthClientsSwitchedInformer.Informer(),
oauthClientLister: oauthv1listers.NewOAuthClientLister(oauthClientsSwitchedInformer.Informer().GetIndexer()),
routeLister: routeInformers.Route().V1().Routes().Lister(),
ingressLister: operatorConfigInformers.Config().V1().Ingresses().Lister(),

authLister: operatorConfigInformers.Config().V1().Authentications().Lister(),
kasLister: kasInformer.Lister(),
Expand All @@ -76,7 +79,7 @@ func NewOAuthClientsController(
WithSyncDegradedOnError(operatorClient).
WithFilteredEventsInformers(
factory.NamesFilter("openshift-browser-client", "openshift-challenging-client", "openshift-cli-client"),
oauthInformers.Oauth().V1().OAuthClients().Informer(),
oauthClientsSwitchedInformer.Informer(),
).
WithFilteredEventsInformers(
factory.NamesFilter("oauth-openshift"),
Expand Down Expand Up @@ -114,6 +117,12 @@ func (c *oauthsClientsController) sync(ctx context.Context, syncCtx factory.Sync
return err
}

waitCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
if !cache.WaitForCacheSync(waitCtx.Done(), c.oauthClientInformer.HasSynced) {
return fmt.Errorf("timed out waiting for OAuthClients informer cache sync")
}

return c.ensureBootstrappedOAuthClients(ctx, "https://"+routeHost)
}

Expand Down
25 changes: 20 additions & 5 deletions pkg/controllers/oauthclientscontroller/oauthclientscontroller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/base64"
"fmt"
"testing"
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
Expand All @@ -20,8 +21,10 @@ import (
routev1 "github.com/openshift/api/route/v1"
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
fakeoauthclient "github.com/openshift/client-go/oauth/clientset/versioned/fake"
oauthinformers "github.com/openshift/client-go/oauth/informers/externalversions"
oauthv1listers "github.com/openshift/client-go/oauth/listers/oauth/v1"
routev1listers "github.com/openshift/client-go/route/listers/route/v1"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/oauthclientsswitchedinformer"
"github.com/openshift/library-go/pkg/oauth/oauthdiscovery"
"github.com/openshift/library-go/pkg/operator/events"
)
Expand Down Expand Up @@ -118,12 +121,24 @@ func newAuthLister(t *testing.T) configv1listers.AuthenticationLister {
}

func newTestOAuthsClientsController(t *testing.T) *oauthsClientsController {
oauthClientset := fakeoauthclient.NewSimpleClientset()
switchedInformer := oauthclientsswitchedinformer.NewSwitchedInformer(
"TestOAuthClientsInformerWithSwitchController",
context.TODO(),
func() (bool, error) { return false, nil },
oauthinformers.NewSharedInformerFactoryWithOptions(oauthClientset, 1*time.Minute).Oauth().V1().OAuthClients().Informer(),
0,
nil,
events.NewInMemoryRecorder("oauthclientscontroller_test"),
)

return &oauthsClientsController{
oauthClientClient: fakeoauthclient.NewSimpleClientset().OauthV1().OAuthClients(),
oauthClientLister: oauthv1listers.NewOAuthClientLister(cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})),
routeLister: newRouteLister(t, defaultRoute),
ingressLister: newIngressLister(t, defaultIngress),
authLister: newAuthLister(t),
oauthClientInformer: switchedInformer.Informer(),
oauthClientClient: oauthClientset.OauthV1().OAuthClients(),
oauthClientLister: oauthv1listers.NewOAuthClientLister(cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})),
routeLister: newRouteLister(t, defaultRoute),
ingressLister: newIngressLister(t, defaultIngress),
authLister: newAuthLister(t),
}
}

Expand Down
4 changes: 0 additions & 4 deletions pkg/operator/replacement_starter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
configclient "github.com/openshift/client-go/config/clientset/versioned"
configinformer "github.com/openshift/client-go/config/informers/externalversions"
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned"
oauthinformers "github.com/openshift/client-go/oauth/informers/externalversions"
operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
operatorinformer "github.com/openshift/client-go/operator/informers/externalversions"
routeclient "github.com/openshift/client-go/route/clientset/versioned"
Expand Down Expand Up @@ -214,7 +213,6 @@ type authenticationOperatorInformerFactories struct {
kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces
operatorConfigInformer configinformer.SharedInformerFactory
operatorInformer operatorinformer.SharedInformerFactory
oauthInformers oauthinformers.SharedInformerFactory
apiregistrationInformers apiregistrationinformers.SharedInformerFactory
migrationInformer migrationv1alpha1informer.SharedInformerFactory
// TODO remove
Expand All @@ -240,7 +238,6 @@ func newInformerFactories(authOperatorInput *authenticationOperatorInput) authen
),
operatorConfigInformer: configinformer.NewSharedInformerFactoryWithOptions(authOperatorInput.configClient, resync),
operatorInformer: operatorinformer.NewSharedInformerFactory(authOperatorInput.operatorClient, 24*time.Hour),
oauthInformers: oauthinformers.NewSharedInformerFactory(authOperatorInput.oauthClient, resync),
apiregistrationInformers: apiregistrationinformers.NewSharedInformerFactory(authOperatorInput.apiregistrationv1Client, 10*time.Minute),
migrationInformer: migrationv1alpha1informer.NewSharedInformerFactory(authOperatorInput.migrationClient, time.Minute*30),
kubeInformers: kubeinformers.NewSharedInformerFactory(authOperatorInput.kubeClient, resync),
Expand All @@ -257,7 +254,6 @@ func (a authenticationOperatorInformerFactories) simplifiedInformerFactories() [
libraryapplyconfiguration.GeneratedNamespacedInformerFactoryAdapter(a.kubeInformersForNamespaces),
libraryapplyconfiguration.GeneratedInformerFactoryAdapter(a.operatorInformer),
libraryapplyconfiguration.GeneratedInformerFactoryAdapter(a.operatorConfigInformer),
libraryapplyconfiguration.GeneratedInformerFactoryAdapter(a.oauthInformers),
libraryapplyconfiguration.GeneratedInformerFactoryAdapter(a.apiregistrationInformers),
libraryapplyconfiguration.GeneratedInformerFactoryAdapter(a.migrationInformer),
libraryapplyconfiguration.GeneratedInformerFactoryAdapter(a.kubeInformers),
Expand Down
22 changes: 21 additions & 1 deletion pkg/operator/starter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
operatorv1 "github.com/openshift/api/operator/v1"
routev1 "github.com/openshift/api/route/v1"
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
oauthinformers "github.com/openshift/client-go/oauth/informers/externalversions"
applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1"
"github.com/openshift/cluster-authentication-operator/bindata"
Expand All @@ -28,6 +29,7 @@ import (
"github.com/openshift/cluster-authentication-operator/pkg/controllers/ingressstate"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/metadata"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/oauthclientscontroller"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/oauthclientsswitchedinformer"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/oauthendpoints"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/payload"
"github.com/openshift/cluster-authentication-operator/pkg/controllers/proxyconfig"
Expand All @@ -41,6 +43,7 @@ import (
"github.com/openshift/cluster-authentication-operator/pkg/operator/workload"
"github.com/openshift/library-go/pkg/authentication/bootstrapauthenticator"
"github.com/openshift/library-go/pkg/controller/controllercmd"
"github.com/openshift/library-go/pkg/controller/factory"
workloadcontroller "github.com/openshift/library-go/pkg/operator/apiserver/controller/workload"
apiservercontrollerset "github.com/openshift/library-go/pkg/operator/apiserver/controllerset"
"github.com/openshift/library-go/pkg/operator/certrotation"
Expand Down Expand Up @@ -256,10 +259,25 @@ func prepareOauthOperator(
authOperatorInput.eventRecorder,
)

oauthClientsSwitchedInformer := oauthclientsswitchedinformer.NewSwitchedInformer(
"OAuthClientsInformerWithSwitchController",
ctx,
func() (bool, error) {
return common.ExternalOIDCConfigAvailable(authLister, kasLister, kasConfigMapLister)
},
oauthinformers.NewSharedInformerFactoryWithOptions(authOperatorInput.oauthClient, 1*time.Minute).Oauth().V1().OAuthClients().Informer(),
0,
[]factory.Informer{
informerFactories.operatorInformer.Operator().V1().KubeAPIServers().Informer(),
informerFactories.operatorConfigInformer.Config().V1().Authentications().Informer(),
},
authOperatorInput.eventRecorder,
)

oauthClientsController := oauthclientscontroller.NewOAuthClientsController(
authOperatorInput.authenticationOperatorClient,
authOperatorInput.oauthClient.OauthV1().OAuthClients(),
informerFactories.oauthInformers,
oauthClientsSwitchedInformer,
informerFactories.namespacedOpenshiftAuthenticationRoutes,
informerFactories.operatorConfigInformer,
informerFactories.operatorInformer.Operator().V1().KubeAPIServers(),
Expand Down Expand Up @@ -386,6 +404,7 @@ func prepareOauthOperator(
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-deploymentController", deploymentController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-managementStateController", managementStateController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-metadataController", metadataController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-oauthClientsSwitchedInformerController", oauthClientsSwitchedInformer.Controller().Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-oauthClientsController", oauthClientsController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-payloadConfigController", payloadConfigController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-routerCertsController", routerCertsController.Sync),
Expand All @@ -408,6 +427,7 @@ func prepareOauthOperator(
libraryapplyconfiguration.AdaptRunFn(deploymentController.Run),
libraryapplyconfiguration.AdaptRunFn(managementStateController.Run),
libraryapplyconfiguration.AdaptRunFn(metadataController.Run),
libraryapplyconfiguration.AdaptRunFn(oauthClientsSwitchedInformer.Controller().Run),
libraryapplyconfiguration.AdaptRunFn(oauthClientsController.Run),
libraryapplyconfiguration.AdaptRunFn(payloadConfigController.Run),
libraryapplyconfiguration.AdaptRunFn(routerCertsController.Run),
Expand Down

0 comments on commit ae635c0

Please sign in to comment.