Improve oauth handling.

deploy/crds/org_v1_che_cr.yaml

@@ -124,9 +124,6 @@ spec:
     identityProviderRealm: ''
     # id of a keycloak client. This client will be created, when externalIdentityProvider is false, otherwise passed to Che server
     identityProviderClientId: ''
-    # instructs an Operator to enable OpenShift v3 identity provider in Keycloak,
-    # as well as create respective oAuthClient and configure Che configMap accordingly
-    openShiftoAuth: true
     # name of oAuthClient used in OpenShift v3 identity provider in Keycloak realm. Auto generated if left blank
     oAuthClientName: ''
     # secret used in oAuthClient. Auto generated if left blank

deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml

@@ -84,13 +84,13 @@ metadata:
     categories: Developer Tools
     certified: "false"
     containerImage: quay.io/eclipse/che-operator:nightly
-    createdAt: "2020-12-14T15:43:45Z"
+    createdAt: "2020-12-15T14:08:40Z"
     description: A Kube-native development solution that delivers portable and collaborative
       developer workspaces.
     operatorframework.io/suggested-namespace: eclipse-che
     repository: https://github.com/eclipse/che-operator
     support: Eclipse Foundation
-  name: eclipse-che-preview-kubernetes.v7.23.0-48.nightly
+  name: eclipse-che-preview-kubernetes.v7.24.0-50.nightly
   namespace: placeholder
 spec:
   apiservicedefinitions: {}
@@ -494,4 +494,4 @@ spec:
   maturity: stable
   provider:
     name: Eclipse Foundation
-  version: 7.23.0-48.nightly
+  version: 7.24.0-50.nightly

deploy/olm-catalog/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml

@@ -20,8 +20,7 @@ metadata:
               "identityProviderRealm": "",
               "identityProviderURL": "",
               "oAuthClientName": "",
-              "oAuthSecret": "",
-              "openShiftoAuth": true
+              "oAuthSecret": ""
             },
             "database": {
               "chePostgresDb": "",
@@ -76,13 +75,13 @@ metadata:
     categories: Developer Tools, OpenShift Optional
     certified: "false"
     containerImage: quay.io/eclipse/che-operator:nightly
-    createdAt: "2020-12-14T15:43:55Z"
+    createdAt: "2020-12-15T14:08:47Z"
     description: A Kube-native development solution that delivers portable and collaborative
       developer workspaces in OpenShift.
     operatorframework.io/suggested-namespace: eclipse-che
     repository: https://github.com/eclipse/che-operator
     support: Eclipse Foundation
-  name: eclipse-che-preview-openshift.v7.23.0-48.nightly
+  name: eclipse-che-preview-openshift.v7.24.0-50.nightly
   namespace: placeholder
 spec:
   apiservicedefinitions: {}
@@ -514,4 +513,4 @@ spec:
   maturity: stable
   provider:
     name: Eclipse Foundation
-  version: 7.23.0-48.nightly
+  version: 7.24.0-50.nightly

e2e/create.go

@@ -13,6 +13,7 @@ package main
 
 import (
 	orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1"
+	"github.com/eclipse/che-operator/pkg/util"
 	"github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -122,6 +123,9 @@ func newCheCluster() (cr *orgv1.CheCluster) {
 			Server: orgv1.CheClusterSpecServer{
 				UseInternalClusterSVCNames: true,
 			},
+			Auth: orgv1.CheClusterSpecAuth{
+				OpenShiftoAuth: util.GetBoolPointer(true),
+			},
 		},
 	}
 	return cr

pkg/apis/org/v1/che_types.go

@@ -410,7 +410,7 @@ type CheClusterSpecAuth struct {
 	// and have their workspaces created under personal OpenShift namespaces.
 	// WARNING: the `kubeadmin` user is NOT supported, and logging through it will NOT allow accessing the Che Dashboard.
 	// +optional
-	OpenShiftoAuth bool `json:"openShiftoAuth"`
+	OpenShiftoAuth *bool `json:"openShiftoAuth"`
 	// Name of the OpenShift `OAuthClient` resource used to setup identity federation on the OpenShift side. Auto-generated if left blank.
 	// See also the `OpenShiftoAuth` field.
 	// +optional

pkg/controller/che/che_controller.go

@@ -340,55 +340,21 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
 			host, err := getDefaultCheHost(deployContext)
 			if host == "" {
 				return reconcile.Result{RequeueAfter: 1 * time.Second}, err
-			} else {
-				deployContext.DefaultCheHost = host
 			}
+			deployContext.DefaultCheHost = host
 		}
 	}
 
