Skip to content
Merged
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
1 change: 1 addition & 0 deletions hack/test-unit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ PACKAGES_TO_TEST=(
"github.com/openshift/console-operator/pkg/console/operator"
"github.com/openshift/console-operator/pkg/console/starter"
"github.com/openshift/console-operator/pkg/console/subresource/configmap"
"github.com/openshift/console-operator/pkg/console/subresource/consoleserver"
Copy link
Member Author

Choose a reason for hiding this comment

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

It appears we have been missing this pkg in our unit tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Not ideal to have to manually add these eh? Prob should update this script to loop the directory instead, but looks good for now.

"github.com/openshift/console-operator/pkg/console/subresource/deployment"
"github.com/openshift/console-operator/pkg/console/subresource/oauthclient"
"github.com/openshift/console-operator/pkg/console/subresource/route"
Expand Down
1 change: 1 addition & 0 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const (
OpenShiftConsoleConfigMapName = "console-config"
OpenShiftConsolePublicConfigMapName = "console-public"
ServiceCAConfigMapName = "service-ca"
RouterCAConfigMapName = "router-ca"
OpenShiftConsoleDeploymentName = OpenShiftConsoleName
OpenShiftConsoleServiceName = OpenShiftConsoleName
OpenShiftConsoleRouteName = OpenShiftConsoleName
Expand Down
138 changes: 138 additions & 0 deletions pkg/console/controllers/resourcesyncdestination/controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package resourcesyncdestination

import (
"fmt"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
coreinformersv1 "k8s.io/client-go/informers/core/v1"
coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
"k8s.io/klog"

operatorsv1 "github.com/openshift/api/operator/v1"
operatorclientv1 "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1"
operatorinformersv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
"github.com/openshift/console-operator/pkg/api"
"github.com/openshift/library-go/pkg/operator/events"
)

const (
controllerWorkQueueKey = "resource-sync-destination-work-queue-key"
controllerName = "ConsoleResourceSyncDestinationController"
)

type ResourceSyncDestinationController struct {
operatorConfigClient operatorclientv1.ConsoleInterface
configMapClient coreclientv1.ConfigMapsGetter
// events
cachesToSync []cache.InformerSynced
queue workqueue.RateLimitingInterface
recorder events.Recorder
}

func NewResourceSyncDestinationController(
// operatorconfig
operatorConfigClient operatorclientv1.ConsoleInterface,
operatorConfigInformer operatorinformersv1.ConsoleInformer,
// configmap
corev1Client coreclientv1.CoreV1Interface,
configMapInformer coreinformersv1.ConfigMapInformer,
// events
recorder events.Recorder,
) *ResourceSyncDestinationController {
corev1Client.ConfigMaps(api.OpenShiftConsoleNamespace)

ctrl := &ResourceSyncDestinationController{
operatorConfigClient: operatorConfigClient,
configMapClient: corev1Client,
// events
recorder: recorder,
cachesToSync: nil,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerName),
}

configMapInformer.Informer().AddEventHandler(ctrl.newEventHandler())
operatorConfigInformer.Informer().AddEventHandler(ctrl.newEventHandler())
ctrl.cachesToSync = append(ctrl.cachesToSync,
operatorConfigInformer.Informer().HasSynced,
configMapInformer.Informer().HasSynced,
)

return ctrl
}

func (c *ResourceSyncDestinationController) sync() error {
operatorConfig, err := c.operatorConfigClient.Get(api.ConfigResourceName, metav1.GetOptions{})
if err != nil {
return err
}

switch operatorConfig.Spec.ManagementState {
case operatorsv1.Managed:
klog.V(4).Infoln("console is in a managed state: syncing router-ca configmap")
case operatorsv1.Unmanaged:
klog.V(4).Infoln("console is in an unmanaged state: skipping router-ca configmap sync")
return nil
case operatorsv1.Removed:
klog.V(4).Infoln("console is in an removed state: removing synced router-ca configmap")
return c.removeRouterCAConfigMap()
default:
return fmt.Errorf("unknown state: %v", operatorConfig.Spec.ManagementState)
}

return err
}

func (c *ResourceSyncDestinationController) removeRouterCAConfigMap() error {
klog.V(2).Info("deleting router-ca configmap")
defer klog.V(2).Info("finished deleting router-ca configmap")
return c.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Delete(api.RouterCAConfigMapName, &metav1.DeleteOptions{})
}

func (c *ResourceSyncDestinationController) Run(workers int, stopCh <-chan struct{}) {
defer runtime.HandleCrash()
defer c.queue.ShutDown()
klog.Infof("starting %v", controllerName)
defer klog.Infof("shutting down %v", controllerName)
if !cache.WaitForCacheSync(stopCh, c.cachesToSync...) {
klog.Infoln("caches did not sync")
runtime.HandleError(fmt.Errorf("caches did not sync"))
return
}
// only start one worker
go wait.Until(c.runWorker, time.Second, stopCh)
<-stopCh
}

func (c *ResourceSyncDestinationController) runWorker() {
for c.processNextWorkItem() {
}
}

func (c *ResourceSyncDestinationController) processNextWorkItem() bool {
processKey, quit := c.queue.Get()
if quit {
return false
}
defer c.queue.Done(processKey)
err := c.sync()
if err == nil {
c.queue.Forget(processKey)
return true
}
runtime.HandleError(fmt.Errorf("%v failed with : %v", processKey, err))
c.queue.AddRateLimited(processKey)
return true
}

func (c *ResourceSyncDestinationController) newEventHandler() cache.ResourceEventHandler {
return cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) { c.queue.Add(controllerWorkQueueKey) },
UpdateFunc: func(old, new interface{}) { c.queue.Add(controllerWorkQueueKey) },
DeleteFunc: func(obj interface{}) { c.queue.Add(controllerWorkQueueKey) },
}
}
35 changes: 32 additions & 3 deletions pkg/console/operator/sync_v400.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console,
// The sync loop may not settle, we are unable to honor it in current state.
status.HandleProgressingOrDegraded(updatedOperatorConfig, "CustomLogoSync", customLogoErrReason, customLogoError)

