CONSOLE-4822: Refactor Console Operator Authentication Controllers

pkg/console/controllers/oauthclientsecret/oauthclientsecret.go → pkg/console/controllers/integratedoauth/integratedoauth.go

@@ -1,4 +1,4 @@
-package oauthclientsecret
+package integratedoauth
 
 import (
 	"context"
@@ -24,52 +24,50 @@ import (
 	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
 	v1helpers "github.com/openshift/library-go/pkg/operator/v1helpers"
 
-	authnsub "github.com/openshift/console-operator/pkg/console/subresource/authentication"
+	deploymentsub "github.com/openshift/console-operator/pkg/console/subresource/deployment"
 	secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
 )
 
-// oauthClientSecretController behaves differently based on authentication/cluster .spec.type:
+// integratedOAuthController manages client secrets for IntegratedOAuth and None authentication types.
 //
-//   - IntegratedOAuth - self-manage the client secret string
-//   - OIDC - lookup our client in the authentication/cluster .spec.oidcProviders[x].oidcClients
-//     slice and use the 'clientSecret' from the secret referred to by .clientSecret.name
-//   - None - do nothing
+// Responsibilities:
+//   - IntegratedOAuth - self-generate and manage the client secret string
+//   - None - self-generate and manage the client secret string (OAuth server still runs)
 //
 // The secret written is 'openshift-console/console-oauth-config' in .Data['clientSecret']
 //
+// Note: OIDC authentication type is handled by the oidcController, not this controller.
+//
 // ==========
 //
 //	writes:
 //	- secrets.console-oauth-config -n openshift-console .Data['clientSecret']
 //	- consoles.operator.openshift.io/cluster .status.conditions:
 //		- type=OAuthClientSecretSyncProgressing
 //		- type=OAuthClientSecretSyncDegraded
-type oauthClientSecretController struct {
+type integratedOAuthController struct {
 	operatorClient v1helpers.OperatorClient
 	secretsClient  corev1clients.SecretsGetter
 
 	authConfigLister      configv1listers.AuthenticationLister
 	consoleOperatorLister operatorv1listers.ConsoleLister
-	configSecretsLister   corev1listers.SecretLister
 	targetNSSecretsLister corev1listers.SecretLister
 }
 
-func NewOAuthClientSecretController(
+func NewIntegratedOAuthController(
 	operatorClient v1helpers.OperatorClient,
 	secretsClient corev1clients.SecretsGetter,
 	authnInformer configv1informers.AuthenticationInformer,
 	consoleOperatorInformer operatorv1informers.ConsoleInformer,
-	configSecretsInformer corev1informers.SecretInformer,
 	targetNSsecretsInformer corev1informers.SecretInformer,
 	recorder events.Recorder,
 ) factory.Controller {
-	c := &oauthClientSecretController{
+	c := &integratedOAuthController{
 		operatorClient: operatorClient,
 		secretsClient:  secretsClient,
 
 		authConfigLister:      authnInformer.Lister(),
 		consoleOperatorLister: consoleOperatorInformer.Lister(),
-		configSecretsLister:   configSecretsInformer.Lister(),
 		targetNSSecretsLister: targetNSsecretsInformer.Lister(),
 	}
 
@@ -78,15 +76,14 @@ func NewOAuthClientSecretController(
 		WithInformers(
 			authnInformer.Informer(),
 			consoleOperatorInformer.Informer(),
-			configSecretsInformer.Informer(),
 		).
 		WithFilteredEventsInformers(
-			factory.NamesFilter("console-oauth-config"), targetNSsecretsInformer.Informer(),
+			factory.NamesFilter(deploymentsub.ConsoleOauthConfigName), targetNSsecretsInformer.Informer(),
 		).
-		ToController("OAuthClientSecretController", recorder.WithComponentSuffix("oauthclient-secret-controller"))
+		ToController("IntegratedOAuthController", recorder.WithComponentSuffix("integrated-oauth-controller"))
 }
 
-func (c *oauthClientSecretController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
+func (c *integratedOAuthController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
 	if shouldSync, err := c.handleManaged(); err != nil {
 		return err
 	} else if !shouldSync {
@@ -95,7 +92,7 @@ func (c *oauthClientSecretController) sync(ctx context.Context, syncCtx factory.
 
 	statusHandler := status.NewStatusHandler(c.operatorClient)
 
-	clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
+	clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(deploymentsub.ConsoleOauthConfigName)
 	if err != nil && !apierrors.IsNotFound(err) {
 		return err
 	}
@@ -117,30 +114,10 @@ func (c *oauthClientSecretController) sync(ctx context.Context, syncCtx factory.
 			secretString = crypto.Random256BitsString()
 		}
 	case configv1.AuthenticationTypeOIDC:
-		_, clientConfig := authnsub.GetOIDCClientConfig(authConfig, api.TargetNamespace, api.OpenShiftConsoleName)
-		if clientConfig == nil {
-			// no config, flush the condition and return
-			statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "", nil))
-			return statusHandler.FlushAndReturn(nil)
-		}
-
-		if len(clientConfig.ClientSecret.Name) == 0 {
-			statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "MissingClientSecretConfig", fmt.Errorf("missing client secret name reference in config")))
-			return statusHandler.FlushAndReturn(nil)
-		}
-
-		conficClientSecret, err := c.configSecretsLister.Secrets(api.OpenShiftConfigNamespace).Get(clientConfig.ClientSecret.Name)
-		if err != nil {
-			statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "FailedClientSecretGet", err))
-			return statusHandler.FlushAndReturn(err)
-		}
-
-		secretString = secretsub.GetSecretString(conficClientSecret)
-		if len(secretString) == 0 {
-			statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "ClientSecretKeyMissing", fmt.Errorf("missing the 'clientSecret' key in the client secret secret %q", clientConfig.ClientSecret.Name)))
-			return statusHandler.FlushAndReturn(nil)
-		}
-
+		// OIDC authentication is handled by the oidcController, not this controller
+		klog.V(4).Infoln("OIDC authentication type - skipping integratedOAuthController")
+		statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "", nil))
+		return statusHandler.FlushAndReturn(nil)
 	default:
 		klog.V(2).Infof("unknown authentication type: %s", authConfig.Spec.Type)
 		statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "", nil))
