Merge pull request #966 from openshift-cherrypick-robot/cherry-pick-965-to-release-4.18

[release-4.18] OCPBUGS-52294: Custom route TLS should be optional when IngressController's DefaultCertificate is set

Author: openshift-merge-bot[bot]

manifests/03-rbac-role-cluster.yaml

@@ -98,6 +98,14 @@ rules:
       - update
       - delete
       - patch
+  - apiGroups:
+      - operator.openshift.io
+    resources:
+      - ingresscontrollers
+    verbs:
+      - get
+      - list
+      - watch
   - apiGroups:
       - console.openshift.io
     resources:

pkg/api/api.go

@@ -48,6 +48,11 @@ const (
 	V1Alpha1PluginI18nAnnotation        = "console.openshift.io/use-i18n"
 	VersionResourceName                 = "version"
 
+	// ingress instance named "default" is the OOTB ingresscontroller
+	// this is an implicit stable API
+	DefaultIngressController   = "default"
+	IngressControllerNamespace = "openshift-ingress-operator"
+
 	OAuthClientName                         = OpenShiftConsoleName
 	OpenShiftConsoleDeploymentName          = OpenShiftConsoleName
 	OpenShiftConsoleDownloadsDeploymentName = DownloadsResourceName

pkg/console/controllers/route/controller.go

@@ -40,13 +40,15 @@ type RouteSyncController struct {
 	routeName            string
 	isHealthCheckEnabled bool
 	// clients
-	operatorClient             v1helpers.OperatorClient
-	routeClient                routeclientv1.RoutesGetter
-	operatorConfigLister       operatorv1listers.ConsoleLister
-	ingressConfigLister        configlistersv1.IngressLister
-	secretLister               corev1listers.SecretLister
-	infrastructureConfigLister configlistersv1.InfrastructureLister
-	clusterVersionLister       configlistersv1.ClusterVersionLister
+	operatorClient                v1helpers.OperatorClient
+	routeClient                   routeclientv1.RoutesGetter
+	operatorConfigLister          operatorv1listers.ConsoleLister
+	ingressConfigLister           configlistersv1.IngressLister
+	ingressControllerLister       operatorv1listers.IngressControllerLister
+	secretLister                  corev1listers.SecretLister
+	ingressControllerSecretLister corev1listers.SecretLister
+	infrastructureConfigLister    configlistersv1.InfrastructureLister
+	clusterVersionLister          configlistersv1.ClusterVersionLister
 }
 
 func NewRouteSyncController(
@@ -59,6 +61,7 @@ func NewRouteSyncController(
 	routev1Client routeclientv1.RoutesGetter,
 	// informers
 	operatorConfigInformer v1.ConsoleInformer,
+	ingressControllerInformer v1.IngressControllerInformer,
 	secretInformer coreinformersv1.SecretInformer,
 	routeInformer routesinformersv1.RouteInformer,
 	// events
@@ -70,6 +73,7 @@ func NewRouteSyncController(
 		operatorClient:             operatorClient,
 		operatorConfigLister:       operatorConfigInformer.Lister(),
 		ingressConfigLister:        configInformer.Config().V1().Ingresses().Lister(),
+		ingressControllerLister:    ingressControllerInformer.Lister(),
 		routeClient:                routev1Client,
 		secretLister:               secretInformer.Lister(),
 		infrastructureConfigLister: configInformer.Config().V1().Infrastructures().Lister(),
@@ -86,6 +90,9 @@ func NewRouteSyncController(
 			configV1Informers.Ingresses().Informer(),
 		).WithInformers(
 		secretInformer.Informer(),
+	).WithFilteredEventsInformers(
+		util.IncludeNamesFilter(api.DefaultIngressController),
+		ingressControllerInformer.Informer(),
 	).WithFilteredEventsInformers( // route
 		util.IncludeNamesFilter(routeName, routesub.GetCustomRouteName(routeName)),
 		routeInformer.Informer(),
@@ -135,6 +142,11 @@ func (c *RouteSyncController) Sync(ctx context.Context, controllerContext factor
 		return statusHandler.FlushAndReturn(err)
 	}
 
+	ingressControllerConfig, err := c.ingressControllerLister.IngressControllers(api.IngressControllerNamespace).Get(api.DefaultIngressController)
+	if err != nil {
+		return statusHandler.FlushAndReturn(err)
+	}
+
 	clusterVersionConfig, err := c.clusterVersionLister.Get("version")
 	if err != nil {
 		return statusHandler.FlushAndReturn(err)
@@ -157,7 +169,7 @@ func (c *RouteSyncController) Sync(ctx context.Context, controllerContext factor
 	// try to sync the custom route first. If the sync fails for any reason, error
 	// out the sync loop and inform about this fact instead of putting default
 	// route into inaccessible state.
-	_, customRouteErrReason, customRouteErr := c.SyncCustomRoute(ctx, routeConfig, controllerContext)
+	_, customRouteErrReason, customRouteErr := c.SyncCustomRoute(ctx, routeConfig, ingressControllerConfig, controllerContext)
 	statusHandler.AddConditions(status.HandleProgressingOrDegraded(typePrefix, customRouteErrReason, customRouteErr))
 	statusHandler.AddCondition(status.HandleUpgradable(typePrefix, customRouteErrReason, customRouteErr))
 	if customRouteErr != nil {
@@ -214,7 +226,7 @@ func (c *RouteSyncController) SyncDefaultRoute(ctx context.Context, routeConfig
 // 2. if secret is defined, verify the TLS certificate and key
 // 4. create the custom console route, if custom TLS certificate and key are defined use them
 // 5. apply the custom route
-func (c *RouteSyncController) SyncCustomRoute(ctx context.Context, routeConfig *routesub.RouteConfig, controllerContext factory.SyncContext) (*routev1.Route, string, error) {
+func (c *RouteSyncController) SyncCustomRoute(ctx context.Context, routeConfig *routesub.RouteConfig, ingressControllerConfig *operatorsv1.IngressController, controllerContext factory.SyncContext) (*routev1.Route, string, error) {
 	if !routeConfig.IsCustomHostnameSet() {
 		if err := c.removeRoute(ctx, routesub.GetCustomRouteName(c.routeName)); err != nil {
 			return nil, "FailedDeleteCustomRoutes", err
@@ -228,7 +240,7 @@ func (c *RouteSyncController) SyncCustomRoute(ctx context.Context, routeConfig *
 		return nil, "", nil
 	}
 
-	if configErr := c.ValidateCustomRouteConfig(ctx, routeConfig); configErr != nil {
+	if configErr := c.ValidateCustomRouteConfig(ctx, routeConfig, ingressControllerConfig); configErr != nil {
 		return nil, "InvalidCustomRouteConfig", configErr
 	}
 
@@ -284,7 +296,13 @@ func (c *RouteSyncController) GetDefaultRouteTLSSecret(ctx context.Context, rout
 	return secret, nil
 }
 
-func (c *RouteSyncController) ValidateCustomRouteConfig(ctx context.Context, routeConfig *routesub.RouteConfig) error {
+func (c *RouteSyncController) ValidateCustomRouteConfig(ctx context.Context, routeConfig *routesub.RouteConfig, ingressControllerConfig *operatorsv1.IngressController) error {
+	// Check if the default cetrificate is set in the ingress controller config.
+	// If it is, then the custom route TLS secret is optional.
+	if ingressControllerConfig.Spec.DefaultCertificate != nil {
+		return nil
+	}
+
 	// Check if the custom hostname has cluster domain suffix, which indicates
 	// if a secret that contains TLS certificate and key needs to exist in the
 	// `openshift-config` namespace and referenced in  the operator config.

pkg/console/starter/starter.go

@@ -23,7 +23,6 @@ import (
 	// openshift
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/api/oauth"
-	operator "github.com/openshift/api/operator"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/openshift/console-operator/pkg/api"
 	"github.com/openshift/console-operator/pkg/console/clientwrapper"
@@ -379,6 +378,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 		routesClient.RouteV1(),
 		// route
 		operatorConfigInformers.Operator().V1().Consoles(),
+		operatorConfigInformers.Operator().V1().IngressControllers(),
 		kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers
 		routesInformersNamespaced.Route().V1().Routes(),
 		// events
@@ -396,6 +396,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 		routesClient.RouteV1(),
 		// route
 		operatorConfigInformers.Operator().V1().Consoles(),
+		operatorConfigInformers.Operator().V1().IngressControllers(),
 		kubeInformersConfigNamespaced.Core().V1().Secrets(), // `openshift-config` namespace informers
 		routesInformersNamespaced.Route().V1().Routes(),
 		// events
@@ -433,7 +434,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 	clusterOperatorStatus := status.NewClusterOperatorStatusController(
 		api.ClusterOperatorName,
 		[]configv1.ObjectReference{
-			{Group: operator.GroupName, Resource: "consoles", Name: api.ConfigResourceName},
+			{Group: operatorv1.GroupName, Resource: "consoles", Name: api.ConfigResourceName},
 			{Group: configv1.GroupName, Resource: "consoles", Name: api.ConfigResourceName},
 			{Group: configv1.GroupName, Resource: "infrastructures", Name: api.ConfigResourceName},
 			{Group: configv1.GroupName, Resource: "proxies", Name: api.ConfigResourceName},

pkg/console/subresource/route/route.go

@@ -28,12 +28,6 @@ import (
 	"github.com/openshift/console-operator/pkg/api"
 )
 
-const (
-	// ingress instance named "default" is the OOTB ingresscontroller
-	// this is an implicit stable API
-	defaultIngressController = "default"
-)
-
 // holds information about custom TLS certificate and its key
 type CustomTLSCert struct {
 	Certificate string
@@ -48,8 +42,8 @@ type RouteConfig struct {
 }
 
 type RouteControllerSpec struct {
-	hostname   string
-	secretName string
+	Hostname   string
+	SecretName string
 }
 
 func getComponentRouteSpec(ingressConfig *configv1.Ingress, componentName string) *configv1.ComponentRouteSpec {
@@ -72,17 +66,17 @@ func getComponentRouteStatus(ingressConfig *configv1.Ingress, componentName stri
 
 func NewRouteConfig(operatorConfig *operatorv1.Console, ingressConfig *configv1.Ingress, routeName string) *RouteConfig {
 	defaultRoute := RouteControllerSpec{
-		hostname: GetDefaultRouteHost(routeName, ingressConfig),
+		Hostname: GetDefaultRouteHost(routeName, ingressConfig),
 	}
 	var customRoute RouteControllerSpec
 	var isIngressConfigCustomHostnameSet bool
 
-	// Custom hostname in ingress config takes precedent over console operator's config
+	// Custom Hostname in ingress config takes precedent over console operator's config
 	componentRouteSpec := getComponentRouteSpec(ingressConfig, routeName)
 	if componentRouteSpec != nil {
-		customRoute.hostname = string(componentRouteSpec.Hostname)
+		customRoute.Hostname = string(componentRouteSpec.Hostname)
 		if componentRouteSpec.ServingCertKeyPairSecret.Name != "" {
-			customRoute.secretName = componentRouteSpec.ServingCertKeyPairSecret.Name
+			customRoute.SecretName = componentRouteSpec.ServingCertKeyPairSecret.Name
 		}
 		isIngressConfigCustomHostnameSet = true
 	}
@@ -92,67 +86,79 @@ func NewRouteConfig(operatorConfig *operatorv1.Console, ingressConfig *configv1.
 	// TLS for the default route.
 	if !isIngressConfigCustomHostnameSet && routeName == api.OpenShiftConsoleRouteName {
 		if len(operatorConfig.Spec.Route.Secret.Name) != 0 {
-			customRoute.secretName = operatorConfig.Spec.Route.Secret.Name
+			customRoute.SecretName = operatorConfig.Spec.Route.Secret.Name
 		}
-		customRoute.hostname = operatorConfig.Spec.Route.Hostname
+		customRoute.Hostname = operatorConfig.Spec.Route.Hostname
 	}
 
-	// if hostname for custom route is the same as the hostname for the default route
-	// OR if the custom route hostname is not set:
+	// if Hostname for custom route is the same as the Hostname for the default route
+	// OR if the custom route Hostname is not set:
 	// - if the custom route TLS secret is set and set it for the default route
-	// - unset hostname and TLS secret for the custom route
-	if defaultRoute.hostname == customRoute.hostname || len(customRoute.hostname) == 0 {
-		if len(customRoute.secretName) != 0 {
-			defaultRoute.secretName = customRoute.secretName
+	// - unset Hostname and TLS secret for the custom route
+	if defaultRoute.Hostname == customRoute.Hostname || len(customRoute.Hostname) == 0 {
+		if len(customRoute.SecretName) != 0 {
+			defaultRoute.SecretName = customRoute.SecretName
 		}
-		customRoute.hostname = ""
-		customRoute.secretName = ""
+		customRoute.Hostname = ""
+		customRoute.SecretName = ""
 	}
 
-	customHostnameSpec := &RouteConfig{
+	routeConfig := &RouteConfig{
 		defaultRoute: defaultRoute,
 		customRoute:  customRoute,
 		domain:       ingressConfig.Spec.Domain,
 		routeName:    routeName,
 	}
 
-	return customHostnameSpec
+	return routeConfig
+}
+
+func (rc *RouteConfig) GetRouteName() string {
+	return rc.routeName
+}
+
+func (rc *RouteConfig) GetDefaultRoute() RouteControllerSpec {
+	return rc.defaultRoute
+}
+
+func (rc *RouteConfig) GetCustomRoute() RouteControllerSpec {
+	return rc.customRoute
 }
 
 func (rc *RouteConfig) HostnameMatch() bool {
-	return rc.customRoute.hostname == rc.defaultRoute.hostname
+	return rc.customRoute.Hostname == rc.defaultRoute.Hostname
 }
 
 func (rc *RouteConfig) IsCustomHostnameSet() bool {
-	return len(rc.customRoute.hostname) != 0
+	return len(rc.customRoute.Hostname) != 0
 }
 
 func (rc *RouteConfig) GetCustomRouteHostname() string {
-	return rc.customRoute.hostname
+	return rc.customRoute.Hostname
 }
 
 func (rc *RouteConfig) IsCustomTLSSecretSet() bool {
-	return len(rc.customRoute.secretName) != 0
+	return len(rc.customRoute.SecretName) != 0
 }
 
 func (rc *RouteConfig) IsDefaultTLSSecretSet() bool {
-	return len(rc.defaultRoute.secretName) != 0
+	return len(rc.defaultRoute.SecretName) != 0
 }
 
 func (rc *RouteConfig) GetCustomTLSSecretName() string {
-	return rc.customRoute.secretName
+	return rc.customRoute.SecretName
 }
 
 func (rc *RouteConfig) GetDefaultTLSSecretName() string {
-	return rc.defaultRoute.secretName
+	return rc.defaultRoute.SecretName
 }
 
 func (rc *RouteConfig) GetDomain() string {
 	return rc.domain
 }
 
 // Default `console` route points by default to the `console` service.
-// If custom hostname for the console is set, then the default route
+// If custom Hostname for the console is set, then the default route
 // should point to the redirect `console-redirect` service and the
 // created custom route should be pointing to the `console` service.
 func (rc *RouteConfig) DefaultRoute(tlsConfig *CustomTLSCert, ingressConfig *configv1.Ingress) *routev1.Route {
@@ -169,11 +175,16 @@ func (rc *RouteConfig) DefaultRoute(tlsConfig *CustomTLSCert, ingressConfig *con
 
 func (rc *RouteConfig) CustomRoute(tlsConfig *CustomTLSCert, routeName string) *routev1.Route {
 	route := resourceread.ReadRouteV1OrDie(bindata.MustAsset(fmt.Sprintf("assets/routes/%s-custom-route.yaml", rc.routeName)))
-	route.Spec.Host = rc.customRoute.hostname
+	route.Spec.Host = rc.customRoute.Hostname
 	setTLS(tlsConfig, route)
 	return route
 }
 
+func (rc *RouteConfig) UnsetTLS() {
+	rc.defaultRoute.SecretName = ""
+	rc.customRoute.SecretName = ""
+}
+
 func GetDefaultRouteHost(routeName string, ingressConfig *configv1.Ingress) string {
 	return fmt.Sprintf("%s-%s.%s", routeName, api.OpenShiftConsoleNamespace, ingressConfig.Spec.Domain)
 }