routerCAConfigMap, routerCAErrReason, routerCAError := co.ValidateRouterCAConfigMap()
status.HandleProgressingOrDegraded(updatedOperatorConfig, "RouterCAValidation", routerCAErrReason, routerCAError)

sec, secChanged, secErr := co.SyncSecret(set.Operator)
toUpdate = toUpdate || secChanged
status.HandleProgressingOrDegraded(updatedOperatorConfig, "OAuthClientSecretSync", "FailedApply", secErr)
Expand All @@ -109,7 +112,7 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console,
return oauthErr
}

actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(set.Operator, cm, serviceCAConfigMap, trustedCAConfigMap, sec, rt, set.Proxy, customLogoCanMount)
actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(set.Operator, cm, serviceCAConfigMap, routerCAConfigMap, trustedCAConfigMap, sec, rt, set.Proxy, customLogoCanMount)
toUpdate = toUpdate || depChanged
status.HandleProgressingOrDegraded(updatedOperatorConfig, "DeploymentSync", depErrReason, depErr)
if depErr != nil {
Expand Down Expand Up @@ -231,13 +234,14 @@ func (co *consoleOperator) SyncDeployment(
operatorConfig *operatorv1.Console,
cm *corev1.ConfigMap,
serviceCAConfigMap *corev1.ConfigMap,
routerCAConfigMap *corev1.ConfigMap,
trustedCAConfigMap *corev1.ConfigMap,
sec *corev1.Secret,
rt *routev1.Route,
proxyConfig *configv1.Proxy,
canMountCustomLogo bool) (consoleDeployment *appsv1.Deployment, changed bool, reason string, err error) {

requiredDeployment := deploymentsub.DefaultDeployment(operatorConfig, cm, serviceCAConfigMap, trustedCAConfigMap, sec, rt, proxyConfig, canMountCustomLogo)
requiredDeployment := deploymentsub.DefaultDeployment(operatorConfig, cm, serviceCAConfigMap, routerCAConfigMap, trustedCAConfigMap, sec, rt, proxyConfig, canMountCustomLogo)
expectedGeneration := getDeploymentGeneration(co)
genChanged := operatorConfig.ObjectMeta.Generation != operatorConfig.Status.ObservedGeneration

Expand Down Expand Up @@ -307,7 +311,18 @@ func (co *consoleOperator) SyncConfigMap(
return nil, false, "FailedManagedConfig", mcErr
}

defaultConfigmap, _, err := configmapsub.DefaultConfigMap(operatorConfig, consoleConfig, managedConfig, infrastructureConfig, consoleRoute)
useDefaultCAFile := true
// We are syncing the `router-ca` configmap from `openshift-config-managed` to `openshift-console`.
// `router-ca` is only published in `openshift-config-managed` if an operator-generated default certificate is used.
// It will not exist if all ingresscontrollers user admin-provided default certificates.
// If the `router-ca` configmap in `openshift-console` exist we should mount that to the console container,
// otherwise default to `/var/run/secrets/kubernetes.io/serviceaccount/ca.crt`
_, rcaErr := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(api.RouterCAConfigMapName, metav1.GetOptions{})
if rcaErr != nil && apierrors.IsNotFound(rcaErr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about other errors? this only covers 404.

Copy link
Contributor

Choose a reason for hiding this comment

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

is there anything else that would matter?

Copy link
Member Author

Choose a reason for hiding this comment

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

was thinking about it and wasn't really sure if any other errors should matter in this context... saying that not really sure what should be the reason and type of the created condition, if there will be other error type... also if it makes sense to even create one in this case...

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should just add a log and call it good:

if rcaErr != nil {
  klog.V(4).Infof("router-ca GET error: %s",rcaErr)
}

Agree with you, probably doesn't matter, unless there is some funny state we aren't thinking about. But a 4+ log level dumping some detail may be helpful.

useDefaultCAFile = false
}

defaultConfigmap, _, err := configmapsub.DefaultConfigMap(operatorConfig, consoleConfig, managedConfig, infrastructureConfig, consoleRoute, useDefaultCAFile)
if err != nil {
return nil, false, "FailedConsoleConfigBuilder", err
}
Expand Down Expand Up @@ -430,6 +445,20 @@ func (c *consoleOperator) SyncCustomLogoConfigMap(operatorConfig *operatorsv1.Co
return okToMount, reason, err
}

func (c *consoleOperator) ValidateRouterCAConfigMap() (routerCA *corev1.ConfigMap, reason string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Pondering if this can live in the new controller instead of adding it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, probably not now yet, as the actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(set.Operator, cm, serviceCAConfigMap, routerCAConfigMap, trustedCAConfigMap, sec, rt, set.Proxy, customLogoCanMount) needs it. Future TODO: break that down into smaller bits.

routerCAConfigMap, err := c.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(api.RouterCAConfigMapName, metav1.GetOptions{})
if err != nil {
klog.V(4).Infoln("router-ca configmap not found")
return nil, "FailedGet", fmt.Errorf("router-ca configmap not found")
}

_, caBundle := routerCAConfigMap.Data["ca-bundle.crt"]
if !caBundle {
return nil, "MissingRouterCABundle", fmt.Errorf("router-ca configmap is missing ca-bundle.crt data")
}
return routerCAConfigMap, "", nil
}

// 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
Expand Down
36 changes: 35 additions & 1 deletion pkg/console/starter/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
operatorv1 "github.com/openshift/api/operator"
"github.com/openshift/console-operator/pkg/api"
"github.com/openshift/console-operator/pkg/console/controllers/clidownloads"
"github.com/openshift/console-operator/pkg/console/controllers/resourcesyncdestination"
"github.com/openshift/console-operator/pkg/console/operatorclient"
"github.com/openshift/library-go/pkg/controller/controllercmd"
"github.com/openshift/library-go/pkg/operator/management"
Expand Down Expand Up @@ -145,6 +146,11 @@ func RunOperator(ctx *controllercmd.ControllerContext) error {

resourceSyncerInformers, resourceSyncer := getResourceSyncer(ctx, clientwrapper.WithoutSecret(kubeClient), operatorClient)

err = startResourceSyncing(resourceSyncer)
if err != nil {
return err
}

// TODO: rearrange these into informer,client pairs, NOT separated.
consoleOperator := operator.NewConsoleOperator(
// top level config
Expand Down Expand Up @@ -173,13 +179,13 @@ func RunOperator(ctx *controllercmd.ControllerContext) error {
recorder,
resourceSyncer,
)

cliDownloadsController := clidownloads.NewCLIDownloadsSyncController(
// clients
operatorClient,
operatorConfigClient.OperatorV1(),
consoleClient.ConsoleV1().ConsoleCLIDownloads(),
routesClient.RouteV1(),

// informers
operatorConfigInformers.Operator().V1().Consoles(), // OperatorConfig
consoleInformers.Console().V1().ConsoleCLIDownloads(), // ConsoleCliDownloads
Expand All @@ -188,6 +194,19 @@ func RunOperator(ctx *controllercmd.ControllerContext) error {
recorder,
)

// ResourceSyncDestinationController contains additional logic for all the
// secrets and configmaps that we resourceSyncer is taking care of
resourceSyncDestinationController := resourcesyncdestination.NewResourceSyncDestinationController(
// operatorconfig
operatorConfigClient.OperatorV1().Consoles(),
operatorConfigInformers.Operator().V1().Consoles(),
// configmap
kubeClient.CoreV1(),
kubeInformersNamespaced.Core().V1().ConfigMaps(),
// events
recorder,
)

consoleServiceController := service.NewServiceSyncController(
// clients
operatorConfigClient.OperatorV1().Consoles(), // operator config so we can update status
Expand Down Expand Up @@ -261,6 +280,7 @@ func RunOperator(ctx *controllercmd.ControllerContext) error {
}

go consoleServiceController.Run(1, ctx.Done())
go resourceSyncDestinationController.Run(1, ctx.Done())
go consoleOperator.Run(ctx.Done())
go resourceSyncer.Run(1, ctx.Done())
go clusterOperatorStatus.Run(1, ctx.Done())
Expand All @@ -274,11 +294,25 @@ func RunOperator(ctx *controllercmd.ControllerContext) error {
return fmt.Errorf("stopped")
}

// startResourceSyncing should start syncing process of all secrets and configmaps that need to be synced.
func startResourceSyncing(resourceSyncer *resourcesynccontroller.ResourceSyncController) error {
// sync: 'router-ca' configmap
// from: 'openshift-config-managed' namespace
// to: 'openshift-console' namespace
err := resourceSyncer.SyncConfigMap(
resourcesynccontroller.ResourceLocation{Name: api.RouterCAConfigMapName, Namespace: api.OpenShiftConsoleNamespace},
resourcesynccontroller.ResourceLocation{Name: api.RouterCAConfigMapName, Namespace: api.OpenShiftConfigManagedNamespace},
)

return err
}

func getResourceSyncer(ctx *controllercmd.ControllerContext, kubeClient kubernetes.Interface, operatorClient v1helpers.OperatorClient) (v1helpers.KubeInformersForNamespaces, *resourcesynccontroller.ResourceSyncController) {
resourceSyncerInformers := v1helpers.NewKubeInformersForNamespaces(
kubeClient,
api.OpenShiftConfigNamespace,
api.OpenShiftConsoleNamespace,
api.OpenShiftConfigManagedNamespace,
)
resourceSyncer := resourcesynccontroller.NewResourceSyncController(
operatorClient,
Expand Down
5 changes: 4 additions & 1 deletion pkg/console/subresource/configmap/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ func DefaultConfigMap(
consoleConfig *configv1.Console,
managedConfig *corev1.ConfigMap,
infrastructureConfig *configv1.Infrastructure,
rt *routev1.Route) (consoleConfigmap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) {
rt *routev1.Route,
useDefaultCAFile bool) (consoleConfigmap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) {

defaultBuilder := &consoleserver.ConsoleServerCLIConfigBuilder{}
defaultConfig, err := defaultBuilder.Host(rt.Spec.Host).
LogoutURL(defaultLogoutURL).
Brand(DEFAULT_BRAND).
DocURL(DEFAULT_DOC_URL).
RouterCA(useDefaultCAFile).
APIServerURL(getApiUrl(infrastructureConfig)).
ConfigYAML()

Expand All @@ -55,6 +57,7 @@ func DefaultConfigMap(
LogoutURL(consoleConfig.Spec.Authentication.LogoutRedirect).
Brand(operatorConfig.Spec.Customization.Brand).
DocURL(operatorConfig.Spec.Customization.DocumentationBaseURL).
RouterCA(useDefaultCAFile).
APIServerURL(getApiUrl(infrastructureConfig)).
CustomLogoFile(operatorConfig.Spec.Customization.CustomLogoFile.Key).
CustomProductName(operatorConfig.Spec.Customization.CustomProductName).
Expand Down
Loading