Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/console/operator/sync_v400.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ 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)
}
sessionSecret, err = co.syncSessionSecret(ctx, updatedOperatorConfig, controllerContext.Recorder())
if err != nil {
return statusHandler.FlushAndReturn(err)
Comment on lines +124 to +126

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Wrap the session Secret synchronization error with context.

Line 126 returns the raw error. Add the failed operation name and preserve the cause with %w. This improves diagnosis because this operation now runs for every authentication type.

Proposed fix
-		return statusHandler.FlushAndReturn(err)
+		return statusHandler.FlushAndReturn(fmt.Errorf("sync session Secret: %w", err))

As per coding guidelines, “When returning errors in Go, wrap them with %w and include meaningful context instead of returning the raw error or using %v.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sessionSecret, err = co.syncSessionSecret(ctx, updatedOperatorConfig, controllerContext.Recorder())
if err != nil {
return statusHandler.FlushAndReturn(err)
sessionSecret, err = co.syncSessionSecret(ctx, updatedOperatorConfig, controllerContext.Recorder())
if err != nil {
return statusHandler.FlushAndReturn(fmt.Errorf("sync session Secret: %w", err))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/console/operator/sync_v400.go` around lines 124 - 126, Update the error
return immediately after syncSessionSecret in the operator synchronization flow
to wrap the error with meaningful session Secret synchronization context while
preserving the original cause via %w. Keep the existing
statusHandler.FlushAndReturn handling unchanged.

Source: Coding guidelines

}

customLogosErr, customLogosErrReason := co.SyncCustomLogos(updatedOperatorConfig)
Expand Down
88 changes: 66 additions & 22 deletions pkg/console/subresource/configmap/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ clusterInfo:
masterPublicURL: ` + mockAPIServer + `
controlPlaneTopology: HighlyAvailable
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -189,7 +191,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand All @@ -215,7 +219,9 @@ providers: {}
managedConfig: &corev1.ConfigMap{
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -271,7 +277,9 @@ clusterInfo:
nodeArchitectures:
- amd64
- arm64
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -299,7 +307,9 @@ telemetry:
managedConfig: &corev1.ConfigMap{
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -352,7 +362,9 @@ clusterInfo:
nodeOperatingSystems:
- foo
- bar
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -387,7 +399,9 @@ providers: {}
managedConfig: &corev1.ConfigMap{
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -436,7 +450,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + string(operatorv1.BrandDedicatedLegacy) + `
documentationBaseURL: ` + mockOperatorDocURL + `
Expand Down Expand Up @@ -477,7 +493,9 @@ providers: {}
managedConfig: &corev1.ConfigMap{
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -526,7 +544,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + string(operatorv1.BrandDedicatedLegacy) + `
documentationBaseURL: ` + mockOperatorDocURL + `
Expand Down Expand Up @@ -630,7 +650,9 @@ providers: {}
managedConfig: &corev1.ConfigMap{
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -679,7 +701,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + string(operatorv1.BrandDedicatedLegacy) + `
documentationBaseURL: ` + mockOperatorDocURL + `
Expand Down Expand Up @@ -749,7 +773,9 @@ providers: {}
managedConfig: &corev1.ConfigMap{
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down Expand Up @@ -798,7 +824,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + string(operatorv1.BrandDedicatedLegacy) + `
documentationBaseURL: ` + mockOperatorDocURL + `
Expand Down Expand Up @@ -871,7 +899,9 @@ clusterInfo:
consoleBaseAddress: https://` + customHostname + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -939,7 +969,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -1010,7 +1042,9 @@ clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -1122,7 +1156,9 @@ clusterInfo:
masterPublicURL: ` + mockAPIServer + `
controlPlaneTopology: External
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -1192,7 +1228,9 @@ clusterInfo:
controlPlaneTopology: External
releaseVersion: ` + testReleaseVersion + `
copiedCSVsDisabled: true
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -1266,7 +1304,9 @@ clusterInfo:
masterPublicURL: ` + mockAPIServer + `
controlPlaneTopology: HighlyAvailable
releaseVersion: ` + testReleaseVersion + `
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: ` + DEFAULT_BRAND + `
documentationBaseURL: ` + DEFAULT_DOC_URL + `
Expand Down Expand Up @@ -1503,7 +1543,9 @@ func Test_extractYAML(t *testing.T) {
},
Data: map[string]string{configKey: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand All @@ -1514,7 +1556,9 @@ customization:
},
want: `kind: ConsoleConfig
apiVersion: console.openshift.io/v1
session: {}
session:
cookieEncryptionKeyFile: /var/session-secret/sessionEncryptionKey
cookieAuthenticationKeyFile: /var/session-secret/sessionAuthenticationKey
customization:
branding: online
documentationBaseURL: https://docs.okd.io/4.4/
Expand Down
30 changes: 22 additions & 8 deletions pkg/console/subresource/consoleserver/config_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ const (
clientSecretFilePath = "/var/oauth-config/clientSecret"
oauthServingCertFilePath = "/var/oauth-serving-cert/ca-bundle.crt"
// serving info
certFilePath = "/var/serving-cert/tls.crt"
keyFilePath = "/var/serving-cert/tls.key"
certFilePath = "/var/serving-cert/tls.crt"
keyFilePath = "/var/serving-cert/tls.key"
sessionAuthKeyFilePath = "/var/session-secret/sessionAuthenticationKey"
sessionEncKeyFilePath = "/var/session-secret/sessionEncryptionKey"
)

// SupportedLightspeedArchitectures defines the list of architectures that support Lightspeed.
Expand Down Expand Up @@ -198,6 +200,8 @@ func (b *ConsoleServerCLIConfigBuilder) AuthConfig(authnConfig *configv1.Authent
b.authType = "openshift"
b.oauthClientID = api.OAuthClientName
b.CAFile = oauthServingCertFilePath
b.sessionAuthenticationFile = sessionAuthKeyFilePath
b.sessionEncryptionFile = sessionEncKeyFilePath
return b

case configv1.AuthenticationTypeOIDC:
Expand All @@ -217,8 +221,8 @@ func (b *ConsoleServerCLIConfigBuilder) AuthConfig(authnConfig *configv1.Authent
b.oauthClientID = oidcConfig.ClientID
b.oidcExtraScopes = oidcConfig.ExtraScopes
b.oidcOCLoginCommand = authconfigsub.GetOIDCOCLoginCommand(authnConfig, apiServerURL)
b.sessionAuthenticationFile = "/var/session-secret/sessionAuthenticationKey"
b.sessionEncryptionFile = "/var/session-secret/sessionEncryptionKey"
b.sessionAuthenticationFile = sessionAuthKeyFilePath
b.sessionEncryptionFile = sessionEncKeyFilePath

if len(oidcProvider.Issuer.CertificateAuthority.Name) > 0 {
b.CAFile = path.Join(api.AuthServerCAMountDir, api.AuthServerCAFileName)
Expand Down Expand Up @@ -452,11 +456,21 @@ func (b *ConsoleServerCLIConfigBuilder) auth() Auth {
}

func (b *ConsoleServerCLIConfigBuilder) session() Session {
conf := Session{
CookieAuthenticationKeyFile: b.sessionAuthenticationFile,
CookieEncryptionKeyFile: b.sessionEncryptionFile,
if b.authType == "disabled" {
return Session{}
}
authFile := b.sessionAuthenticationFile
encFile := b.sessionEncryptionFile
if authFile == "" {
authFile = sessionAuthKeyFilePath
}
if encFile == "" {
encFile = sessionEncKeyFilePath
}
return Session{
CookieAuthenticationKeyFile: authFile,
CookieEncryptionKeyFile: encFile,
}
return conf
}

func (b *ConsoleServerCLIConfigBuilder) customization() Customization {
Expand Down
Loading