diff --git a/pkg/controllers/common/external_oidc.go b/pkg/controllers/common/external_oidc.go new file mode 100644 index 0000000000..0e1f3755b7 --- /dev/null +++ b/pkg/controllers/common/external_oidc.go @@ -0,0 +1,69 @@ +package common + +import ( + "fmt" + "strings" + + configv1 "github.com/openshift/api/config/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/sets" + corelistersv1 "k8s.io/client-go/listers/core/v1" +) + +// ExternalOIDCConfigAvailable checks the kubeapiservers/cluster resource for KAS pod +// rollout status; it returns true if auth type is OIDC, all KAS pods are currently on a revision +// that includes the structured auth-config ConfigMap, and the KAS args include the respective +// arg that enables usage of the structured auth-config. It returns false otherwise. +func ExternalOIDCConfigAvailable(authLister configv1listers.AuthenticationLister, kasLister operatorv1listers.KubeAPIServerLister, cmLister corelistersv1.ConfigMapLister) (bool, error) { + auth, err := authLister.Get("cluster") + if err != nil { + return false, err + } + + if auth.Spec.Type != configv1.AuthenticationTypeOIDC { + return false, nil + } + + kas, err := kasLister.Get("cluster") + if err != nil { + return false, err + } + + observedRevisions := sets.New[int32]() + for _, nodeStatus := range kas.Status.NodeStatuses { + observedRevisions.Insert(nodeStatus.CurrentRevision) + } + + if observedRevisions.Len() == 0 { + return false, nil + } + + for _, revision := range observedRevisions.UnsortedList() { + // ensure every observed revision includes an auth-config revisioned configmap + _, err := cmLister.ConfigMaps("openshift-kube-apiserver").Get(fmt.Sprintf("auth-config-%d", revision)) + if errors.IsNotFound(err) { + return false, nil + } else if err != nil { + return false, err + } + + // every observed revision includes a copy of the KAS config configmap + cm, err := cmLister.ConfigMaps("openshift-kube-apiserver").Get(fmt.Sprintf("config-%d", revision)) + if err != nil { + return false, err + } + + // ensure the KAS config of every observed revision contains the appropriate CLI arg for OIDC + // but not the respective ones for OAuth + if !strings.Contains(cm.Data["config.yaml"], `"oauthMetadataFile":""`) || + strings.Contains(cm.Data["config.yaml"], `"authentication-token-webhook-config-file":`) || + !strings.Contains(cm.Data["config.yaml"], `"authentication-config":["/etc/kubernetes/static-pod-resources/configmaps/auth-config/auth-config.json"]`) { + return false, nil + } + } + + return true, nil +} diff --git a/pkg/controllers/common/external_oidc_test.go b/pkg/controllers/common/external_oidc_test.go new file mode 100644 index 0000000000..bd1aa7c105 --- /dev/null +++ b/pkg/controllers/common/external_oidc_test.go @@ -0,0 +1,317 @@ +package common + +import ( + "testing" + + configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corelistersv1 "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" +) + +const ( + kasConfigJSONWithOIDC = `"spec":{"apiServerArguments":{"authentication-config":["/etc/kubernetes/static-pod-resources/configmaps/auth-config/auth-config.json"]},"oauthMetadataFile":""}` + kasConfigJSONWithoutOIDC = `"spec":{"apiServerArguments":{"authentication-token-webhook-config-file":["/etc/kubernetes/static-pod-resources/secrets/webhook-authenticator/kubeConfig"]},"oauthMetadataFile":"/etc/kubernetes/static-pod-resources/configmaps/oauth-metadata/oauthMetadata"}` +) + +func TestExternalOIDCConfigAvailable(t *testing.T) { + for _, tt := range []struct { + name string + configMaps []*corev1.ConfigMap + authType configv1.AuthenticationType + nodeStatuses []operatorv1.NodeStatus + expectAvailable bool + expectError bool + }{ + { + name: "no node statuses observed", + authType: configv1.AuthenticationTypeOIDC, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc disabled, no rollout", + configMaps: []*corev1.ConfigMap{cm("config-10", "config.yaml", kasConfigJSONWithoutOIDC)}, + authType: configv1.AuthenticationTypeIntegratedOAuth, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 10}, + {CurrentRevision: 10}, + {CurrentRevision: 10}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc getting enabled, rollout in progress", + configMaps: []*corev1.ConfigMap{ + cm("config-10", "config.yaml", kasConfigJSONWithoutOIDC), + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 10, TargetRevision: 11}, + {CurrentRevision: 10}, + {CurrentRevision: 10}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc getting enabled, rollout in progress, one node ready", + configMaps: []*corev1.ConfigMap{ + cm("config-10", "config.yaml", kasConfigJSONWithoutOIDC), + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 11}, + {CurrentRevision: 10, TargetRevision: 11}, + {CurrentRevision: 10}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc getting enabled, rollout in progress, two nodes ready", + configMaps: []*corev1.ConfigMap{ + cm("config-10", "config.yaml", kasConfigJSONWithoutOIDC), + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 11}, + {CurrentRevision: 11}, + {CurrentRevision: 10, TargetRevision: 11}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc got enabled", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 11}, + {CurrentRevision: 11}, + {CurrentRevision: 11}, + }, + expectAvailable: true, + expectError: false, + }, + { + name: "oidc enabled, rollout in progress", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 11, TargetRevision: 12}, + {CurrentRevision: 11}, + {CurrentRevision: 11}, + }, + expectAvailable: true, + expectError: false, + }, + { + name: "oidc enabled, rollout in progress, one node ready", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 12}, + {CurrentRevision: 11, TargetRevision: 12}, + {CurrentRevision: 11}, + }, + expectAvailable: true, + expectError: false, + }, + { + name: "oidc enabled, rollout in progress, two nodes ready", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 12}, + {CurrentRevision: 12}, + {CurrentRevision: 11, TargetRevision: 12}, + }, + expectAvailable: true, + expectError: false, + }, + { + name: "oidc still enabled", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 12}, + {CurrentRevision: 12}, + {CurrentRevision: 12}, + }, + expectAvailable: true, + expectError: false, + }, + { + name: "oidc getting disabled, rollout in progress", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("config-13", "config.yaml", kasConfigJSONWithoutOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeIntegratedOAuth, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 12, TargetRevision: 13}, + {CurrentRevision: 12}, + {CurrentRevision: 12}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc getting disabled, rollout in progress, one node ready", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("config-13", "config.yaml", kasConfigJSONWithoutOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeIntegratedOAuth, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 13}, + {CurrentRevision: 12, TargetRevision: 13}, + {CurrentRevision: 12}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc getting disabled, rollout in progress, two nodes ready", + authType: configv1.AuthenticationTypeIntegratedOAuth, + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-13", "config.yaml", kasConfigJSONWithoutOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 13}, + {CurrentRevision: 13}, + {CurrentRevision: 12, TargetRevision: 13}, + }, + expectAvailable: false, + expectError: false, + }, + { + name: "oidc got disabled", + configMaps: []*corev1.ConfigMap{ + cm("config-11", "config.yaml", kasConfigJSONWithOIDC), + cm("config-12", "config.yaml", kasConfigJSONWithOIDC), + cm("config-13", "config.yaml", kasConfigJSONWithoutOIDC), + cm("auth-config-11", "", ""), + cm("auth-config-12", "", ""), + }, + authType: configv1.AuthenticationTypeIntegratedOAuth, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 13}, + {CurrentRevision: 13}, + {CurrentRevision: 13}, + }, + expectAvailable: false, + expectError: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + + cmIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + for _, cm := range tt.configMaps { + cmIndexer.Add(cm) + } + + kasIndexer := cache.NewIndexer(func(obj interface{}) (string, error) { + return "cluster", nil + }, cache.Indexers{}) + + kasIndexer.Add(&operatorv1.KubeAPIServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: operatorv1.KubeAPIServerStatus{ + StaticPodOperatorStatus: operatorv1.StaticPodOperatorStatus{ + NodeStatuses: tt.nodeStatuses, + }, + }, + }) + + authIndexer := cache.NewIndexer(func(obj interface{}) (string, error) { + return "cluster", nil + }, cache.Indexers{}) + + authIndexer.Add(&configv1.Authentication{ + Spec: configv1.AuthenticationSpec{ + Type: tt.authType, + }, + }) + + available, err := ExternalOIDCConfigAvailable( + configv1listers.NewAuthenticationLister(authIndexer), + operatorv1listers.NewKubeAPIServerLister(kasIndexer), + corelistersv1.NewConfigMapLister(cmIndexer), + ) + + if tt.expectError != (err != nil) { + t.Fatalf("expected error %v; got %v", tt.expectError, err) + } + + if tt.expectAvailable != available { + t.Fatalf("expected available %v; got %v", tt.expectAvailable, available) + } + }) + } +} + +func cm(name, dataKey, dataValue string) *corev1.ConfigMap { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "openshift-kube-apiserver", + }, + } + + if len(dataKey) > 0 { + cm.Data = map[string]string{ + dataKey: dataValue, + } + } + + return cm +} diff --git a/pkg/controllers/customroute/custom_route_controller.go b/pkg/controllers/customroute/custom_route_controller.go index b9271b4e61..1d935aff0c 100644 --- a/pkg/controllers/customroute/custom_route_controller.go +++ b/pkg/controllers/customroute/custom_route_controller.go @@ -12,6 +12,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/informers" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" @@ -19,8 +20,11 @@ import ( routev1 "github.com/openshift/api/route/v1" applyconfigv1 "github.com/openshift/client-go/config/applyconfigurations/config/v1" configsetterv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" - configinformers "github.com/openshift/client-go/config/informers/externalversions/config/v1" + configinformers "github.com/openshift/client-go/config/informers/externalversions" + configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions/route/v1" routev1lister "github.com/openshift/client-go/route/listers/route/v1" @@ -51,6 +55,10 @@ type customRouteController struct { secretLister corev1listers.SecretLister resourceSyncer resourcesynccontroller.ResourceSyncer operatorClient v1helpers.OperatorClient + + authLister configlistersv1.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1listers.ConfigMapLister } func NewCustomRouteController( @@ -58,11 +66,14 @@ func NewCustomRouteController( componentRouteName string, destSecretNamespace string, destSecretName string, - ingressInformer configinformers.IngressInformer, + ingressInformer configinformersv1.IngressInformer, ingressClient configsetterv1.IngressInterface, routeInformer routeinformer.RouteInformer, routeClient routeclient.RouteInterface, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, + operatorConfigInformers configinformers.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, operatorClient v1helpers.OperatorClient, eventRecorder events.Recorder, resourceSyncer resourcesynccontroller.ResourceSyncer, @@ -83,6 +94,10 @@ func NewCustomRouteController( secretLister: kubeInformersForNamespaces.SecretLister(), operatorClient: operatorClient, resourceSyncer: resourceSyncer, + + authLister: operatorConfigInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New(). @@ -91,6 +106,8 @@ func NewCustomRouteController( routeInformer.Informer(), kubeInformersForNamespaces.InformersFor("openshift-config").Core().V1().Secrets().Informer(), kubeInformersForNamespaces.InformersFor("openshift-authentication").Core().V1().Secrets().Informer(), + operatorConfigInformers.Config().V1().Authentications().Informer(), + kasInformer.Informer(), ). WithSyncDegradedOnError(operatorClient). WithSync(controller.sync). @@ -106,6 +123,12 @@ func (c *customRouteController) sync(ctx context.Context, syncCtx factory.SyncCo ingressConfigCopy := ingressConfig.DeepCopy() + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + return c.removeOperands(ctx, ingressConfigCopy) + } + // configure the expected route expectedRoute, secretName, errors := c.getOAuthRouteAndSecretName(ingressConfigCopy) if errors != nil { @@ -289,3 +312,46 @@ func (c *customRouteController) getFieldManager() string { // TODO find a way to get the client name and combine it with the controller name automatically return "AuthenticationCustomRouteController" } + +func (c *customRouteController) removeOperands(ctx context.Context, ingressConfig *configv1.Ingress) error { + if _, err := c.routeLister.Routes(c.componentRoute.Namespace).Get(c.componentRoute.Name); err != nil && !errors.IsNotFound(err) { + return err + } else if !errors.IsNotFound(err) { + if err := c.routeClient.Delete(ctx, c.componentRoute.Name, metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) { + return err + } + } + + ingressStatus, err := applyconfigv1.ExtractIngressStatus(ingressConfig, c.getFieldManager()) + if err != nil { + return err + } + + if ingressStatus != nil && ingressStatus.Status != nil { + componentRoutes := make([]applyconfigv1.ComponentRouteStatusApplyConfiguration, 0) + routeFound := false + for _, cr := range ingressStatus.Status.ComponentRoutes { + if *cr.Name == c.componentRoute.Name && *cr.Namespace == c.componentRoute.Namespace { + routeFound = true + continue + } + + componentRoutes = append(componentRoutes, cr) + } + + if routeFound { + ingressStatus.Status.ComponentRoutes = componentRoutes + ingress := applyconfigv1.Ingress(ingressConfig.Name).WithStatus(ingressStatus.Status) + if _, err := c.ingressClient.ApplyStatus(ctx, ingress, c.forceApply()); err != nil { + return err + } + } + } + + // delete secret by syncing an empty source + if err := c.syncSecret(""); err != nil { + return err + } + + return nil +} diff --git a/pkg/controllers/deployment/deployment_controller.go b/pkg/controllers/deployment/deployment_controller.go index 4889a0a932..f8455a52ec 100644 --- a/pkg/controllers/deployment/deployment_controller.go +++ b/pkg/controllers/deployment/deployment_controller.go @@ -21,8 +21,11 @@ import ( configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" configinformer "github.com/openshift/client-go/config/informers/externalversions" configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeinformers "github.com/openshift/client-go/route/informers/externalversions" routev1listers "github.com/openshift/client-go/route/listers/route/v1" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" bootstrap "github.com/openshift/library-go/pkg/authentication/bootstrapauthenticator" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/apiserver/controller/workload" @@ -60,6 +63,10 @@ type oauthServerDeploymentSyncer struct { proxyLister configv1listers.ProxyLister routeLister routev1listers.RouteLister + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1listers.ConfigMapLister + bootstrapUserDataGetter bootstrap.BootstrapUserDataGetter bootstrapUserChangeRollOut bool } @@ -77,6 +84,8 @@ func NewOAuthServerWorkloadController( eventsRecorder events.Recorder, versionRecorder status.VersionGetter, kubeInformersForTargetNamespace informers.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, ) factory.Controller { targetNS := "openshift-authentication" @@ -94,6 +103,10 @@ func NewOAuthServerWorkloadController( proxyLister: configInformers.Config().V1().Proxies().Lister(), routeLister: routeInformersForTargetNamespace.Route().V1().Routes().Lister(), + authLister: configInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), + bootstrapUserDataGetter: bootstrapUserDataGetter, } @@ -117,7 +130,9 @@ func NewOAuthServerWorkloadController( []factory.Informer{ configInformers.Config().V1().Ingresses().Informer(), configInformers.Config().V1().Proxies().Informer(), + configInformers.Config().V1().Authentications().Informer(), nodeInformer.Informer(), + kasInformer.Informer(), }, []factory.Informer{ kubeInformersForTargetNamespace.Apps().V1().Deployments().Informer(), @@ -135,6 +150,13 @@ func NewOAuthServerWorkloadController( } func (c *oauthServerDeploymentSyncer) PreconditionFulfilled(_ context.Context) (bool, error) { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return false, err + } else if oidcAvailable { + // the route is no longer a pre-requisite + return true, nil + } + route, err := c.routeLister.Routes("openshift-authentication").Get("oauth-openshift") if err != nil { return false, fmt.Errorf("waiting for the oauth-openshift route to appear: %w", err) diff --git a/pkg/controllers/ingressnodesavailable/ingress_nodes_available_controller.go b/pkg/controllers/ingressnodesavailable/ingress_nodes_available_controller.go index f8c15fba90..c8e1150c63 100644 --- a/pkg/controllers/ingressnodesavailable/ingress_nodes_available_controller.go +++ b/pkg/controllers/ingressnodesavailable/ingress_nodes_available_controller.go @@ -6,18 +6,22 @@ import ( "time" operatorv1 "github.com/openshift/api/operator/v1" + configinformer "github.com/openshift/client-go/config/informers/externalversions" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/informers" corev1informers "k8s.io/client-go/informers/core/v1" corev1listers "k8s.io/client-go/listers/core/v1" ) @@ -32,6 +36,10 @@ type ingressNodesAvailableController struct { operatorClient v1helpers.OperatorClient ingressLister operatorv1listers.IngressControllerLister nodeLister corev1listers.NodeLister + + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1listers.ConfigMapLister } func NewIngressNodesAvailableController( @@ -40,12 +48,19 @@ func NewIngressNodesAvailableController( ingressControllerInformer operatorv1informers.IngressControllerInformer, eventRecorder events.Recorder, nodeInformer corev1informers.NodeInformer, + configInformers configinformer.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, ) factory.Controller { controller := &ingressNodesAvailableController{ controllerInstanceName: factory.ControllerInstanceName(instanceName, "IngressNodesAvailable"), operatorClient: operatorClient, ingressLister: ingressControllerInformer.Lister(), nodeLister: nodeInformer.Lister(), + + authLister: configInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New(). @@ -72,6 +87,12 @@ func countReadyWorkerNodes(nodes []*corev1.Node) int { } func (c *ingressNodesAvailableController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, knownConditionNames, nil) + } + foundConditions := []operatorv1.OperatorCondition{} workers, err := c.nodeLister.List(labels.SelectorFromSet(labels.Set{"node-role.kubernetes.io/worker": ""})) diff --git a/pkg/controllers/ingressstate/ingress_state_controller.go b/pkg/controllers/ingressstate/ingress_state_controller.go index 190b3bc28c..4f192fabcb 100644 --- a/pkg/controllers/ingressstate/ingress_state_controller.go +++ b/pkg/controllers/ingressstate/ingress_state_controller.go @@ -7,9 +7,15 @@ import ( "time" operatorv1 "github.com/openshift/api/operator/v1" + configinformer "github.com/openshift/client-go/config/informers/externalversions" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,8 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - - "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" + corev1listers "k8s.io/client-go/listers/core/v1" ) const ( @@ -37,11 +42,18 @@ type ingressStateController struct { podsGetter corev1client.PodsGetter targetNamespace string operatorClient v1helpers.OperatorClient + + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1listers.ConfigMapLister } func NewIngressStateController( instanceName string, kubeInformersForTargetNamespace informers.SharedInformerFactory, + configInformers configinformer.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, endpointsGetter corev1client.EndpointsGetter, podsGetter corev1client.PodsGetter, operatorClient v1helpers.OperatorClient, @@ -54,6 +66,10 @@ func NewIngressStateController( podsGetter: podsGetter, targetNamespace: targetNamespace, operatorClient: operatorClient, + + authLister: configInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New(). @@ -76,6 +92,13 @@ func (c *ingressStateController) checkPodStatus(ctx context.Context, reference * } func (c *ingressStateController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + // clear all operator conditions + return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, degradedConditionTypes, nil) + } + endpoints, err := c.endpointsGetter.Endpoints(c.targetNamespace).Get(context.TODO(), "oauth-openshift", metav1.GetOptions{}) if apierrors.IsNotFound(err) { // Clear the error to allow checkSubset to report degraded because endpoints == nil diff --git a/pkg/controllers/metadata/metadata_controller.go b/pkg/controllers/metadata/metadata_controller.go index 267c2ea9ab..d3d3d30819 100644 --- a/pkg/controllers/metadata/metadata_controller.go +++ b/pkg/controllers/metadata/metadata_controller.go @@ -7,6 +7,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" @@ -19,6 +20,8 @@ import ( configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" configinformers "github.com/openshift/client-go/config/informers/externalversions" configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions" "github.com/openshift/library-go/pkg/controller/factory" @@ -43,36 +46,66 @@ type metadataController struct { ingressLister configv1listers.IngressLister route routeclient.RouteInterface secretLister corev1listers.SecretLister + configMapLister corev1listers.ConfigMapLister configMaps corev1client.ConfigMapsGetter authentication configv1client.AuthenticationInterface operatorClient v1helpers.OperatorClient + + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1listers.ConfigMapLister } // NewMetadataController assure that ingress configuration is available to determine the domain suffix that this controller use to create // a route for oauth. The controller then update the oauth metadata config map and update the cluster authentication config. // The controller use degraded condition if any part of the process fail and use the "AuthMetadataProgressing=false" condition when the controller job is done // and all resources exists. -func NewMetadataController(instanceName string, kubeInformersForTargetNamespace informers.SharedInformerFactory, configInformer configinformers.SharedInformerFactory, routeInformer routeinformer.SharedInformerFactory, - configMaps corev1client.ConfigMapsGetter, route routeclient.RouteInterface, authentication configv1client.AuthenticationInterface, operatorClient v1helpers.OperatorClient, - recorder events.Recorder) factory.Controller { +func NewMetadataController(instanceName string, + kubeInformersForTargetNamespace informers.SharedInformerFactory, + configInformer configinformers.SharedInformerFactory, + routeInformer routeinformer.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, + configMaps corev1client.ConfigMapsGetter, + route routeclient.RouteInterface, + authentication configv1client.AuthenticationInterface, + operatorClient v1helpers.OperatorClient, + recorder events.Recorder, +) factory.Controller { c := &metadataController{ controllerInstanceName: factory.ControllerInstanceName(instanceName, "Metadata"), ingressLister: configInformer.Config().V1().Ingresses().Lister(), secretLister: kubeInformersForTargetNamespace.Core().V1().Secrets().Lister(), + configMapLister: kubeInformersForTargetNamespace.Core().V1().ConfigMaps().Lister(), configMaps: configMaps, route: route, authentication: authentication, operatorClient: operatorClient, + + authLister: configInformer.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New().WithInformers( kubeInformersForTargetNamespace.Core().V1().Secrets().Informer(), configInformer.Config().V1().Authentications().Informer(), configInformer.Config().V1().Ingresses().Informer(), routeInformer.Route().V1().Routes().Informer(), + kasInformer.Informer(), ).ResyncEvery(wait.Jitter(time.Minute, 1.0)).WithSync(c.sync).ToController(c.controllerInstanceName, recorder.WithComponentSuffix("metadata-controller")) } func (c *metadataController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + if err := c.removeOperands(ctx); err != nil { + return err + } + + return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, knownConditionNames, nil) + } + foundConditions := []operatorv1.OperatorCondition{} foundConditions = append(foundConditions, c.handleOAuthMetadataConfigMap(ctx, syncCtx.Recorder())...) @@ -156,6 +189,21 @@ func (c *metadataController) handleAuthConfig(ctx context.Context) []operatorv1. return nil } +func (c *metadataController) removeOperands(ctx context.Context) error { + if _, err := c.configMapLister.ConfigMaps("openshift-authentication").Get("v4-0-config-system-metadata"); errors.IsNotFound(err) { + return nil + } else if err != nil { + return err + } + + err := c.configMaps.ConfigMaps("openshift-authentication").Delete(ctx, "v4-0-config-system-metadata", metav1.DeleteOptions{}) + if err != nil && !errors.IsNotFound(err) { + return err + } + + return nil +} + func getOAuthMetadata(host string) string { stubMetadata := ` { diff --git a/pkg/controllers/oauthclientscontroller/oauthclientscontroller.go b/pkg/controllers/oauthclientscontroller/oauthclientscontroller.go index df080ea9a9..62acdfd7c3 100644 --- a/pkg/controllers/oauthclientscontroller/oauthclientscontroller.go +++ b/pkg/controllers/oauthclientscontroller/oauthclientscontroller.go @@ -8,9 +8,13 @@ import ( "time" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "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" @@ -18,8 +22,9 @@ import ( 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" routeinformers "github.com/openshift/client-go/route/informers/externalversions" routev1listers "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/library-go/pkg/controller/factory" @@ -30,30 +35,43 @@ 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 + kasConfigMapLister corev1listers.ConfigMapLister } func NewOAuthClientsController( operatorClient v1helpers.OperatorClient, oauthsClientClient oauthclient.OAuthClientInterface, - oauthInformers oauthinformers.SharedInformerFactory, + oauthClientsSwitchedInformer *oauthclientsswitchedinformer.InformerWithSwitch, routeInformers routeinformers.SharedInformerFactory, - ingressInformers configinformers.SharedInformerFactory, + operatorConfigInformers configinformers.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, eventRecorder events.Recorder, ) factory.Controller { c := &oauthsClientsController{ oauthClientClient: oauthsClientClient, - oauthClientLister: oauthInformers.Oauth().V1().OAuthClients().Lister(), - routeLister: routeInformers.Route().V1().Routes().Lister(), - ingressLister: ingressInformers.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(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New(). @@ -61,18 +79,28 @@ 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"), routeInformers.Route().V1().Routes().Informer(), ). - WithInformers(ingressInformers.Config().V1().Ingresses().Informer()). + WithInformers( + operatorConfigInformers.Config().V1().Ingresses().Informer(), + operatorConfigInformers.Config().V1().Authentications().Informer(), + kasInformer.Informer(), + ). ResyncEvery(wait.Jitter(time.Minute, 1.0)). ToController("OAuthClientsController", eventRecorder.WithComponentSuffix("oauth-clients-controller")) } func (c *oauthsClientsController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + return c.ensureBootstrappedOAuthClientsMissing(ctx) + } + ingress, err := c.getIngressConfig() if err != nil { return err @@ -89,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) } @@ -146,6 +180,27 @@ func (c *oauthsClientsController) ensureBootstrappedOAuthClients(ctx context.Con return nil } +func (c *oauthsClientsController) ensureBootstrappedOAuthClientsMissing(ctx context.Context) error { + for _, clientName := range []string{ + "openshift-browser-client", + "openshift-challenging-client", + "openshift-cli-client", + } { + _, err := c.oauthClientLister.Get(clientName) + if errors.IsNotFound(err) { + continue + } else if err != nil { + return err + } + + if err := c.oauthClientClient.Delete(ctx, clientName, metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) { + return err + } + } + + return nil +} + func randomBits(bits uint) []byte { size := bits / 8 if bits%8 != 0 { diff --git a/pkg/controllers/oauthclientscontroller/oauthclientscontroller_test.go b/pkg/controllers/oauthclientscontroller/oauthclientscontroller_test.go index 4c2fd37600..45de84e40b 100644 --- a/pkg/controllers/oauthclientscontroller/oauthclientscontroller_test.go +++ b/pkg/controllers/oauthclientscontroller/oauthclientscontroller_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "testing" + "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -14,14 +15,17 @@ import ( clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" + clocktesting "k8s.io/utils/clock/testing" configv1 "github.com/openshift/api/config/v1" oauthv1 "github.com/openshift/api/oauth/v1" 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" ) @@ -104,13 +108,39 @@ func newRouteLister(t *testing.T, routes ...*routev1.Route) routev1listers.Route return routev1listers.NewRouteLister(routeIndexer) } +func newAuthLister(t *testing.T) configv1listers.AuthenticationLister { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + indexer.Add(&configv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: configv1.AuthenticationSpec{ + Type: configv1.AuthenticationTypeIntegratedOAuth, + }, + }) + return configv1listers.NewAuthenticationLister(indexer) +} + func newTestOAuthsClientsController(t *testing.T) (*oauthsClientsController, cache.Indexer) { indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + oauthClientset := fakeoauthclient.NewSimpleClientset() + switchedInformer := oauthclientsswitchedinformer.NewSwitchedInformer( + "TestOAuthClientsInformerWithSwitchController", + context.TODO(), + func() (bool, error) { return false, nil }, + oauthinformers.NewSharedInformerFactoryWithOptions(oauthClientset, 1*time.Minute).Oauth().V1().OAuthClients(), + 0, + nil, + events.NewInMemoryRecorder("oauthclientscontroller_test", clocktesting.NewFakePassiveClock(time.Now())), + ) + return &oauthsClientsController{ - oauthClientClient: fakeoauthclient.NewSimpleClientset().OauthV1().OAuthClients(), - oauthClientLister: oauthv1listers.NewOAuthClientLister(indexer), - routeLister: newRouteLister(t, defaultRoute), - ingressLister: newIngressLister(t, defaultIngress), + authLister: newAuthLister(t), + ingressLister: newIngressLister(t, defaultIngress), + oauthClientClient: oauthClientset.OauthV1().OAuthClients(), + oauthClientInformer: switchedInformer.Informer(), + oauthClientLister: oauthv1listers.NewOAuthClientLister(indexer), + routeLister: newRouteLister(t, defaultRoute), }, indexer } diff --git a/pkg/controllers/oauthclientsswitchedinformer/oauthclientsswitchedinformer_controller.go b/pkg/controllers/oauthclientsswitchedinformer/oauthclientsswitchedinformer_controller.go new file mode 100644 index 0000000000..0bf106e8e7 --- /dev/null +++ b/pkg/controllers/oauthclientsswitchedinformer/oauthclientsswitchedinformer_controller.go @@ -0,0 +1,130 @@ +package oauthclientsswitchedinformer + +import ( + "context" + "time" + + oauthinformersv1 "github.com/openshift/client-go/oauth/informers/externalversions/oauth/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + + "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" +) + +// InformerWithSwitch is a controller that can start and stop an informer based on +// a condition func (shouldStopFn), that returns a bool and an error. If an error +// is returned, then the controller's sync will fail with that error. If no error +// is returned, then the controller stops/starts the informer based on the bool value +// (true means stop). +type InformerWithSwitch struct { + delegateInformer oauthinformersv1.OAuthClientInformer + switchController factory.Controller + shouldStopFn func() (bool, error) + parentCtx context.Context + runCtx context.Context + stopFunc func() +} + +type alwaysSyncedInformer struct { + isRunning func() bool + cache.SharedIndexInformer +} + +// HasSynced returns true when the informer's caches have synced, false otherwise. +// Since the SwitchedInformer can be stopped, waiting for its cache to sync can lead to +// timeouts, as a stopped informer will never sync. We override the HasSynced() +// method to always return true when stopped; clients should explicitly call cache.WaitForCacheSync. +func (s *alwaysSyncedInformer) HasSynced() bool { + if s.isRunning() { + return s.SharedIndexInformer.HasSynced() + } + return true +} + +func NewSwitchedInformer( + name string, + ctx context.Context, + shouldStopFn func() (bool, error), + delegateInformer oauthinformersv1.OAuthClientInformer, + resync time.Duration, + informers []factory.Informer, + recorder events.Recorder, +) *InformerWithSwitch { + + s := &InformerWithSwitch{ + parentCtx: ctx, + delegateInformer: delegateInformer, + shouldStopFn: shouldStopFn, + } + + controllerFactory := factory.New().WithSync(s.sync) + + if len(informers) > 0 { + controllerFactory.WithInformers(informers...) + } + + if resync > 0 { + controllerFactory.ResyncEvery(resync) + } + + s.switchController = controllerFactory.ToController(name, recorder) + return s +} + +func (s *InformerWithSwitch) Controller() factory.Controller { + return s.switchController +} + +func (s *InformerWithSwitch) Informer() cache.SharedIndexInformer { + return &alwaysSyncedInformer{ + isRunning: func() bool { return s.runCtx != nil }, + SharedIndexInformer: s.delegateInformer.Informer(), + } +} + +func (s *InformerWithSwitch) Start(stopCh <-chan struct{}) { + go s.switchController.Run(s.parentCtx, 1) + go func() { + <-stopCh + s.stop() + }() +} + +func (s *InformerWithSwitch) ensureRunning() { + if s.runCtx != nil { + return + } + + klog.Infof("%s delegate informer starting", s.switchController.Name()) + s.runCtx, s.stopFunc = context.WithCancel(s.parentCtx) + go s.delegateInformer.Informer().Run(s.runCtx.Done()) +} + +func (s *InformerWithSwitch) stop() { + if s.runCtx == nil { + return + } + + klog.Infof("%s delegate informer stopping", s.switchController.Name()) + s.stopFunc() + s.runCtx = nil + s.stopFunc = nil +} + +func (s *InformerWithSwitch) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if s.shouldStopFn != nil { + shouldStop, err := s.shouldStopFn() + if err != nil { + return err + } + + if shouldStop { + s.stop() + return nil + } + } + + s.ensureRunning() + return nil +} diff --git a/pkg/controllers/oauthclientsswitchedinformer/oauthclientsswitchedinformer_controller_test.go b/pkg/controllers/oauthclientsswitchedinformer/oauthclientsswitchedinformer_controller_test.go new file mode 100644 index 0000000000..2ef68d9b8d --- /dev/null +++ b/pkg/controllers/oauthclientsswitchedinformer/oauthclientsswitchedinformer_controller_test.go @@ -0,0 +1,131 @@ +package oauthclientsswitchedinformer + +import ( + "context" + "fmt" + "testing" + "time" + + oauthv1 "github.com/openshift/api/oauth/v1" + fakeoauthclient "github.com/openshift/client-go/oauth/clientset/versioned/fake" + oauthinformers "github.com/openshift/client-go/oauth/informers/externalversions" + "github.com/openshift/library-go/pkg/operator/events" + "k8s.io/apimachinery/pkg/util/wait" + clocktesting "k8s.io/utils/clock/testing" +) + +func TestSync(t *testing.T) { + testCtx, testCancel := context.WithCancel(context.TODO()) + defer testCancel() + + testOAuthClient := &oauthv1.OAuthClient{} + testClient := fakeoauthclient.NewSimpleClientset(testOAuthClient) + testInformer := oauthinformers.NewSharedInformerFactory(testClient, 0).Oauth().V1().OAuthClients() + + var makeItStop bool + var shouldStopFnErr error + shouldStopFn := func() (bool, error) { + return makeItStop, shouldStopFnErr + } + + informerSwitch := NewSwitchedInformer( + "TestInformerWithSwitchController", + testCtx, + shouldStopFn, + testInformer, + 0, + nil, + events.NewInMemoryRecorder("oauthclientscontroller_test", clocktesting.NewFakePassiveClock(time.Now())), + ) + + t.Run("start informer", func(tt *testing.T) { + makeItStop = false + shouldStopFnErr = nil + err := informerSwitch.sync(testCtx, nil) + if err != nil { + tt.Errorf("unexpected sync error: %v", err) + } + waitForInformerSynced(tt, testCtx, informerSwitch) + + // informer should be running + + if informerSwitch.runCtx == nil { + tt.Error("EnsureRunning: runCtx is nil when it should be non-nil") + } + + if informerSwitch.stopFunc == nil { + tt.Error("EnsureRunning: stopFunc is nil when it should be non-nil") + } + + if informerSwitch.Informer().IsStopped() { + tt.Error("EnsureRunning: informer is stopped when it should be started") + } + }) + + t.Run("stop informer with error", func(tt *testing.T) { + makeItStop = true + shouldStopFnErr = fmt.Errorf("stop fails") + err := informerSwitch.sync(testCtx, nil) + if err == nil { + tt.Errorf("got no error while expecting one") + } + waitForInformerSynced(tt, testCtx, informerSwitch) + + // informer should still be running + + if informerSwitch.runCtx == nil { + tt.Error("EnsureRunning: runCtx is nil when it should be non-nil") + } + + if informerSwitch.stopFunc == nil { + tt.Error("EnsureRunning: stopFunc is nil when it should be non-nil") + } + + if informerSwitch.Informer().IsStopped() { + tt.Error("EnsureRunning: informer is stopped when it should be started") + } + + }) + + t.Run("stop informer without error", func(tt *testing.T) { + makeItStop = true + shouldStopFnErr = nil + err := informerSwitch.sync(testCtx, nil) + if err != nil { + tt.Errorf("unexpected sync error: %v", err) + } + waitForInformerStopped(tt, testCtx, informerSwitch) + + // informer should stop + + if informerSwitch.runCtx != nil { + tt.Error("Stop: runCtx is not nil when it should be nil") + } + + if informerSwitch.stopFunc != nil { + tt.Error("Stop: stopFunc is not nil when it should be nil") + } + + if !informerSwitch.Informer().IsStopped() { + tt.Error("Stop: informer is started when it should be stopped") + } + }) +} + +func waitForInformerSynced(t *testing.T, ctx context.Context, informerSwitch *InformerWithSwitch) { + err := wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 1*time.Second, true, func(ctx context.Context) (done bool, err error) { + return informerSwitch.Informer().HasSynced(), nil + }) + if err != nil { + t.Fatalf("unexpected error while waiting for informer to sync: %v", err) + } +} + +func waitForInformerStopped(t *testing.T, ctx context.Context, informerSwitch *InformerWithSwitch) { + err := wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 1*time.Second, true, func(ctx context.Context) (done bool, err error) { + return informerSwitch.Informer().IsStopped(), nil + }) + if err != nil { + t.Fatalf("unexpected error while waiting for informer to stop: %v", err) + } +} diff --git a/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go b/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go index dfb2e2d898..a5165d09b0 100644 --- a/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go +++ b/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go @@ -15,8 +15,10 @@ import ( corev1listers "k8s.io/client-go/listers/core/v1" routev1 "github.com/openshift/api/route/v1" + configinformer "github.com/openshift/client-go/config/informers/externalversions" configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1" configv1lister "github.com/openshift/client-go/config/listers/config/v1" + operatorv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" routev1informers "github.com/openshift/client-go/route/informers/externalversions/route/v1" routev1listers "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/library-go/pkg/controller/factory" @@ -34,6 +36,9 @@ func NewOAuthRouteCheckController( kubeInformersForConfigManagedNS informers.SharedInformerFactory, routeInformerNamespaces routev1informers.RouteInformer, ingressInformerAllNamespaces configv1informers.IngressInformer, + operatorConfigInformer configinformer.SharedInformerFactory, + kasInformer operatorv1.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, systemCABundle []byte, recorder events.Recorder, ) factory.Controller { @@ -55,10 +60,18 @@ func NewOAuthRouteCheckController( return getOAuthRouteTLSConfig(cmLister, secretLister, ingressLister, systemCABundle) } + endpointCheckDisabledFunc := func() (bool, error) { + return common.ExternalOIDCConfigAvailable( + operatorConfigInformer.Config().V1().Authentications().Lister(), + kasInformer.Lister(), + kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), + ) + } + return endpointaccessible.NewEndpointAccessibleController( "OAuthServerRoute", operatorClient, - endpointListFunc, getTLSConfigFunc, + endpointListFunc, getTLSConfigFunc, endpointCheckDisabledFunc, []factory.Informer{ cmInformer, secretInformer, @@ -72,6 +85,9 @@ func NewOAuthRouteCheckController( func NewOAuthServiceCheckController( operatorClient v1helpers.OperatorClient, kubeInformersForTargetNS informers.SharedInformerFactory, + operatorConfigInformer configinformer.SharedInformerFactory, + kasInformer operatorv1.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, recorder events.Recorder, ) factory.Controller { endpointsListFunc := func() ([]string, error) { @@ -82,10 +98,18 @@ func NewOAuthServiceCheckController( return getOAuthEndpointTLSConfig(kubeInformersForTargetNS.Core().V1().ConfigMaps().Lister()) } + endpointCheckDisabledFunc := func() (bool, error) { + return common.ExternalOIDCConfigAvailable( + operatorConfigInformer.Config().V1().Authentications().Lister(), + kasInformer.Lister(), + kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), + ) + } + return endpointaccessible.NewEndpointAccessibleController( "OAuthServerService", operatorClient, - endpointsListFunc, getTLSConfigFunc, + endpointsListFunc, getTLSConfigFunc, endpointCheckDisabledFunc, []factory.Informer{ kubeInformersForTargetNS.Core().V1().ConfigMaps().Informer(), kubeInformersForTargetNS.Core().V1().Services().Informer(), @@ -98,6 +122,9 @@ func NewOAuthServiceCheckController( func NewOAuthServiceEndpointsCheckController( operatorClient v1helpers.OperatorClient, kubeInformersForTargetNS informers.SharedInformerFactory, + operatorConfigInformer configinformer.SharedInformerFactory, + kasInformer operatorv1.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, recorder events.Recorder, ) factory.Controller { endpointsListFn := func() ([]string, error) { @@ -108,10 +135,18 @@ func NewOAuthServiceEndpointsCheckController( return getOAuthEndpointTLSConfig(kubeInformersForTargetNS.Core().V1().ConfigMaps().Lister()) } + endpointCheckDisabledFunc := func() (bool, error) { + return common.ExternalOIDCConfigAvailable( + operatorConfigInformer.Config().V1().Authentications().Lister(), + kasInformer.Lister(), + kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), + ) + } + return endpointaccessible.NewEndpointAccessibleController( "OAuthServerServiceEndpoints", operatorClient, - endpointsListFn, getTLSConfigFunc, + endpointsListFn, getTLSConfigFunc, endpointCheckDisabledFunc, []factory.Informer{ kubeInformersForTargetNS.Core().V1().Endpoints().Informer(), kubeInformersForTargetNS.Core().V1().ConfigMaps().Informer(), diff --git a/pkg/controllers/payload/payload_config_controller.go b/pkg/controllers/payload/payload_config_controller.go index bfa72b374e..6f862da95b 100644 --- a/pkg/controllers/payload/payload_config_controller.go +++ b/pkg/controllers/payload/payload_config_controller.go @@ -10,6 +10,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -25,6 +26,10 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" osinv1 "github.com/openshift/api/osin/v1" routev1 "github.com/openshift/api/route/v1" + configinformers "github.com/openshift/client-go/config/informers/externalversions" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions/route/v1" routev1lister "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/library-go/pkg/controller/factory" @@ -66,11 +71,18 @@ type payloadConfigController struct { configMaps corev1client.ConfigMapsGetter secrets corev1client.SecretsGetter operatorClient v1helpers.OperatorClient + + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1lister.ConfigMapLister } func NewPayloadConfigController( instanceName string, kubeInformersForTargetNamespace informers.SharedInformerFactory, + operatorConfigInformers configinformers.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, secrets corev1client.SecretsGetter, configMaps corev1client.ConfigMapsGetter, operatorClient v1helpers.OperatorClient, @@ -83,6 +95,10 @@ func NewPayloadConfigController( secrets: secrets, configMaps: configMaps, operatorClient: operatorClient, + + authLister: operatorConfigInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New().WithInformers( kubeInformersForTargetNamespace.Core().V1().Secrets().Informer(), @@ -90,6 +106,8 @@ func NewPayloadConfigController( kubeInformersForTargetNamespace.Core().V1().ConfigMaps().Informer(), routeInformer.Informer(), operatorClient.Informer(), + operatorConfigInformers.Config().V1().Authentications().Informer(), + kasInformer.Informer(), ).ResyncEvery(wait.Jitter(time.Minute, 1.0)).WithSync(c.sync).ToController(c.controllerInstanceName, recorder.WithComponentSuffix("payload-config-controller")) } @@ -138,6 +156,16 @@ func (c *payloadConfigController) getSessionSecret(ctx context.Context, recorder } func (c *payloadConfigController) sync(ctx context.Context, syncContext factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + if err := c.removeOperands(ctx); err != nil { + return err + } + + return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, knownConditionNames, nil) + } + foundConditions := []operatorv1.OperatorCondition{} foundConditions = append(foundConditions, c.getSessionSecret(ctx, syncContext.Recorder())...) @@ -159,6 +187,18 @@ func (c *payloadConfigController) sync(ctx context.Context, syncContext factory. return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, knownConditionNames, foundConditions) } +func (c *payloadConfigController) removeOperands(ctx context.Context) error { + if err := c.secrets.Secrets("openshift-authentication").Delete(ctx, "v4-0-config-system-session", metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) { + return err + } + + if err := c.configMaps.ConfigMaps("openshift-authentication").Delete(ctx, "v4-0-config-system-cliconfig", metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) { + return err + } + + return nil +} + func (c *payloadConfigController) handleOAuthConfig(ctx context.Context, operatorSpec *operatorv1.OperatorSpec, route *routev1.Route, service *corev1.Service, recorder events.Recorder) []operatorv1.OperatorCondition { ca := "/var/config/system/configmaps/v4-0-config-system-service-ca/service-ca.crt" cliConfig := &osinv1.OsinServerConfig{ diff --git a/pkg/controllers/proxyconfig/proxyconfig_controller.go b/pkg/controllers/proxyconfig/proxyconfig_controller.go index 897dd9b2f1..f776544323 100644 --- a/pkg/controllers/proxyconfig/proxyconfig_controller.go +++ b/pkg/controllers/proxyconfig/proxyconfig_controller.go @@ -12,11 +12,17 @@ import ( "golang.org/x/net/http/httpproxy" + "k8s.io/client-go/informers" corev1lister "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" + configinformer "github.com/openshift/client-go/config/informers/externalversions" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions/route/v1" v1 "github.com/openshift/client-go/route/listers/route/v1" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" @@ -30,11 +36,18 @@ type proxyConfigChecker struct { routeName string routeNamespace string caConfigMaps map[string][]string // ns -> []configmapNames + + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1lister.ConfigMapLister } func NewProxyConfigChecker( routeInformer routeinformer.RouteInformer, configMapInformers v1helpers.KubeInformersForNamespaces, + configInformers configinformer.SharedInformerFactory, + kasInformer operatorv1.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, routeNamespace string, routeName string, caConfigMaps map[string][]string, @@ -46,6 +59,10 @@ func NewProxyConfigChecker( routeName: routeName, routeNamespace: routeNamespace, caConfigMaps: caConfigMaps, + + authLister: configInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } c := factory.New(). @@ -68,6 +85,12 @@ func NewProxyConfigChecker( // sync attempts to connect to route using configured proxy settings and reports any error. func (p *proxyConfigChecker) sync(ctx context.Context, _ factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(p.authLister, p.kasLister, p.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + return nil + } + proxyConfig := httpproxy.FromEnvironment() if !isProxyConfigured(proxyConfig) { // If proxy is not configured, then it is a no-op. diff --git a/pkg/controllers/readiness/wellknown_ready_controller.go b/pkg/controllers/readiness/wellknown_ready_controller.go index 578ab9e8f9..1904bbcdd6 100644 --- a/pkg/controllers/readiness/wellknown_ready_controller.go +++ b/pkg/controllers/readiness/wellknown_ready_controller.go @@ -19,6 +19,8 @@ import ( configinformer "github.com/openshift/client-go/config/informers/externalversions" configv1lister "github.com/openshift/client-go/config/listers/config/v1" applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions/route/v1" routev1lister "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" @@ -26,12 +28,14 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" netutil "k8s.io/apimachinery/pkg/util/net" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/informers" corev1lister "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" ) @@ -58,12 +62,23 @@ type wellKnownReadyController struct { configMapLister corev1lister.ConfigMapLister routeLister routev1lister.RouteLister infrastructureLister configv1lister.InfrastructureLister + + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1lister.ConfigMapLister } const controllerName = "WellKnownReadyController" -func NewWellKnownReadyController(instanceName string, kubeInformers v1helpers.KubeInformersForNamespaces, configInformers configinformer.SharedInformerFactory, routeInformer routeinformer.RouteInformer, - operatorClient v1helpers.OperatorClient, recorder events.Recorder) factory.Controller { +func NewWellKnownReadyController( + instanceName string, + kubeInformers v1helpers.KubeInformersForNamespaces, + configInformers configinformer.SharedInformerFactory, + routeInformer routeinformer.RouteInformer, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, + operatorClient v1helpers.OperatorClient, + recorder events.Recorder, +) factory.Controller { nsOpenshiftConfigManagedInformers := kubeInformers.InformersFor("openshift-config-managed") nsDefaultInformers := kubeInformers.InformersFor("default") @@ -77,6 +92,9 @@ func NewWellKnownReadyController(instanceName string, kubeInformers v1helpers.Ku configMapLister: nsOpenshiftConfigManagedInformers.Core().V1().ConfigMaps().Lister(), routeLister: routeInformer.Lister(), operatorClient: operatorClient, + + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New().WithInformers( @@ -128,6 +146,15 @@ func (c *wellKnownReadyController) sync(ctx context.Context, controllerContext f } }() + // if OIDC is enabled, clear operator conditions and skip checks + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + available = available.WithStatus(operatorv1.ConditionTrue) + progressing = progressing.WithStatus(operatorv1.ConditionFalse) + return nil + } + // the well-known endpoint cannot be ready until we know the oauth-server's hostname _, err = c.routeLister.Routes("openshift-authentication").Get("oauth-openshift") if apierrors.IsNotFound(err) { @@ -185,7 +212,7 @@ func (c *wellKnownReadyController) isWellknownEndpointsReady(ctx context.Context return nil } - caData, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/ca.crt") + caData, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/ca.crt") if err != nil { return fmt.Errorf("failed to read SA ca.crt: %v", err) } diff --git a/pkg/controllers/routercerts/controller.go b/pkg/controllers/routercerts/controller.go index 4327b009b5..c3fc7b190d 100644 --- a/pkg/controllers/routercerts/controller.go +++ b/pkg/controllers/routercerts/controller.go @@ -15,9 +15,12 @@ import ( "k8s.io/klog/v2" operatorv1 "github.com/openshift/api/operator/v1" + configinformers "github.com/openshift/client-go/config/informers/externalversions" configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1" configv1listers "github.com/openshift/client-go/config/listers/config/v1" applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" @@ -45,6 +48,10 @@ type routerCertsDomainValidationController struct { customSecretName string routeName string + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1listers.ConfigMapLister + systemCertPool func() (*x509.CertPool, error) // enables unit testing } @@ -57,6 +64,9 @@ func NewRouterCertsDomainValidationController( targetNSsecretInformer corev1informers.SecretInformer, machineConfigNSSecretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer, + operatorConfigInformers configinformers.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer corev1informers.ConfigMapInformer, secretNamespace string, defaultSecretName string, customSecretName string, @@ -74,6 +84,10 @@ func NewRouterCertsDomainValidationController( customSecretName: customSecretName, routeName: routeName, systemCertPool: x509.SystemCertPool, + + authLister: operatorConfigInformers.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Lister(), } return factory.New(). @@ -81,7 +95,9 @@ func NewRouterCertsDomainValidationController( operatorClient.Informer(), ingressInformer.Informer(), targetNSsecretInformer.Informer(), - configMapInformer.Informer()). + configMapInformer.Informer(), + operatorConfigInformers.Config().V1().Authentications().Informer(), + kasInformer.Informer()). WithFilteredEventsInformers( factory.NamesFilter("router-certs"), machineConfigNSSecretInformer.Informer(), @@ -117,6 +133,14 @@ func (c *routerCertsDomainValidationController) sync(ctx context.Context, syncCt WithMessage(condition.Message))) }() + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + // do not remove secret "v4-0-config-system-router-certs" as the ConfigObserver controller + // monitors it and it will go degraded if missing + return nil + } + // get ingress ingress, err := c.ingressLister.Get("cluster") if err != nil { diff --git a/pkg/controllers/routercerts/controller_test.go b/pkg/controllers/routercerts/controller_test.go index 999e529658..52edc5ce4d 100644 --- a/pkg/controllers/routercerts/controller_test.go +++ b/pkg/controllers/routercerts/controller_test.go @@ -238,11 +238,21 @@ func TestValidateRouterCertificates(t *testing.T) { secretsClient = fake.NewSimpleClientset() } + authIndexer := cache.NewIndexer(func(_ interface{}) (string, error) { + return "cluster", nil + }, cache.Indexers{}) + authIndexer.Add(&configv1.Authentication{ + Spec: configv1.AuthenticationSpec{ + Type: configv1.AuthenticationTypeIntegratedOAuth, + }, + }) + controller := routerCertsDomainValidationController{ operatorClient: operatorClient, ingressLister: configv1listers.NewIngressLister(ingresses), secretLister: corev1listers.NewSecretLister(secrets), configMapLister: corev1listers.NewConfigMapLister(configMapsIndexer), + authLister: configv1listers.NewAuthenticationLister(authIndexer), secretNamespace: "openshift-authentication", defaultSecretName: "v4-0-config-system-router-certs", customSecretName: "v4-0-config-system-custom-router-certs", diff --git a/pkg/controllers/serviceca/service_ca_controller.go b/pkg/controllers/serviceca/service_ca_controller.go index 5f1725f3bf..9606a46fcf 100644 --- a/pkg/controllers/serviceca/service_ca_controller.go +++ b/pkg/controllers/serviceca/service_ca_controller.go @@ -18,6 +18,9 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" configinformers "github.com/openshift/client-go/config/informers/externalversions" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" @@ -37,18 +40,36 @@ type serviceCAController struct { controllerInstanceName string serviceLister corev1lister.ServiceLister secretLister corev1lister.SecretLister + configMapLister corev1lister.ConfigMapLister configMaps corev1client.ConfigMapsGetter operatorClient v1helpers.OperatorClient + + authLister configv1listers.AuthenticationLister + kasLister operatorv1listers.KubeAPIServerLister + kasConfigMapLister corev1lister.ConfigMapLister } -func NewServiceCAController(instanceName string, kubeInformersForTargetNamespace informers.SharedInformerFactory, configInformer configinformers.SharedInformerFactory, configMaps corev1client.ConfigMapsGetter, - operatorClient v1helpers.OperatorClient, recorder events.Recorder) factory.Controller { +func NewServiceCAController( + instanceName string, + kubeInformersForTargetNamespace informers.SharedInformerFactory, + configInformer configinformers.SharedInformerFactory, + kasInformer operatorv1informers.KubeAPIServerInformer, + kasConfigMapInformer informers.SharedInformerFactory, + configMaps corev1client.ConfigMapsGetter, + operatorClient v1helpers.OperatorClient, + recorder events.Recorder, +) factory.Controller { c := &serviceCAController{ controllerInstanceName: factory.ControllerInstanceName(instanceName, "ServiceCA"), serviceLister: kubeInformersForTargetNamespace.Core().V1().Services().Lister(), secretLister: kubeInformersForTargetNamespace.Core().V1().Secrets().Lister(), + configMapLister: kubeInformersForTargetNamespace.Core().V1().ConfigMaps().Lister(), configMaps: configMaps, operatorClient: operatorClient, + + authLister: configInformer.Config().V1().Authentications().Lister(), + kasLister: kasInformer.Lister(), + kasConfigMapLister: kasConfigMapInformer.Core().V1().ConfigMaps().Lister(), } return factory.New().WithInformers( kubeInformersForTargetNamespace.Core().V1().Secrets().Informer(), @@ -56,6 +77,7 @@ func NewServiceCAController(instanceName string, kubeInformersForTargetNamespace kubeInformersForTargetNamespace.Core().V1().ConfigMaps().Informer(), configInformer.Config().V1().Authentications().Informer(), configInformer.Config().V1().Ingresses().Informer(), + kasInformer.Informer(), ).ResyncEvery( wait.Jitter(time.Minute, 1.0), ).WithSync( @@ -66,6 +88,22 @@ func NewServiceCAController(instanceName string, kubeInformersForTargetNamespace } func (c *serviceCAController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(c.authLister, c.kasLister, c.kasConfigMapLister); err != nil { + return err + } else if oidcAvailable { + if _, err := c.configMapLister.ConfigMaps("openshift-authentication").Get("v4-0-config-system-service-ca"); errors.IsNotFound(err) { + return nil + } else if err != nil { + return err + } + + if err := c.configMaps.ConfigMaps("openshift-authentication").Delete(ctx, "v4-0-config-system-service-ca", metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) { + return err + } + + return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, knownConditionNames, nil) + } + foundConditions := []operatorv1.OperatorCondition{} _, serviceConditions := common.GetOAuthServerService(c.serviceLister, "OAuthService") diff --git a/pkg/libs/endpointaccessible/endpoint_accessible_controller.go b/pkg/libs/endpointaccessible/endpoint_accessible_controller.go index e4a6dd96dc..2d7bc58d96 100644 --- a/pkg/libs/endpointaccessible/endpoint_accessible_controller.go +++ b/pkg/libs/endpointaccessible/endpoint_accessible_controller.go @@ -21,15 +21,17 @@ import ( ) type endpointAccessibleController struct { - controllerInstanceName string - operatorClient v1helpers.OperatorClient - endpointListFn EndpointListFunc - getTLSConfigFn EndpointTLSConfigFunc - availableConditionName string + controllerInstanceName string + operatorClient v1helpers.OperatorClient + endpointListFn EndpointListFunc + getTLSConfigFn EndpointTLSConfigFunc + availableConditionName string + endpointCheckDisabledFunc EndpointCheckDisabledFunc } type EndpointListFunc func() ([]string, error) type EndpointTLSConfigFunc func() (*tls.Config, error) +type EndpointCheckDisabledFunc func() (bool, error) // NewEndpointAccessibleController returns a controller that checks if the endpoints // listed by endpointListFn are reachable @@ -38,17 +40,19 @@ func NewEndpointAccessibleController( operatorClient v1helpers.OperatorClient, endpointListFn EndpointListFunc, getTLSConfigFn EndpointTLSConfigFunc, + endpointCheckDisabledFunc EndpointCheckDisabledFunc, triggers []factory.Informer, recorder events.Recorder, ) factory.Controller { controllerName := name + "EndpointAccessibleController" c := &endpointAccessibleController{ - controllerInstanceName: factory.ControllerInstanceName(name, "EndpointAccessible"), - operatorClient: operatorClient, - endpointListFn: endpointListFn, - getTLSConfigFn: getTLSConfigFn, - availableConditionName: name + "EndpointAccessibleControllerAvailable", + controllerInstanceName: factory.ControllerInstanceName(name, "EndpointAccessible"), + operatorClient: operatorClient, + endpointListFn: endpointListFn, + getTLSConfigFn: getTLSConfigFn, + availableConditionName: name + "EndpointAccessibleControllerAvailable", + endpointCheckDisabledFunc: endpointCheckDisabledFunc, } return factory.New(). @@ -73,6 +77,12 @@ func humanizeError(err error) error { } func (c *endpointAccessibleController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if c.endpointCheckDisabledFunc != nil { + if skip, err := c.endpointCheckDisabledFunc(); skip || err != nil { + return err + } + } + endpoints, err := c.endpointListFn() if err != nil { if apierrors.IsNotFound(err) { diff --git a/pkg/operator/replacement_starter.go b/pkg/operator/replacement_starter.go index 4a8d388896..6fdb5a0245 100644 --- a/pkg/operator/replacement_starter.go +++ b/pkg/operator/replacement_starter.go @@ -24,7 +24,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" @@ -232,7 +231,6 @@ type authenticationOperatorInformerFactories struct { kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces operatorConfigInformer configinformer.SharedInformerFactory operatorInformer operatorinformer.SharedInformerFactory - oauthInformers oauthinformers.SharedInformerFactory apiregistrationInformers apiregistrationinformers.SharedInformerFactory migrationInformer migrationv1alpha1informer.SharedInformerFactory // TODO remove @@ -257,7 +255,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), @@ -274,7 +271,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), diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index 95e27123bd..9a47ec2bcc 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -5,7 +5,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "fmt" - "io/ioutil" "os" "time" @@ -15,8 +14,12 @@ import ( "github.com/openshift/api/features" 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" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation/configobservercontroller" componentroutesecretsync "github.com/openshift/cluster-authentication-operator/pkg/controllers/customroute" "github.com/openshift/cluster-authentication-operator/pkg/controllers/deployment" @@ -25,6 +28,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" @@ -38,6 +42,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" @@ -66,9 +71,10 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/sets" + corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" - utilpointer "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ) const ( @@ -130,25 +136,38 @@ func prepareOauthOperator( authOperatorInput.eventRecorder, ) + authLister := informerFactories.operatorConfigInformer.Config().V1().Authentications().Lister() + kasLister := informerFactories.operatorInformer.Operator().V1().KubeAPIServers().Lister() + kasConfigMapLister := informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver").Core().V1().ConfigMaps().Lister() + staticResourceController := staticresourcecontroller.NewStaticResourceController( "OpenshiftAuthenticationStaticResources", bindata.Asset, - []string{ + []string{ // always created, never deleted "oauth-openshift/audit-policy.yaml", "oauth-openshift/ns.yaml", "oauth-openshift/authentication-clusterrolebinding.yaml", - "oauth-openshift/cabundle.yaml", - "oauth-openshift/branding-secret.yaml", "oauth-openshift/serviceaccount.yaml", - "oauth-openshift/oauth-service.yaml", "oauth-openshift/trust_distribution_role.yaml", "oauth-openshift/trust_distribution_rolebinding.yaml", - "oauth-openshift/authorization.openshift.io_rolebindingrestrictions.yaml", }, resourceapply.NewKubeClientHolder(authOperatorInput.kubeClient).WithAPIExtensionsClient(authOperatorInput.apiextensionClient), authOperatorInput.authenticationOperatorClient, authOperatorInput.eventRecorder, - ).AddKubeInformers(informerFactories.kubeInformersForNamespaces) + ).AddKubeInformers(informerFactories.kubeInformersForNamespaces). + WithConditionalResources(bindata.Asset, + []string{ // created if external OIDC disabled, deleted otherwise + "oauth-openshift/cabundle.yaml", + "oauth-openshift/branding-secret.yaml", + "oauth-openshift/oauth-service.yaml", + "oauth-openshift/authorization.openshift.io_rolebindingrestrictions.yaml", + }, + func() bool { + oidcAvailable, _ := common.ExternalOIDCConfigAvailable(authLister, kasLister, kasConfigMapLister) + return !oidcAvailable + }, + nil, + ) configObserver := configobservercontroller.NewConfigObserver( authOperatorInput.authenticationOperatorClient, @@ -168,6 +187,9 @@ func prepareOauthOperator( informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication").Core().V1().Secrets(), informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().Secrets(), informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().ConfigMaps(), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver").Core().V1().ConfigMaps(), "openshift-authentication", "v4-0-config-system-router-certs", "v4-0-config-system-custom-router-certs", @@ -177,6 +199,9 @@ func prepareOauthOperator( ingressStateController := ingressstate.NewIngressStateController( "openshift-authentication", informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.kubeClient.CoreV1(), authOperatorInput.kubeClient.CoreV1(), authOperatorInput.authenticationOperatorClient, @@ -188,6 +213,8 @@ func prepareOauthOperator( informerFactories.kubeInformersForNamespaces, informerFactories.operatorConfigInformer, informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(), + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.authenticationOperatorClient, authOperatorInput.eventRecorder, ) @@ -197,6 +224,8 @@ func prepareOauthOperator( informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), informerFactories.operatorConfigInformer, informerFactories.namespacedOpenshiftAuthenticationRoutes, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.kubeClient.CoreV1(), authOperatorInput.routeClient.RouteV1().Routes("openshift-authentication"), authOperatorInput.configClient.ConfigV1().Authentications(), @@ -208,6 +237,8 @@ func prepareOauthOperator( "openshift-authentication", informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.kubeClient.CoreV1(), authOperatorInput.authenticationOperatorClient, authOperatorInput.eventRecorder, @@ -216,6 +247,9 @@ func prepareOauthOperator( payloadConfigController := payload.NewPayloadConfigController( "openshift-authentication", informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.kubeClient.CoreV1(), authOperatorInput.kubeClient.CoreV1(), authOperatorInput.authenticationOperatorClient, @@ -223,12 +257,29 @@ 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(), + 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(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.eventRecorder, ) @@ -245,6 +296,8 @@ func prepareOauthOperator( authOperatorInput.eventRecorder, versionRecorder, informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), ) workersAvailableController := ingressnodesavailable.NewIngressNodesAvailableController( @@ -253,6 +306,9 @@ func prepareOauthOperator( informerFactories.operatorInformer.Operator().V1().IngressControllers(), authOperatorInput.eventRecorder, informerFactories.kubeInformersForNamespaces.InformersFor("").Core().V1().Nodes(), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), ) systemCABundle, err := loadSystemCACertBundle() @@ -266,6 +322,9 @@ func prepareOauthOperator( informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config-managed"), informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(), informerFactories.operatorConfigInformer.Config().V1().Ingresses(), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), systemCABundle, authOperatorInput.eventRecorder, ) @@ -273,18 +332,27 @@ func prepareOauthOperator( authServiceCheckController := oauthendpoints.NewOAuthServiceCheckController( authOperatorInput.authenticationOperatorClient, informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.eventRecorder, ) authServiceEndpointCheckController := oauthendpoints.NewOAuthServiceEndpointsCheckController( authOperatorInput.authenticationOperatorClient, informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.eventRecorder, ) proxyConfigController := proxyconfig.NewProxyConfigChecker( informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(), informerFactories.kubeInformersForNamespaces, + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), "openshift-authentication", "oauth-openshift", map[string][]string{ @@ -305,6 +373,9 @@ func prepareOauthOperator( informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(), authOperatorInput.routeClient.RouteV1().Routes("openshift-authentication"), informerFactories.kubeInformersForNamespaces, + informerFactories.operatorConfigInformer, + informerFactories.operatorInformer.Operator().V1().KubeAPIServers(), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver"), authOperatorInput.authenticationOperatorClient, authOperatorInput.eventRecorder, resourceSyncController, @@ -326,6 +397,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), @@ -348,6 +420,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), @@ -433,6 +506,10 @@ func prepareOauthAPIServerOperator( const apiServerConditionsPrefix = "APIServer" + authLister := informerFactories.operatorConfigInformer.Config().V1().Authentications().Lister() + kasLister := informerFactories.operatorInformer.Operator().V1().KubeAPIServers().Lister() + kasConfigMapLister := informerFactories.kubeInformersForNamespaces.InformersFor("openshift-kube-apiserver").Core().V1().ConfigMaps().Lister() + apiServerControllers, err := apiservercontrollerset.NewAPIServerControllerSet( "oauth-apiserver", authOperatorInput.authenticationOperatorClient, @@ -519,9 +596,7 @@ func prepareOauthAPIServerOperator( ).WithAPIServiceController( "openshift-apiserver", "openshift-oauth-apiserver", - func() ([]*apiregistrationv1.APIService, []*apiregistrationv1.APIService, error) { - return apiServices(), nil, nil - }, + apiServicesFuncWrapper(authLister, kasLister, kasConfigMapLister), informerFactories.apiregistrationInformers, authOperatorInput.apiregistrationv1Client.ApiregistrationV1(), informerFactories.kubeInformersForNamespaces, @@ -735,7 +810,7 @@ func apiServices() []*apiregistrationv1.APIService { Service: &apiregistrationv1.ServiceReference{ Namespace: "openshift-oauth-apiserver", Name: "api", - Port: utilpointer.Int32Ptr(443), + Port: ptr.To(int32(443)), }, GroupPriorityMinimum: 9900, VersionPriority: 15, @@ -753,7 +828,7 @@ func apiServices() []*apiregistrationv1.APIService { // nil if it fails to load. It is to be used for controllers that generally require a // cert bundle and not necessary the system trust store contents. func loadSystemCACertBundle() ([]byte, error) { - systemCABundle, err := ioutil.ReadFile("/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem") + systemCABundle, err := os.ReadFile("/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem") if err != nil { // this may fail route-health checks in proxy environments klog.Warningf("unable to read system CA from /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem: %v", err) @@ -801,3 +876,17 @@ func extractOperatorStatus(obj *unstructured.Unstructured, fieldManager string) } return &ret.Status.OperatorStatusApplyConfiguration, nil } + +func apiServicesFuncWrapper(authLister configv1listers.AuthenticationLister, kasLister operatorv1listers.KubeAPIServerLister, kasConfigMapLister corev1listers.ConfigMapLister) func() ([]*apiregistrationv1.APIService, []*apiregistrationv1.APIService, error) { + return func() ([]*apiregistrationv1.APIService, []*apiregistrationv1.APIService, error) { + apiServices := apiServices() + if oidcAvailable, err := common.ExternalOIDCConfigAvailable(authLister, kasLister, kasConfigMapLister); err != nil { + return nil, nil, err + } else if oidcAvailable { + // return apiServices as disabled + return nil, apiServices, nil + } + + return apiServices, nil, nil + } +}