diff --git a/pkg/console/controllers/oauthclientsecret/oauthclientsecret.go b/pkg/console/controllers/integratedoauth/integratedoauth.go similarity index 66% rename from pkg/console/controllers/oauthclientsecret/oauthclientsecret.go rename to pkg/console/controllers/integratedoauth/integratedoauth.go index 1791cca7c..524466e0a 100644 --- a/pkg/console/controllers/oauthclientsecret/oauthclientsecret.go +++ b/pkg/console/controllers/integratedoauth/integratedoauth.go @@ -1,4 +1,4 @@ -package oauthclientsecret +package integratedoauth import ( "context" @@ -24,19 +24,20 @@ 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: @@ -44,32 +45,29 @@ import ( // - 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) diff --git a/pkg/console/controllers/oauthclients/oauthclients.go b/pkg/console/controllers/oauthclients/oauthclients.go index 0e716c56c..2c30752c1 100644 --- a/pkg/console/controllers/oauthclients/oauthclients.go +++ b/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 } diff --git a/pkg/console/controllers/oidcsetup/oidcsetup.go b/pkg/console/controllers/oidc/oidc.go similarity index 77% rename from pkg/console/controllers/oidcsetup/oidcsetup.go rename to pkg/console/controllers/oidc/oidc.go index ef0b506a6..deeb9046c 100644 --- a/pkg/console/controllers/oidcsetup/oidcsetup.go +++ b/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) diff --git a/pkg/console/operator/sync_v400.go b/pkg/console/operator/sync_v400.go index 048b19aa0..3a2cf9fd4 100644 --- a/pkg/console/operator/sync_v400.go +++ b/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) diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index 47bb46b54..5e0a8c14e 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -30,9 +30,9 @@ import ( "github.com/openshift/console-operator/pkg/console/controllers/clioidcclientstatus" "github.com/openshift/console-operator/pkg/console/controllers/downloadsdeployment" "github.com/openshift/console-operator/pkg/console/controllers/healthcheck" + "github.com/openshift/console-operator/pkg/console/controllers/integratedoauth" "github.com/openshift/console-operator/pkg/console/controllers/oauthclients" - "github.com/openshift/console-operator/pkg/console/controllers/oauthclientsecret" - "github.com/openshift/console-operator/pkg/console/controllers/oidcsetup" + "github.com/openshift/console-operator/pkg/console/controllers/oidc" pdb "github.com/openshift/console-operator/pkg/console/controllers/poddisruptionbudget" "github.com/openshift/console-operator/pkg/console/controllers/route" "github.com/openshift/console-operator/pkg/console/controllers/service" @@ -277,24 +277,25 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle recorder, ) - oauthClientSecretController := oauthclientsecret.NewOAuthClientSecretController( + integratedOAuthController := integratedoauth.NewIntegratedOAuthController( operatorClient, kubeClient.CoreV1(), configInformers.Config().V1().Authentications(), operatorConfigInformers.Operator().V1().Consoles(), - kubeInformersConfigNamespaced.Core().V1().Secrets(), kubeInformersNamespaced.Core().V1().Secrets(), recorder, ) externalOIDCEnabled := featureGates.Enabled("ExternalOIDC") - oidcSetupController := oidcsetup.NewOIDCSetupController( + oidcController := oidc.NewOIDCController( operatorClient, kubeClient.CoreV1(), + kubeClient.CoreV1(), configInformers.Config().V1().Authentications(), configClient.ConfigV1().Authentications(), operatorConfigInformers.Operator().V1().Consoles(), kubeInformersConfigNamespaced.Core().V1().ConfigMaps(), + kubeInformersConfigNamespaced.Core().V1().Secrets(), kubeInformersNamespaced.Core().V1().Secrets(), kubeInformersNamespaced.Core().V1().ConfigMaps(), kubeInformersNamespaced.Apps().V1().Deployments(), @@ -598,8 +599,8 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle consolePDBController, downloadsPDBController, oauthClientController, - oauthClientSecretController, - oidcSetupController, + integratedOAuthController, + oidcController, cliOIDCClientStatusController, upgradeNotificationController, staleConditionsController,