Skip to content

OCPBUGS-54780: Fix issue where console operator orphans custom logo configmaps in openshift-console namespace #978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 5, 2025
Merged

OCPBUGS-54780: Fix issue where console operator orphans custom logo configmaps in openshift-console namespace #978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab68bff
OCPBUGS-54780: Fix issue where console operator orphans custom logo c…
TheRealJon Apr 10, 2025
920e476
improve debug logs for custom logo sync
TheRealJon May 2, 2025
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
5 changes: 3 additions & 2 deletions pkg/console/operator/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@ type trackables struct {
// used to keep track of OLM capability
isOLMDisabled bool
// track organization ID and mail
organizationID string
accountMail string
organizationID string
accountMail string
customLogoConfigMaps []string
}

func NewConsoleOperator(
Expand Down
124 changes: 88 additions & 36 deletions pkg/console/operator/sync_v400.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/url"
"os"
"slices"
"strings"

// kube
Expand Down Expand Up @@ -559,30 +560,39 @@ func (co *consoleOperator) SyncCustomLogos(operatorConfig *operatorv1.Console) (
aggregatedError error
err error
reason string
newSyncedLogos []string
)
for _, logo := range operatorConfig.Spec.Customization.Logos {
for _, theme := range logo.Themes {
logoToSync := theme.Source.ConfigMap
err, reason = co.UpdateCustomLogoSyncSource(logoToSync)
if err != nil {
if err, reason = co.ValidateCustomLogo(logoToSync); err != nil {
if aggregatedError == nil {
aggregatedError = fmt.Errorf("One or more errors were encountered while syncing custom logos:\n - %v, %s", logoToSync, err.Error())
aggregatedError = fmt.Errorf("error syncing custom logos: - Invalid config: %v, %s", logoToSync, err.Error())
} else {
aggregatedError = fmt.Errorf("%s\n - %v, %s", aggregatedError.Error(), logoToSync, err.Error())
aggregatedError = fmt.Errorf("%s - %v, %s", aggregatedError.Error(), logoToSync, err.Error())
}
} else {
newSyncedLogos = append(newSyncedLogos, logoToSync.Name)
}
}
}
if aggregatedError != nil {
return aggregatedError, reason
}
return nil, ""
slices.Sort(newSyncedLogos)
return co.UpdateCustomLogoSyncSources(newSyncedLogos)
}

// TODO remove deprecated CustomLogoFile API
func (co *consoleOperator) SyncCustomLogoConfigMap(operatorConfig *operatorv1.Console) (error, string) {
var customLogoRef = operatorv1.ConfigMapFileReference(operatorConfig.Spec.Customization.CustomLogoFile)
return co.UpdateCustomLogoSyncSource(&customLogoRef)
klog.V(4).Infof("[SyncCustomLogoConfigMap] syncing customLogoFile, Name: %s, Key: %s", customLogoRef.Name, customLogoRef.Key)
err, reason := co.ValidateCustomLogo(&customLogoRef)
if err != nil {
klog.V(4).Infof("[SyncCustomLogoConfigMap] failed to sync customLogoFile, %v", err)
return err, reason
}
return co.UpdateCustomLogoSyncSources([]string{customLogoRef.Name})
}

func (co *consoleOperator) ValidateOAuthServingCertConfigMap(ctx context.Context) (oauthServingCert *corev1.ConfigMap, reason string, err error) {
Expand All @@ -600,35 +610,54 @@ func (co *consoleOperator) ValidateOAuthServingCertConfigMap(ctx context.Context
}

// on each pass of the operator sync loop, we need to check the
// operator config for a custom logo. If this has been set, then
// we notify the resourceSyncer that it needs to start watching this
// configmap in its own sync loop. Note that the resourceSyncer's actual
// operator config for custom logos. If this has been set, then
// we notify the resourceSyncer that it needs to start watching the associated
// configmaps in its own sync loop. Note that the resourceSyncer's actual
// sync loop will run later. Our operator is waiting to receive
// the copied configmap into the console namespace for a future
// the copied configmaps into the console namespace for a future
// sync loop to mount into the console deployment.
func (c *consoleOperator) UpdateCustomLogoSyncSource(cmRef *operatorv1.ConfigMapFileReference) (error, string) {
// validate first, to avoid a broken volume mount & a crashlooping console
err, reason := c.ValidateCustomLogo(cmRef)
if err != nil {
return err, reason
func (co *consoleOperator) UpdateCustomLogoSyncSources(configMapNames []string) (error, string) {
klog.V(4).Info("[UpdateCustomLogoSyncSources] syncing custom logo configmap resources")
klog.V(4).Infof("%#v", configMapNames)

errors := []string{}
if len(co.trackables.customLogoConfigMaps) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

this does not seem to be right,cause we will be unsyncing on each sync loop run, except the first one, which will set the trackables

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this was intentional. On every sync loop we:

  • unsync all configmaps from the previous loop
  • resync all configmaps for the current loop
  • set trackables.customLogoConfigMaps to configmaps from current loop

This means that if the list of configmaps for the previous loop and the current loop are the same, the net result is that we just continue syncing the same configmaps.

klog.V(4).Info("[UpdateCustomLogoSyncSources] unsyncing custom logo configmap resources from previous sync loop...")
for _, configMapName := range co.trackables.customLogoConfigMaps {
err := co.UpdateCustomLogoSyncSource(configMapName, true)
if err != nil {
errors = append(errors, err.Error())
}
}

if len(errors) > 0 {
msg := fmt.Sprintf("error syncing custom logo configmap resources:\n%v", errors)
klog.V(4).Infof("[UpdateCustomLogoSyncSources] %s", msg)
return fmt.Errorf(msg), "FailedResourceSync"
}
}

source := resourcesynccontroller.ResourceLocation{}
logoConfigMapName := cmRef.Name
if len(configMapNames) > 0 {
// If the new list of synced configmaps is different than the last sync, we need to update the
// resource syncer with the new list, and re
klog.V(4).Infof("[UpdateCustomLogoSyncSources] syncing new custom logo configmap resources...")
for _, configMapName := range configMapNames {
err := co.UpdateCustomLogoSyncSource(configMapName, false)
if err != nil {
errors = append(errors, err.Error())
}
}

if logoConfigMapName != "" {
source.Name = logoConfigMapName
source.Namespace = api.OpenShiftConfigNamespace
}
// if no custom logo provided, sync an empty source to delete
err = c.resourceSyncer.SyncConfigMap(
resourcesynccontroller.ResourceLocation{Namespace: api.OpenShiftConsoleNamespace, Name: cmRef.Name},
source,
)
if err != nil {
return err, "FailedResourceSync"
if len(errors) > 0 {
msg := fmt.Sprintf("error syncing custom logo configmap resources:\n%v", errors)
klog.V(4).Infof("[UpdateCustomLogoSyncSources] %s", msg)
return fmt.Errorf(msg), "FailedResourceSync"
}
}

co.trackables.customLogoConfigMaps = configMapNames

klog.V(4).Info("[UpdateCustomLogoSyncSources] done")
return nil, ""
}

Expand All @@ -637,34 +666,57 @@ func (co *consoleOperator) ValidateCustomLogo(logoFileRef *operatorv1.ConfigMapF
logoImageKey := logoFileRef.Key

if (len(logoConfigMapName) == 0) != (len(logoImageKey) == 0) {
klog.V(4).Infoln("custom logo filename or key have not been set")
return customerrors.NewCustomLogoError("either custom logo filename or key have not been set"), "KeyOrFilenameInvalid"
msg := "custom logo filename or key have not been set"
klog.V(4).Infof("[ValidateCustomLogo] %s", msg)
return customerrors.NewCustomLogoError(msg), "KeyOrFilenameInvalid"
}
// fine if nothing set, but don't mount it
if len(logoConfigMapName) == 0 {
klog.V(4).Infoln("no custom logo configured")
klog.V(4).Infoln("[ValidateCustomLogo] no custom logo configured")
return nil, ""
}
logoConfigMap, err := co.configNSConfigMapLister.ConfigMaps(api.OpenShiftConfigNamespace).Get(logoConfigMapName)
// If we 404, the logo file may not have been created yet.
if err != nil {
klog.V(4).Infof("failed to get ConfigMap %v, %v", logoConfigMapName, err)
return customerrors.NewCustomLogoError(fmt.Sprintf("failed to get ConfigMap %v, %v", logoConfigMapName, err)), "FailedGet"
msg := fmt.Sprintf("failed to get ConfigMap %v, %v", logoConfigMapName, err)
klog.V(4).Infof("[ValidateCustomLogo] %s", msg)
return customerrors.NewCustomLogoError(msg), "FailedGet"
}

_, imageDataFound := logoConfigMap.BinaryData[logoImageKey]
if !imageDataFound {
_, imageDataFound = logoConfigMap.Data[logoImageKey]
}
if !imageDataFound {
klog.V(4).Infoln("custom logo file exists but no image provided")
return customerrors.NewCustomLogoError("custom logo file exists but no image provided"), "NoImageProvided"
msg := "custom logo file exists but no image provided"
klog.V(4).Infof("[ValidateCustomLogo] %s", msg)
return customerrors.NewCustomLogoError(msg), "NoImageProvided"
}

klog.V(4).Infof("custom logo %s ok to mount", logoConfigMapName)
klog.V(4).Infof("[ValidateCustomLogo] custom logo %s ok to mount", logoConfigMapName)
return nil, ""
}

func (co *consoleOperator) UpdateCustomLogoSyncSource(targetName string, unsync bool) error {
source := resourcesynccontroller.ResourceLocation{}
if !unsync {
source.Name = targetName
source.Namespace = api.OpenShiftConfigNamespace
}

target := resourcesynccontroller.ResourceLocation{
Namespace: api.OpenShiftConsoleNamespace,
Name: targetName,
}

if unsync {
klog.V(4).Infof("[UpdateCustomLogoSyncSource] unsyncing %s", targetName)
} else {
klog.V(4).Infof("[UpdateCustomLogoSyncSource] syncing %s", targetName)
}
return co.resourceSyncer.SyncConfigMap(target, source)
}

func (co *consoleOperator) GetAvailablePlugins(enabledPluginsNames []string) []*v1.ConsolePlugin {
var availablePlugins []*v1.ConsolePlugin
for _, pluginName := range utilsub.RemoveDuplicateStr(enabledPluginsNames) {
Expand Down
31 changes: 15 additions & 16 deletions test/e2e/brand_customization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,22 +115,21 @@ func TestCustomBrand(t *testing.T) {
t.Fatalf("could not clear customizations from operator config (%s)", err)
}

// TODO re-enable this check once https://issues.redhat.com/browse/OCPBUGS-54780 is fixed
// t.Logf("ensure that the %s configmap is no longer synced to 'openshift-console' namespace", suite.customLogoConfigMapName)
// err = wait.Poll(pollFrequency, pollStandardMax, func() (stop bool, err error) {
// _, err = framework.GetCustomLogoConfigMap(client, suite.customLogoConfigMapName)
// if apiErrors.IsNotFound(err) {
// return true, nil
// }
// if err != nil {
// return false, err
// }
// // Try until timeout
// return false, nil
// })
// if err != nil {
// t.Fatalf("timed out checking for configmap '%s' in 'openshift-console'", suite.customLogoConfigMapName)
// }
t.Logf("ensure that the %s configmap is no longer synced to 'openshift-console' namespace", suite.customLogoConfigMapName)
err = wait.Poll(pollFrequency, pollStandardMax, func() (stop bool, err error) {
_, err = framework.GetCustomLogoConfigMap(client, suite.customLogoConfigMapName)
if apiErrors.IsNotFound(err) {
return true, nil
}
if err != nil {
return false, err
}
// Try until timeout
return false, nil
})
if err != nil {
t.Fatalf("configmap '%s' was orphaned in 'openshift-console' namespace", suite.customLogoConfigMapName)
}
}
}

Expand Down