@@ -152,13 +129,13 @@ func (c *oauthClientSecretController) sync(ctx context.Context, syncCtx factory.
 	return statusHandler.FlushAndReturn(err)
 }
 
-func (c *oauthClientSecretController) syncSecret(ctx context.Context, clientSecret string, recorder events.Recorder) error {
+func (c *integratedOAuthController) syncSecret(ctx context.Context, clientSecret string, recorder events.Recorder) error {
 	operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName)
 	if err != nil {
 		return err
 	}
 
-	secret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
+	secret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(deploymentsub.ConsoleOauthConfigName)
 	if apierrors.IsNotFound(err) || secretsub.GetSecretString(secret) != clientSecret {
 		_, _, err = resourceapply.ApplySecret(ctx, c.secretsClient, recorder, secretsub.DefaultSecret(operatorConfig, clientSecret))
 	}
@@ -168,7 +145,7 @@ func (c *oauthClientSecretController) syncSecret(ctx context.Context, clientSecr
 // handleStatus returns whether sync should happen and any error encountering
 // determining the operator's management state
 // TODO: extract this logic to where it can be used for all controllers
-func (c *oauthClientSecretController) handleManaged() (bool, error) {
+func (c *integratedOAuthController) handleManaged() (bool, error) {
 	operatorSpec, _, _, err := c.operatorClient.GetOperatorState()
 	if err != nil {
 		return false, fmt.Errorf("failed to retrieve operator config: %w", err)

pkg/console/controllers/oauthclients/oauthclients.go

@@ -34,6 +34,7 @@ import (
 	"github.com/openshift/console-operator/pkg/api"
 	"github.com/openshift/console-operator/pkg/console/controllers/util"
 	"github.com/openshift/console-operator/pkg/console/status"
+	deploymentsub "github.com/openshift/console-operator/pkg/console/subresource/deployment"
 	oauthsub "github.com/openshift/console-operator/pkg/console/subresource/oauthclient"
 	routesub "github.com/openshift/console-operator/pkg/console/subresource/route"
 	secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
@@ -163,7 +164,7 @@ func (c *oauthClientsController) sync(ctx context.Context, controllerContext fac
 		return statusHandler.FlushAndReturn(fmt.Errorf("timed out waiting for OAuthClients cache sync"))
 	}
 
-	clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
+	clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(deploymentsub.ConsoleOauthConfigName)
 	if err != nil {
 		return err
 	}

pkg/console/controllers/oidcsetup/oidcsetup.go → pkg/console/controllers/oidc/oidc.go

@@ -1,4 +1,4 @@
-package oidcsetup
+package oidc
 
 import (
 	"context"
@@ -7,6 +7,7 @@ import (
 
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "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"
@@ -32,32 +33,62 @@ import (
 
 	authnsub "github.com/openshift/console-operator/pkg/console/subresource/authentication"
 	deploymentsub "github.com/openshift/console-operator/pkg/console/subresource/deployment"
+	secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
 	utilsub "github.com/openshift/console-operator/pkg/console/subresource/util"
 )
 
-// oidcSetupController:
+// oidcController manages all OIDC authentication resources for the console.
 //
-//	writes:
-//	- authentication.config.openshift.io/cluster .status.oidcClients:
-//		- componentName=console
-//		- componentNamespace=openshift-console
-//		- currentOIDCClients
-//		- conditions:
-//			- Available
-//			- Progressing
-//			- Degraded
-//	- consoles.operator.openshift.io/cluster .status.conditions:
-//		- type=OIDCClientConfigProgressing
-//		- type=OIDCClientConfigDegraded
-//		- type=AuthStatusHandlerProgressing
-//		- type=AuthStatusHandlerDegraded
-type oidcSetupController struct {
+// Responsibilities:
+//
+//   - Fetches OIDC client secret from openshift-config namespace
+//
+//   - Syncs OIDC client secret to openshift-console/console-oauth-config
+//
+//   - Syncs CA configmaps from openshift-config to openshift-console
+//
+//   - Updates Authentication CR status with OIDC client information
+//
+//   - Only runs when authentication type is OIDC
+//
+//     writes:
+//
+//   - secrets.console-oauth-config -n openshift-console .Data['clientSecret']
+//
+//   - authentication.config.openshift.io/cluster .status.oidcClients:
+//
+//   - componentName=console
+//
+//   - componentNamespace=openshift-console
+//
+//   - currentOIDCClients
+//
+//   - conditions:
+//
+//   - Available
+//
+//   - Progressing
+//
+//   - Degraded
+//
+//   - consoles.operator.openshift.io/cluster .status.conditions:
+//
+//   - type=OIDCClientConfigProgressing
+//
+//   - type=OIDCClientConfigDegraded
+//
+//   - type=AuthStatusHandlerProgressing
+//
+//   - type=AuthStatusHandlerDegraded
+type oidcController struct {
 	operatorClient  v1helpers.OperatorClient
 	configMapClient corev1client.ConfigMapsGetter
+	secretsClient   corev1client.SecretsGetter
 
 	authnLister               configv1listers.AuthenticationLister
 	consoleOperatorLister     operatorv1listers.ConsoleLister
 	configConfigMapLister     corev1listers.ConfigMapLister
+	configSecretsLister       corev1listers.SecretLister
 	targetNSSecretsLister     corev1listers.SecretLister
 	targetNSConfigMapLister   corev1listers.ConfigMapLister
 	targetNSDeploymentsLister appsv1listers.DeploymentLister
@@ -67,26 +98,30 @@ type oidcSetupController struct {
 	authStatusHandler *status.AuthStatusHandler
 }
 
-func NewOIDCSetupController(
+func NewOIDCController(
 	operatorClient v1helpers.OperatorClient,
 	configMapClient corev1client.ConfigMapsGetter,
+	secretsClient corev1client.SecretsGetter,
 	authnInformer configv1informers.AuthenticationInformer,
 	authenticationClient configv1client.AuthenticationInterface,
 	consoleOperatorInformer operatorv1informers.ConsoleInformer,
 	configConfigMapInformer corev1informers.ConfigMapInformer,
+	configSecretInformer corev1informers.SecretInformer,
 	targetNSsecretsInformer corev1informers.SecretInformer,
 	targetNSConfigMapInformer corev1informers.ConfigMapInformer,
 	targetNSDeploymentsInformer appsv1informers.DeploymentInformer,
 	externalOIDCFeatureEnabled bool,
 	recorder events.Recorder,
 ) factory.Controller {
-	c := &oidcSetupController{
+	c := &oidcController{
 		operatorClient:  operatorClient,
 		configMapClient: configMapClient,
+		secretsClient:   secretsClient,
 
 		authnLister:               authnInformer.Lister(),
 		consoleOperatorLister:     consoleOperatorInformer.Lister(),
 		configConfigMapLister:     configConfigMapInformer.Lister(),
+		configSecretsLister:       configSecretInformer.Lister(),
 		targetNSSecretsLister:     targetNSsecretsInformer.Lister(),
 		targetNSDeploymentsLister: targetNSDeploymentsInformer.Lister(),
 		targetNSConfigMapLister:   targetNSConfigMapInformer.Lister(),
@@ -102,14 +137,15 @@ func NewOIDCSetupController(
 			authnInformer.Informer(),
 			configConfigMapInformer.Informer(),
 			consoleOperatorInformer.Informer(),
+			configSecretInformer.Informer(),
 			targetNSsecretsInformer.Informer(),
 			targetNSDeploymentsInformer.Informer(),
 			targetNSConfigMapInformer.Informer(),
 		).
-		ToController("OIDCSetupController", recorder.WithComponentSuffix("oidc-setup-controller"))
+		ToController("OIDCController", recorder.WithComponentSuffix("oidc-controller"))
 }
 
-func (c *oidcSetupController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
+func (c *oidcController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
 	statusHandler := status.NewStatusHandler(c.operatorClient)
 
 	if shouldSync, err := c.handleManaged(); err != nil {
@@ -182,7 +218,7 @@ func (c *oidcSetupController) sync(ctx context.Context, syncCtx factory.SyncCont
 	return statusHandler.FlushAndReturn(nil)
 }
 
-func (c *oidcSetupController) syncAuthTypeOIDC(ctx context.Context, authnConfig *configv1.Authentication, operatorConfig *operatorv1.Console, recorder events.Recorder) error {
+func (c *oidcController) syncAuthTypeOIDC(ctx context.Context, authnConfig *configv1.Authentication, operatorConfig *operatorv1.Console, recorder events.Recorder) error {
 	oidcProvider, clientConfig := authnsub.GetOIDCClientConfig(authnConfig, api.TargetNamespace, api.OpenShiftConsoleName)
 	if clientConfig == nil {
 		c.authStatusHandler.WithCurrentOIDCClient("")
@@ -200,7 +236,7 @@ func (c *oidcSetupController) syncAuthTypeOIDC(ctx context.Context, authnConfig
 		return nil
 	}
 
-	clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
+	clientSecret, err := c.configSecretsLister.Secrets(api.OpenShiftConfigNamespace).Get(clientConfig.ClientSecret.Name)
 	if err != nil {
 		c.authStatusHandler.Degraded("OIDCClientSecretGet", err.Error())
 		return err
@@ -224,6 +260,22 @@ func (c *oidcSetupController) syncAuthTypeOIDC(ctx context.Context, authnConfig
 		}
 	}
 
+	// Sync OIDC client secret to target namespace
+	// This secret is used by the console deployment to authenticate with the OIDC provider
+	clientSecretString := secretsub.GetSecretString(clientSecret)
+	if len(clientSecretString) == 0 {
+		return fmt.Errorf("missing the 'clientSecret' key in the client secret %q", clientConfig.ClientSecret.Name)
+	}
+
+	targetSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(deploymentsub.ConsoleOauthConfigName)
+	if apierrors.IsNotFound(err) || secretsub.GetSecretString(targetSecret) != clientSecretString {
+		_, _, err = resourceapply.ApplySecret(ctx, c.secretsClient, recorder,
+			secretsub.DefaultSecret(operatorConfig, clientSecretString))
+		if err != nil {
+			return fmt.Errorf("failed to sync OIDC client secret to target namespace: %w", err)
+		}
+	}
+
 	if valid, msg, err := c.checkClientConfigStatus(authnConfig, clientSecret); err != nil {
 		c.authStatusHandler.Degraded("DeploymentOIDCConfig", err.Error())
 		return err
@@ -241,7 +293,7 @@ func (c *oidcSetupController) syncAuthTypeOIDC(ctx context.Context, authnConfig
 // by looking at the deployment status. It checks whether the deployment is available and updated,
 // and also whether the resource versions for the oauth secret and server CA trust configmap match
 // the deployment.
-func (c *oidcSetupController) checkClientConfigStatus(authnConfig *configv1.Authentication, clientSecret *corev1.Secret) (bool, string, error) {
+func (c *oidcController) checkClientConfigStatus(authnConfig *configv1.Authentication, clientSecret *corev1.Secret) (bool, string, error) {
 	depl, err := c.targetNSDeploymentsLister.Deployments(api.OpenShiftConsoleNamespace).Get(api.OpenShiftConsoleDeploymentName)
 	if err != nil {
 		return false, "", err
@@ -278,7 +330,7 @@ func (c *oidcSetupController) checkClientConfigStatus(authnConfig *configv1.Auth
 // handleStatus returns whether sync should happen and any error encountering
 // determining the operator's management state
 // TODO: extract this logic to where it can be used for all controllers
-func (c *oidcSetupController) handleManaged() (bool, error) {
+func (c *oidcController) handleManaged() (bool, error) {
 	operatorSpec, _, _, err := c.operatorClient.GetOperatorState()
 	if err != nil {
 		return false, fmt.Errorf("failed to retrieve operator config: %w", err)

pkg/console/operator/sync_v400.go

@@ -117,6 +117,9 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact
 		if err != nil {
 			return statusHandler.FlushAndReturn(err)
 		}
+	default:
+		// Clear OIDC-related conditions when auth type is not OIDC
+		statusHandler.AddConditions(status.HandleProgressingOrDegraded("OIDCProviderTrustedAuthorityConfigGet", "", nil))
 	}
 
 	customLogosErr, customLogosErrReason := co.SyncCustomLogos(updatedOperatorConfig)