-	if isOpenShift && instance.Spec.Auth.OpenShiftoAuth {
-		if isOpenShift4 {
-			oauthv1 := &oauthv1.OAuth{}
-			if err := r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oauthv1); err != nil {
-				getOAuthV1ErrMsg := failedUnableToGetOAuth + " Cause: " + err.Error()
-				logrus.Errorf(getOAuthV1ErrMsg)
-				if err := r.SetStatusDetails(instance, request, failedNoOpenshiftUserReason, getOAuthV1ErrMsg, ""); err != nil {
-					return reconcile.Result{}, err
-				}
-				return reconcile.Result{}, err
-			}
-			if len(oauthv1.Spec.IdentityProviders) < 1 {
-				logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4)
-				instance.Spec.Auth.OpenShiftoAuth = false
-				if err := r.UpdateCheCRSpec(instance, "OpenShiftoAuth", strconv.FormatBool(false)); err != nil {
-					return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err
-				}
-			}
-		} else {
-			users := &userv1.UserList{}
-			listOptions := &client.ListOptions{}
-			if err := r.nonCachedClient.List(context.TODO(), users, listOptions); err != nil {
-				getUsersErrMsg := failedUnableToGetOpenshiftUsers + " Cause: " + err.Error()
-				logrus.Errorf(getUsersErrMsg)
-				if err := r.SetStatusDetails(instance, request, failedNoOpenshiftUserReason, getUsersErrMsg, ""); err != nil {
-					return reconcile.Result{}, err
-				}
-				return reconcile.Result{}, err
-			}
-			if len(users.Items) < 1 {
-				logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3)
-				instance.Spec.Auth.OpenShiftoAuth = false
-				if err := r.UpdateCheCRSpec(instance, "OpenShiftoAuth", strconv.FormatBool(false)); err != nil {
-					return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err
-				}
-			}
+	if isOpenShift && instance.Spec.Auth.OpenShiftoAuth == nil {
+		if reconcileResult, err := r.autoEnableOAuth(instance, request, isOpenShift4); err != nil {
+			return reconcileResult, err
 		}
+	}
 
-		// delete oAuthClient before CR is deleted
-		if instance.Spec.Auth.OpenShiftoAuth {
-			if err := r.ReconcileFinalizer(instance); err != nil {
-				return reconcile.Result{}, err
-			}
+	// delete oAuthClient before CR is deleted
+	if instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth {
+		if err := r.ReconcileFinalizer(instance); err != nil {
+			return reconcile.Result{}, err
 		}
 
 	}
@@ -431,7 +397,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
 			// To use Openshift v4 OAuth, the OAuth endpoints are served from a namespace
 			// and NOT from the Openshift API Master URL (as in v3)
 			// So we also need the self-signed certificate to access them (same as the Che server)
-			(isOpenShift4 && instance.Spec.Auth.OpenShiftoAuth && !instance.Spec.Server.TlsSupport) {
+			(isOpenShift4 && instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth && !instance.Spec.Server.TlsSupport) {
 			if err := deploy.CreateTLSSecretFromEndpoint(deployContext, "", deploy.CheTLSSelfSignedCertificateSecretName); err != nil {
 				return reconcile.Result{}, err
 			}
@@ -447,7 +413,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
 			}
 		}
 
-		if instance.Spec.Auth.OpenShiftoAuth {
+		if instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth {
 			// create a secret with OpenShift API crt to be added to keystore that RH SSO will consume
 			baseURL, err := util.GetClusterPublicHostname(isOpenShift4)
 			if err != nil {
@@ -1123,3 +1089,50 @@ func isTrustedBundleConfigMap(mgr manager.Manager, obj handler.MapObject) (bool,
 		},
 	}
 }
+
+func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.Request, isOpenShift4 bool) (reconcile.Result, error) {
+	var message, reason string
+	if isOpenShift4 {
+		oauthv1 := &oauthv1.OAuth{}
+		if err := r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oauthv1); err != nil {
+			getOAuthV1ErrMsg := failedUnableToGetOAuth + " Cause: " + err.Error()
+			logrus.Errorf(getOAuthV1ErrMsg)
+			message = getOAuthV1ErrMsg
+			reason = failedNoOpenshiftUserReason
+			cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false)
+		} else {
+			cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(len(oauthv1.Spec.IdentityProviders) >= 1)
+			if !*cr.Spec.Auth.OpenShiftoAuth {
+				logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4)
+			}
+		}
+		// openshift 3
+	} else {
+		users := &userv1.UserList{}
+		listOptions := &client.ListOptions{}
+		if err := r.nonCachedClient.List(context.TODO(), users, listOptions); err != nil {
+			getUsersErrMsg := failedUnableToGetOpenshiftUsers + " Cause: " + err.Error()
+			logrus.Errorf(getUsersErrMsg)
+			message = getUsersErrMsg
+			reason = failedNoOpenshiftUserReason
+			cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false)
+		}
+
+		cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(len(users.Items) >= 1)
+		if !*cr.Spec.Auth.OpenShiftoAuth {
+			logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3)
+		}
+	}
+
+	if err := r.UpdateCheCRSpec(cr, "OpenShiftoAuth", strconv.FormatBool(*cr.Spec.Auth.OpenShiftoAuth)); err != nil {
+		return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err
+	}
+
+	if message != "" && reason != "" {
+		if err := r.SetStatusDetails(cr, request, message, reason, ""); err != nil {
+			return reconcile.Result{}, err
+		}
+	}
+
+	return reconcile.Result{}, nil
+}