From cd455bf2e0a2dd29f76d1cb8637442027ab12c11 Mon Sep 17 00:00:00 2001 From: Samuel Padgett Date: Fri, 18 Jul 2025 13:20:25 -0400 Subject: [PATCH] [WIP] Gracefully handle config changes --- .vscode/settings.json | 3 +++ pkg/api/api.go | 1 + pkg/console/operator/sync_v400.go | 12 +++++----- .../consoleserver/config_builder.go | 7 ++++++ .../subresource/consoleserver/types.go | 1 + .../subresource/deployment/deployment.go | 24 +++++++++++++++---- .../subresource/deployment/deployment_test.go | 3 +-- 7 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..082b19437 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "makefile.configureOnOpen": false +} \ No newline at end of file diff --git a/pkg/api/api.go b/pkg/api/api.go index dd1e209df..888d79599 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -38,6 +38,7 @@ const ( RedirectContainerPortName = "custom-route-redirect" ServiceCAConfigMapName = "service-ca" SessionSecretName = "session-secret" + SessionStorageVolumeName = "session-storage" TargetNamespace = "openshift-console" TrustedCABundleKey = "ca-bundle.crt" TrustedCABundleMountDir = "/etc/pki/ca-trust/extracted/pem" diff --git a/pkg/console/operator/sync_v400.go b/pkg/console/operator/sync_v400.go index beddeaef4..496055701 100644 --- a/pkg/console/operator/sync_v400.go +++ b/pkg/console/operator/sync_v400.go @@ -95,10 +95,15 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact return statusHandler.FlushAndReturn(err) } + sessionSecret, err := co.syncSessionSecret(ctx, updatedOperatorConfig, controllerContext.Recorder()) + if err != nil { + return statusHandler.FlushAndReturn(err) + } + var ( targetNamespaceAuthServerCA *corev1.ConfigMap - sessionSecret *corev1.Secret ) + switch authnConfig.Spec.Type { case configv1.AuthenticationTypeOIDC: if len(authnConfig.Spec.OIDCProviders) > 0 { @@ -112,11 +117,6 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact } } } - - sessionSecret, err = co.syncSessionSecret(ctx, updatedOperatorConfig, controllerContext.Recorder()) - if err != nil { - return statusHandler.FlushAndReturn(err) - } } customLogosErr, customLogosErrReason := co.SyncCustomLogos(updatedOperatorConfig) diff --git a/pkg/console/subresource/consoleserver/config_builder.go b/pkg/console/subresource/consoleserver/config_builder.go index 408b5245a..37b85724d 100644 --- a/pkg/console/subresource/consoleserver/config_builder.go +++ b/pkg/console/subresource/consoleserver/config_builder.go @@ -63,6 +63,7 @@ type ConsoleServerCLIConfigBuilder struct { monitoring map[string]string customHostnameRedirectPort int inactivityTimeoutSeconds int + sessionDir string pluginsList map[string]string pluginsOrder []string i18nNamespaceList []string @@ -190,12 +191,17 @@ func (b *ConsoleServerCLIConfigBuilder) Capabilities(capabilities []operatorv1.C } func (b *ConsoleServerCLIConfigBuilder) AuthConfig(authnConfig *configv1.Authentication, apiServerURL string) *ConsoleServerCLIConfigBuilder { + b.sessionDir = "/var/sessions" + switch authnConfig.Spec.Type { // We don't disable auth since the internal OAuth server is not disabled even with auth type 'None'. case "", configv1.AuthenticationTypeIntegratedOAuth, configv1.AuthenticationTypeNone: b.authType = "openshift" b.oauthClientID = api.OAuthClientName b.CAFile = oauthServingCertFilePath + b.sessionAuthenticationFile = "/var/session-secret/sessionAuthenticationKey" + b.sessionEncryptionFile = "/var/session-secret/sessionEncryptionKey" + return b case configv1.AuthenticationTypeOIDC: @@ -419,6 +425,7 @@ func (b *ConsoleServerCLIConfigBuilder) session() Session { conf := Session{ CookieAuthenticationKeyFile: b.sessionAuthenticationFile, CookieEncryptionKeyFile: b.sessionEncryptionFile, + SessionDir: b.sessionDir, } return conf } diff --git a/pkg/console/subresource/consoleserver/types.go b/pkg/console/subresource/consoleserver/types.go index 78ea9ec0f..a89f3825c 100644 --- a/pkg/console/subresource/consoleserver/types.go +++ b/pkg/console/subresource/consoleserver/types.go @@ -100,6 +100,7 @@ type Auth struct { type Session struct { CookieEncryptionKeyFile string `yaml:"cookieEncryptionKeyFile,omitempty"` CookieAuthenticationKeyFile string `yaml:"cookieAuthenticationKeyFile,omitempty"` + SessionDir string `yaml:"sessionDir,omitempty"` // TODO: move InactivityTimeoutSeconds here } diff --git a/pkg/console/subresource/deployment/deployment.go b/pkg/console/subresource/deployment/deployment.go index 542df12a4..7d5bb721c 100644 --- a/pkg/console/subresource/deployment/deployment.go +++ b/pkg/console/subresource/deployment/deployment.go @@ -58,9 +58,10 @@ type volumeConfig struct { name string readOnly bool path string - // isSecret or isConfigMap are mutually exclusive + // isSecret, isConfigMap, and isEmptyDir are mutually exclusive isSecret bool isConfigMap bool + isEmptyDir bool mappedKeys map[string]string } @@ -87,7 +88,6 @@ func DefaultDeployment( withStrategy(deployment, infrastructureConfig) withConsoleAnnotations( deployment, - consoleConfigMap, serviceCAConfigMap, authnCATrustConfigMap, trustedCAConfigMap, @@ -194,7 +194,6 @@ func withStrategy(deployment *appsv1.Deployment, infrastructureConfig *configv1. // version changes. func withConsoleAnnotations( deployment *appsv1.Deployment, - consoleConfigMap *corev1.ConfigMap, serviceCAConfigMap *corev1.ConfigMap, authServerCAConfigMap *corev1.ConfigMap, trustedCAConfigMap *corev1.ConfigMap, @@ -203,8 +202,9 @@ func withConsoleAnnotations( proxyConfig *configv1.Proxy, infrastructureConfig *configv1.Infrastructure, ) { + // Avoid rolling out when the console-config configmap is updated. + // Console now watches the configmap for changes without needing to redeploy. deployment.ObjectMeta.Annotations = map[string]string{ - configMapResourceVersionAnnotation: consoleConfigMap.GetResourceVersion(), serviceCAConfigMapResourceVersionAnnotation: serviceCAConfigMap.GetResourceVersion(), trustedCAConfigMapResourceVersionAnnotation: trustedCAConfigMap.GetResourceVersion(), proxyConfigResourceVersionAnnotation: proxyConfig.GetResourceVersion(), @@ -304,6 +304,16 @@ func withConsoleVolumes( }, } } + if item.isEmptyDir { + vols[i] = corev1.Volume{ + Name: item.name, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + }, + } + } } deployment.Spec.Template.Spec.Volumes = vols } @@ -519,6 +529,12 @@ func defaultVolumeConfig() []volumeConfig { path: "/var/service-ca", isConfigMap: true, }, + { + name: api.SessionStorageVolumeName, + readOnly: false, + path: "/var/sessions", + isEmptyDir: true, + }, } } diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index fd0caedb6..b513470fd 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -632,7 +632,6 @@ func TestWithConsoleAnnotations(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ workloadManagementAnnotation: workloadManagementAnnotationValue, - configMapResourceVersionAnnotation: consoleConfigMap.GetResourceVersion(), serviceCAConfigMapResourceVersionAnnotation: serviceCAConfigMap.GetResourceVersion(), authnCATrustConfigMapResourceVersionAnnotation: oauthServingCertConfigMap.GetResourceVersion(), trustedCAConfigMapResourceVersionAnnotation: trustedCAConfigMap.GetResourceVersion(), @@ -649,7 +648,7 @@ func TestWithConsoleAnnotations(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - withConsoleAnnotations(tt.args.deployment, tt.args.consoleConfigMap, tt.args.serviceCAConfigMap, tt.args.authServerCAConfigMap, tt.args.trustedCAConfigMap, tt.args.oAuthClientSecret, tt.args.sessionSecret, tt.args.proxyConfig, tt.args.infrastructureConfig) + withConsoleAnnotations(tt.args.deployment, tt.args.serviceCAConfigMap, tt.args.authServerCAConfigMap, tt.args.trustedCAConfigMap, tt.args.oAuthClientSecret, tt.args.sessionSecret, tt.args.proxyConfig, tt.args.infrastructureConfig) if diff := deep.Equal(tt.args.deployment, tt.want); diff != nil { t.Error(diff) }