Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
34 changes: 33 additions & 1 deletion pkg/console/operator/sync_v400.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,12 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact
return statusHandler.FlushAndReturn(techPreviewErr)
}

olmLifecycleMetadataEnabled, olmLifecycleMetadataErrReason, olmLifecycleMetadataErr := co.SyncOLMLifecycleMetadata()
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OLMLifecycleMetadataSync", olmLifecycleMetadataErrReason, olmLifecycleMetadataErr))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor terminology check: if status string visible to users, use full word "Synchronization"?

"OLMLifecycleMetadataSynchronization"

not a hill i'd 💀 on though!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Keeping "Sync" — every existing condition name in this file uses that abbreviation: CustomLogoSync, TechPreviewSync, ConfigMapSync, ServiceCASync, TrustedCASync, DeploymentSync.

if olmLifecycleMetadataErr != nil {
return statusHandler.FlushAndReturn(olmLifecycleMetadataErr)
}

cm, cmErrReason, cmErr := co.SyncConfigMap(
ctx,
set.Operator,
Expand All @@ -141,6 +147,7 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact
controllerContext.Recorder(),
consoleURL.Hostname(),
techPreviewEnabled,
olmLifecycleMetadataEnabled,
)
statusHandler.AddConditions(status.HandleProgressingOrDegraded("ConfigMapSync", cmErrReason, cmErr))
if cmErr != nil {
Expand Down Expand Up @@ -352,6 +359,7 @@ func (co *consoleOperator) SyncConfigMap(
recorder events.Recorder,
consoleHost string,
techPreviewEnabled bool,
olmLifecycleMetadataEnabled bool,
) (consoleConfigMap *corev1.ConfigMap, reason string, err error) {

managedConfig, mcErr := co.managedNSConfigMapLister.ConfigMaps(api.OpenShiftConfigManagedNamespace).Get(api.OpenShiftConsoleConfigMapName)
Expand Down Expand Up @@ -426,6 +434,7 @@ func (co *consoleOperator) SyncConfigMap(
telemetryConfig,
consoleHost,
techPreviewEnabled,
olmLifecycleMetadataEnabled,
)
if err != nil {
return nil, "FailedConsoleConfigBuilder", err
Expand Down Expand Up @@ -600,7 +609,7 @@ func (co *consoleOperator) SyncTrustedCAConfigMap(ctx context.Context, operatorC
func (co *consoleOperator) SyncTechPreview() (techPreviewEnabled bool, reason string, err error) {
featureGate, err := co.featureGateLister.Get(api.ConfigResourceName)
if err != nil {
klog.V(4).Infof("failed to get FeatureGate resource: %v.", err)
klog.V(4).Infof("failed to get FeatureGate resource: %v", err)
return false, "FailedGet", err
}

Expand All @@ -612,6 +621,29 @@ func (co *consoleOperator) SyncTechPreview() (techPreviewEnabled bool, reason st
return techPreviewEnabled, "", nil
}

// Replace with features.FeatureGateOLMLifecycleAndCompatibility once openshift/api is bumped.
const olmLifecycleAndCompatibilityFeatureGate configv1.FeatureGateName = "OLMLifecycleAndCompatibility"

// SyncOLMLifecycleMetadata determines if OLM lifecycle metadata features should be enabled
// by checking if the OLMLifecycleAndCompatibility feature gate is enabled in the cluster.
func (co *consoleOperator) SyncOLMLifecycleMetadata() (olmLifecycleMetadataEnabled bool, reason string, err error) {
featureGate, err := co.featureGateLister.Get(api.ConfigResourceName)
if err != nil {
klog.V(4).Infof("failed to get FeatureGate resource: %v", err)
return false, "FailedGet", err

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

633: ambiguous for anyone troubleshooting cluster statuses/events. strongly urge being more specific about what resource failed to load. -->

return false, "FailedGetFeatureGate", err

for ex.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

"FailedGet" is the established convention across this file — it's used in 9 places (lines 108, 184, 190, 537, 581, 613, 633, 695, 776) including the existing SyncTechPreview method which does the identical FeatureGate lookup and returns "FailedGet" on the same error path. The specificity comes from how the caller consumes the reason (e.g. HandleProgressingOrDegraded("OAuthClientSecretGet", "FailedGet", err) — the first arg provides the resource context). Changing just this instance would be inconsistent.

}

for _, versionedGates := range featureGate.Status.FeatureGates {
for _, gate := range versionedGates.Enabled {
if gate.Name == olmLifecycleAndCompatibilityFeatureGate {
klog.V(4).Infoln("OLM lifecycle metadata features are enabled based on the OLMLifecycleAndCompatibility feature gate")
return true, "", nil
}
}
}
return false, "", nil
}

func (co *consoleOperator) SyncCustomLogos(operatorConfig *operatorv1.Console) (error, string) {
if operatorConfig.Spec.Customization.CustomLogoFile.Name != "" || operatorConfig.Spec.Customization.CustomLogoFile.Key != "" {
return co.SyncCustomLogoConfigMap(operatorConfig)
Expand Down
7 changes: 5 additions & 2 deletions pkg/console/subresource/configmap/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func DefaultConfigMap(
telemeterConfig map[string]string,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GLOBAL

compound modifiers preceding nouns should be hyphenated (ex., "user-defined").

  1. from user/operator perspective, these messages say that something failed, but don't answer "what do I do next?" strongly urge adding at least brief hint about impact or next step.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hyphenation is fixed.

On the remediation hints — these are operator-internal klog.Errorf messages, not user-facing. The existing pattern across the codebase (sync_v400.go, operator.go, storageversionmigration/controller.go, etc.) is consistently "failed to X: %v" without next-step guidance. The error detail comes through in %v, and the operator's reconciliation loop retries automatically. Adding hints here would be inconsistent with the rest of the codebase and these specific errors already bubble up as return values to the caller. Happy to discuss further if you feel strongly about it though.

consoleHost string,
techPreviewEnabled bool,
olmLifecycleMetadataEnabled bool,
) (consoleConfigMap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) {

apiServerURL := infrastructuresub.GetAPIServerURL(infrastructureConfig)
Expand All @@ -66,9 +67,10 @@ func DefaultConfigMap(
NodeOperatingSystems(nodeOperatingSystems).
CopiedCSVsDisabled(copiedCSVsDisabled).
TechPreviewEnabled(techPreviewEnabled).
OLMLifecycleMetadataEnabled(olmLifecycleMetadataEnabled).
ConfigYAML()
if err != nil {
klog.Errorf("failed to generate default console-config config: %v", err)
klog.Errorf("failed to generate default console-config: %v", err)
return nil, false, err
}

Expand Down Expand Up @@ -106,9 +108,10 @@ func DefaultConfigMap(
AuthConfig(authConfig, apiServerURL).
Capabilities(operatorConfig.Spec.Customization.Capabilities).
TechPreviewEnabled(techPreviewEnabled).
OLMLifecycleMetadataEnabled(olmLifecycleMetadataEnabled).
ConfigYAML()
if err != nil {
klog.Errorf("failed to generate user defined console-config config: %v", err)
klog.Errorf("failed to generate user-defined console-config: %v", err)
return nil, false, err
}

Expand Down
1 change: 1 addition & 0 deletions pkg/console/subresource/configmap/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,7 @@ providers: {}
tt.args.telemetryConfig,
tt.args.rt.Spec.Host,
false, // techPreviewEnabled - default to false for tests
false, // olmLifecycleMetadataEnabled - default to false for tests
)

// marshall the exampleYaml to map[string]interface{} so we can use it in diff below
Expand Down
142 changes: 101 additions & 41 deletions pkg/console/subresource/configmap/tech_preview_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,80 +42,140 @@ func TestTechPreviewEnabled(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Create minimal test configuration
operatorConfig := &operatorv1.Console{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: operatorv1.ConsoleSpec{},
}
cm, _, err := DefaultConfigMap(
minimalOperatorConfig(),
minimalConsoleConfig(),
minimalAuthConfig(),
&corev1.ConfigMap{},
&corev1.ConfigMap{},
minimalInfrastructureConfig(),
minimalRoute(),
0, // inactivityTimeoutSeconds
[]*consolev1.ConsolePlugin{}, // availablePlugins
[]string{"amd64"}, // nodeArchitectures
[]string{"linux"}, // nodeOperatingSystems
false, // copiedCSVsDisabled
map[string]string{}, // telemetryConfig
"console.test.cluster", // consoleHost
tt.args.techPreviewEnabled,
false, // olmLifecycleMetadataEnabled
)

consoleConfig := &configv1.Console{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
if err != nil {
t.Errorf("DefaultConfigMap() error = %v.", err)
return
}

authConfig := &configv1.Authentication{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
var config consoleserver.Config
err = yaml.Unmarshal([]byte(cm.Data["console-config.yaml"]), &config)
if err != nil {
t.Errorf("Failed to unmarshal config: %v.", err)
return
}

infrastructureConfig := &configv1.Infrastructure{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Status: configv1.InfrastructureStatus{
APIServerURL: "https://api.test.cluster:6443",
},
if config.ClusterInfo.TechPreviewEnabled != tt.want {
t.Errorf("TechPreviewEnabled: got %t, want %t (case %q, techPreviewEnabled input=%t).", config.ClusterInfo.TechPreviewEnabled, tt.want, tt.name, tt.args.techPreviewEnabled)
}
})
}
}

route := &routev1.Route{
ObjectMeta: metav1.ObjectMeta{
Name: "console",
},
Spec: routev1.RouteSpec{
Host: "console.test.cluster",
},
}
func TestOLMLifecycleMetadataEnabled(t *testing.T) {
type args struct {
olmLifecycleMetadataEnabled bool
}
tests := []struct {
name string
args args
want bool
}{
{
name: "OLM Lifecycle Metadata enabled",
args: args{
olmLifecycleMetadataEnabled: true,
},
want: true,
},
{
name: "OLM Lifecycle Metadata disabled",
args: args{
olmLifecycleMetadataEnabled: false,
},
want: false,
},
}

// Generate configmap with tech preview setting
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cm, _, err := DefaultConfigMap(
operatorConfig,
consoleConfig,
authConfig,
minimalOperatorConfig(),
minimalConsoleConfig(),
minimalAuthConfig(),
&corev1.ConfigMap{},
&corev1.ConfigMap{},
infrastructureConfig,
route,
minimalInfrastructureConfig(),
minimalRoute(),
0, // inactivityTimeoutSeconds
[]*consolev1.ConsolePlugin{}, // availablePlugins
[]string{"amd64"}, // nodeArchitectures
[]string{"linux"}, // nodeOperatingSystems
false, // copiedCSVsDisabled
map[string]string{}, // telemetryConfig
"console.test.cluster", // consoleHost
tt.args.techPreviewEnabled,
false, // techPreviewEnabled
tt.args.olmLifecycleMetadataEnabled,
)

if err != nil {
t.Errorf("DefaultConfigMap() error = %v.", err)
return
}

// Parse the generated config
var config consoleserver.Config
err = yaml.Unmarshal([]byte(cm.Data["console-config.yaml"]), &config)
if err != nil {
t.Errorf("Failed to unmarshal config: %v.", err)
return
}

// Verify tech preview setting
if config.ClusterInfo.TechPreviewEnabled != tt.want {
t.Errorf("TechPreviewEnabled: got %t, want %t (case %q, techPreviewEnabled input=%t).", config.ClusterInfo.TechPreviewEnabled, tt.want, tt.name, tt.args.techPreviewEnabled)
if config.ClusterInfo.OLMLifecycleMetadataEnabled != tt.want {
t.Errorf("OLMLifecycleMetadataEnabled: got %t, want %t (case %q, olmLifecycleMetadataEnabled input=%t).", config.ClusterInfo.OLMLifecycleMetadataEnabled, tt.want, tt.name, tt.args.olmLifecycleMetadataEnabled)
}
})
}
}

func minimalOperatorConfig() *operatorv1.Console {
return &operatorv1.Console{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Spec: operatorv1.ConsoleSpec{},
}
}

func minimalConsoleConfig() *configv1.Console {
return &configv1.Console{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
}
}

func minimalAuthConfig() *configv1.Authentication {
return &configv1.Authentication{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
}
}

func minimalInfrastructureConfig() *configv1.Infrastructure {
return &configv1.Infrastructure{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: configv1.InfrastructureStatus{
APIServerURL: "https://api.test.cluster:6443",
},
}
}

func minimalRoute() *routev1.Route {
return &routev1.Route{
ObjectMeta: metav1.ObjectMeta{Name: "console"},
Spec: routev1.RouteSpec{Host: "console.test.cluster"},
}
}
81 changes: 44 additions & 37 deletions pkg/console/subresource/consoleserver/config_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,43 +46,44 @@ var SupportedLightspeedArchitectures = []string{"amd64"}
//
// b.Host().Brand("").Config()
type ConsoleServerCLIConfigBuilder struct {
host string
logoutRedirectURL string
brand operatorv1.Brand
docURL string
apiServerURL string
controlPlaneToplogy configv1.TopologyMode
statusPageID string
customProductName string
devCatalogCustomization operatorv1.DeveloperConsoleCatalogCustomization
projectAccess operatorv1.ProjectAccess
quickStarts operatorv1.QuickStarts
addPage operatorv1.AddPage
perspectives []operatorv1.Perspective
CAFile string
monitoring map[string]string
customHostnameRedirectPort int
inactivityTimeoutSeconds int
pluginsList map[string]string
pluginsOrder []string
i18nNamespaceList []string
proxyServices []ProxyService
telemetry map[string]string
releaseVersion string
nodeArchitectures []string
nodeOperatingSystems []string
copiedCSVsDisabled bool
oauthClientID string
oidcExtraScopes []string
oidcIssuerURL string
oidcOCLoginCommand string
authType string
sessionEncryptionFile string
sessionAuthenticationFile string
capabilities []operatorv1.Capability
contentSecurityPolicyList map[v1.DirectiveType][]string
logos []operatorv1.Logo
techPreviewEnabled bool
host string
logoutRedirectURL string
brand operatorv1.Brand
docURL string
apiServerURL string
controlPlaneToplogy configv1.TopologyMode
statusPageID string
customProductName string
devCatalogCustomization operatorv1.DeveloperConsoleCatalogCustomization
projectAccess operatorv1.ProjectAccess
quickStarts operatorv1.QuickStarts
addPage operatorv1.AddPage
perspectives []operatorv1.Perspective
CAFile string
monitoring map[string]string
customHostnameRedirectPort int
inactivityTimeoutSeconds int
pluginsList map[string]string
pluginsOrder []string
i18nNamespaceList []string
proxyServices []ProxyService
telemetry map[string]string
releaseVersion string
nodeArchitectures []string
nodeOperatingSystems []string
copiedCSVsDisabled bool
oauthClientID string
oidcExtraScopes []string
oidcIssuerURL string
oidcOCLoginCommand string
authType string
sessionEncryptionFile string
sessionAuthenticationFile string
capabilities []operatorv1.Capability
contentSecurityPolicyList map[v1.DirectiveType][]string
logos []operatorv1.Logo
techPreviewEnabled bool
olmLifecycleMetadataEnabled bool
}

func (b *ConsoleServerCLIConfigBuilder) Host(host string) *ConsoleServerCLIConfigBuilder {
Expand Down Expand Up @@ -311,6 +312,11 @@ func (b *ConsoleServerCLIConfigBuilder) TechPreviewEnabled(techPreviewEnabled bo
return b
}

func (b *ConsoleServerCLIConfigBuilder) OLMLifecycleMetadataEnabled(olmLifecycleMetadataEnabled bool) *ConsoleServerCLIConfigBuilder {
b.olmLifecycleMetadataEnabled = olmLifecycleMetadataEnabled
return b
}

func (b *ConsoleServerCLIConfigBuilder) Config() Config {
return Config{
Kind: "ConsoleConfig",
Expand Down Expand Up @@ -381,6 +387,7 @@ func (b *ConsoleServerCLIConfigBuilder) clusterInfo() ClusterInfo {
}
conf.CopiedCSVsDisabled = b.copiedCSVsDisabled
conf.TechPreviewEnabled = b.techPreviewEnabled
conf.OLMLifecycleMetadataEnabled = b.olmLifecycleMetadataEnabled
return conf
}

Expand Down
Loading