Improve oauth handling.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

deploy/crds/org_v1_che_cr.yaml

@@ -125,9 +125,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

pkg/apis/org/v1/che_types.go

@@ -406,7 +406,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

@@ -319,55 +319,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
 		}
 	}
 
@@ -409,7 +375,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
 			}
@@ -425,7 +391,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 {
@@ -1101,3 +1067,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
+}

pkg/controller/che/che_controller_test.go

@@ -162,7 +162,7 @@ func TestCheController(t *testing.T) {
 	}
 
 	// update CR and make sure Che configmap has been updated
-	cheCR.Spec.Auth.OpenShiftoAuth = true
+	cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(true)
 	if err := cl.Update(context.TODO(), cheCR); err != nil {
 		t.Error("Failed to update CheCluster custom resource")
 	}

pkg/controller/che/update.go

@@ -46,7 +46,7 @@ func (r *ReconcileChe) UpdateCheCRSpec(instance *orgv1.CheCluster, updatedField
 }
 
 func (r *ReconcileChe) ReconcileIdentityProvider(instance *orgv1.CheCluster, isOpenShift4 bool) (deleted bool, err error) {
-	if instance.Spec.Auth.OpenShiftoAuth == false && instance.Status.OpenShiftoAuthProvisioned == true {
+	if instance.Spec.Auth.OpenShiftoAuth != nil && !*instance.Spec.Auth.OpenShiftoAuth && instance.Status.OpenShiftoAuthProvisioned == true {
 		keycloakDeployment := &appsv1.Deployment{}
 		if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "keycloak", Namespace: instance.Namespace}, keycloakDeployment); err != nil {
 			logrus.Errorf("Deployment %s not found: %s", keycloakDeployment.Name, err)

pkg/deploy/identity-provider/identity_provider.go

@@ -127,12 +127,9 @@ func SyncIdentityProviderToCluster(deployContext *deploy.DeployContext) (bool, e
 		}
 	}
 
-	if isOpenShift {
-		doInstallOpenShiftoAuthProvider := instance.Spec.Auth.OpenShiftoAuth
-		if doInstallOpenShiftoAuthProvider {
-			if err := SyncIdentityProviderItems(deployContext, cheFlavor); err != nil {
-				return false, err
-			}
+	if isOpenShift && instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth {
+		if err := SyncIdentityProviderItems(deployContext, cheFlavor); err != nil {
+			return false, err
 		}
 	}
 

pkg/deploy/server/che_configmap.go

@@ -117,7 +117,7 @@ func GetCheConfigMapData(deployContext *deploy.DeployContext) (cheEnv map[string
 	openShiftIdentityProviderId := "NULL"
 	openshiftOAuth := deployContext.CheCluster.Spec.Auth.OpenShiftoAuth
 	defaultTargetNamespaceDefault := deployContext.CheCluster.Namespace // By default Che SA has right in the namespace where Che in installed ...
-	if openshiftOAuth && isOpenShift {
+	if openshiftOAuth != nil && *openshiftOAuth && isOpenShift {
 		// ... But if the workspace is created under the openshift identity of the end-user,
 		// Then we'll have rights to create any new namespace
 		defaultTargetNamespaceDefault = "<username>-" + cheFlavor

pkg/deploy/server/che_configmap_test.go

@@ -28,7 +28,7 @@ func TestNewCheConfigMap(t *testing.T) {
 	cr := &orgv1.CheCluster{}
 	cr.Spec.Server.CheHost = "myhostname.com"
 	cr.Spec.Server.TlsSupport = true
-	cr.Spec.Auth.OpenShiftoAuth = true
+	*cr.Spec.Auth.OpenShiftoAuth = true
 	deployContext := &deploy.DeployContext{
 		CheCluster: cr,
 		Proxy:      &deploy.Proxy{},
@@ -58,7 +58,7 @@ func TestConfigMapOverride(t *testing.T) {
 	cr.Spec.Server.CustomCheProperties = map[string]string{
 		"CHE_WORKSPACE_NO_PROXY": "myproxy.myhostname.com",
 	}
-	cr.Spec.Auth.OpenShiftoAuth = true
+	*cr.Spec.Auth.OpenShiftoAuth = true
 	deployContext := &deploy.DeployContext{
 		CheCluster: cr,
 		Proxy:      &deploy.Proxy{},

pkg/util/util.go

@@ -330,3 +330,10 @@ func GetArchitectureDependentEnv(env string) string {
 
 	return env
 }
+
+// GetBoolPointer returns `bool` pointer to value in the memory. 
+// Unfortunately golang hasn't got syntax to create `bool` pointer.
+func GetBoolPointer(value bool) *bool {
+	variable := value
+	return &variable
+}