Remove self-signed certificate option. Always propogate router certif…

…icate into trust store.

Signed-off-by: Mykola Morhun <[email protected]>

README.md

@@ -47,14 +47,11 @@ Make sure your current user has cluster-admin privileges.
 
 #### OpenShift
 
-When using self-signed certificates make sure you set `server.selfSignedCert` to true
-or create a secret called `self-signed-certificate` in a target namespace with ca.crt holding your OpenShift router crt body.
-When `server.selfSignedCert` the operator will create a test TLS route, GET it, extract certificate chain, convert to a secret `self-signed-certificate`,
-and Che/CRW server will automatically add it to Java trust store.
+TLS enabled and handled automatically by adding router certificate into trust store.
 
 #### K8S
 
-When enabling TLS, make sure you create a secret with crt and key, and let the Operator know about it in `k8s.tlsSecretName`
+TLS enabled by default. If secrets is missing, self-signed certificate will be generated. If one need to provide own certificate, just create a secret with `tls.crt` and `tls.key` keys in `data`(and `self-signed-certificate` secret in case of using one), and let the Operator know about the certificate for Che server in `k8s.tlsSecretName` field of Che custom resource (`che-tls` by default).
 
 ## How to Configure
 
@@ -164,4 +161,4 @@ TODO: add more scenarios
 
 
 
-   
+

deploy/crds/org_v1_che_cr.yaml

@@ -30,15 +30,12 @@ spec:
     # specifies a custom cluster role to user for the Che workspaces
     # Uses the default roles if left blank.
     cheWorkspaceClusterRole: ''
-    # when set to true the operator will attempt to get a secret in OpenShift router namespace
-    # to add it to Java trust store of Che server. Requires cluster-admin privileges for operator service account
-    selfSignedCert: false
     # Name of the config-map with public certificates to add to Java trust store of the Che server.
     serverTrustStoreConfigMapName: ''
     # If enabled then the certificate from `che-git-self-signed-cert` config map
     # will be propagated to the Che components and provide particular configuration for Git.
     gitSelfSignedCert: false
-    # TLS mode for Che. Make sure you either have public cert, or set selfSignedCert to true
+    # TLS mode for Che.
     tlsSupport: true
     # protocol+hostname of a proxy server. Automatically added as JAVA_OPTS and https(s)_proxy
     # to Che server and workspaces containers

deploy/crds/org_v1_che_crd.yaml

@@ -422,13 +422,9 @@ spec:
                     a proxy is required (see also the `proxyURL` `proxySecret` fields).
                   type: string
                 selfSignedCert:
-                  description: Enables the support of OpenShift clusters whose router
-                    uses self-signed certificates. When enabled, the operator retrieves
-                    the default self-signed certificate of OpenShift routes and adds
-                    it to the Java trust store of the Che server. This is usually
-                    required when activating the `tlsSupport` field on demo OpenShift
-                    clusters that have not been setup with a valid certificate for
-                    the routes. This is disabled by default.
+                  description: Obsolete. The value of this flag is ignored.
+                    Che operator automatically propogates router certificate to Che server
+                    and some other components.
                   type: boolean
                 serverMemoryLimit:
                   description: Overrides the memory limit used in the Che server deployment.
@@ -446,10 +442,8 @@ spec:
                     its CA cert to be able to request it. This is disabled by default.
                   type: string
                 tlsSupport:
-                  description: 'Instructs the operator to deploy Che in TLS mode,
+                  description: Instructs the operator to deploy Che in TLS mode,
                     ie with TLS routes or ingresses. This is disabled by default.
-                    WARNING: Enabling TLS might require enabling the `selfSignedCert`
-                    field also in some cases.'
                   type: boolean
                 workspaceNamespaceDefault:
                   description: 'Defines Kubernetes default namespace in which user''s

e2e/create.go

@@ -46,7 +46,7 @@ func createOperatorServiceAccountRole(operatorServiceAccountRole *rbac.Role) (er
 func createOperatorServiceAccountClusterRole(operatorServiceAccountClusterRole *rbac.ClusterRole) (err error) {
 
 	operatorServiceAccountClusterRole, err = client.clientset.RbacV1().ClusterRoles().Create(operatorServiceAccountClusterRole)
-	if err != nil && ! errors.IsAlreadyExists(err) {
+	if err != nil && !errors.IsAlreadyExists(err) {
 		logrus.Fatalf("Failed to create role %s: %s", operatorServiceAccountClusterRole.Name, err)
 		return err
 	}
@@ -87,18 +87,16 @@ func deployOperator(deployment *appsv1.Deployment) (err error) {
 
 }
 
-func newNamespace() (ns *corev1.Namespace){
+func newNamespace() (ns *corev1.Namespace) {
 
 	return &corev1.Namespace{
-
 		TypeMeta: metav1.TypeMeta{
-			Kind: "Namespace",
+			Kind:       "Namespace",
 			APIVersion: corev1.SchemeGroupVersion.Version,
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:namespace,
+			Name: namespace,
 		},
-
 	}
 }
 
@@ -121,10 +119,8 @@ func newCheCluster() (cr *orgv1.CheCluster) {
 		TypeMeta: metav1.TypeMeta{
 			Kind: kind,
 		},
-		Spec:orgv1.CheClusterSpec{
-			Server:orgv1.CheClusterSpecServer{
-				SelfSignedCert: true,
-			},
+		Spec: orgv1.CheClusterSpec{
+			Server: orgv1.CheClusterSpecServer{},
 		},
 	}
 	return cr

go.mod

@@ -65,6 +65,7 @@ replace (
 )
 
 require (
+	github.com/PuerkitoBio/purell v1.1.1 // indirect
 	github.com/docker/spdystream v0.0.0-00010101000000-000000000000 // indirect
 	github.com/elazarl/goproxy v0.0.0-20200426045556-49ad98f6dac1 // indirect
 	github.com/go-logr/logr v0.0.0-00010101000000-000000000000 // indirect
@@ -78,6 +79,7 @@ require (
 	github.com/onsi/gomega v1.10.0 // indirect
 	github.com/openshift/api v0.0.0-00010101000000-000000000000
 	github.com/operator-framework/operator-sdk v0.0.0-00010101000000-000000000000
+	github.com/pborman/uuid v1.2.0 // indirect
 	github.com/peterbourgon/diskv v0.0.0-00010101000000-000000000000 // indirect
 	github.com/prometheus/common v0.4.1
 	github.com/sirupsen/logrus v1.4.2

pkg/controller/che/che_controller.go

@@ -314,7 +314,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
 	}
 
 	// Handle Che TLS certificates on Kubernetes like infrastructures
-	if instance.Spec.Server.TlsSupport && instance.Spec.Server.SelfSignedCert && !isOpenShift {
+	if instance.Spec.Server.TlsSupport && !isOpenShift {
 		// Ensure TLS configuration is correct
 		if err := CheckAndUpdateTLSConfiguration(instance, clusterAPI); err != nil {
 			instance, _ = r.GetCR(request)
@@ -395,18 +395,12 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
 	cheFlavor := deploy.DefaultCheFlavor(instance)
 	cheDeploymentName := cheFlavor
 	if isOpenShift {
-		// create a secret with router tls cert when on OpenShift infra and router is configured with a self signed certificate
-		if instance.Spec.Server.SelfSignedCert ||
-			// 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) {
+		if !tests {
+			// Create a secret with router tls certificate. If routes cert id self-signed it is required to propogate it to Che server and some other components.
 			if err := r.CreateTLSSecret(instance, "", "self-signed-certificate", clusterAPI); err != nil {
 				return reconcile.Result{}, err
 			}
-		}
 
-		if !tests {
 			deployment := &appsv1.Deployment{}
 			err = r.client.Get(context.TODO(), types.NamespacedName{Name: cheDeploymentName, Namespace: instance.Namespace}, deployment)
 			if err != nil && instance.Status.CheClusterRunning != UnavailableStatus {

pkg/controller/che/tls-secrets.go

@@ -33,15 +33,16 @@ const (
 	CheTLSJobRoleName                     = "che-tls-job-role"
 	CheTLSJobRoleBindingName              = "che-tls-job-role-binding"
 	CheTLSJobName                         = "che-tls-job"
-	CheTlsJobComponentName                = "che-create-tls-secret-job"
+	CheTLSJobComponentName                = "che-create-tls-secret-job"
 	CheTLSSelfSignedCertificateSecretName = "self-signed-certificate"
 )
 
 // HandleCheTLSSecrets handles TLS secrets required for Che deployment
 func HandleCheTLSSecrets(checluster *orgv1.CheCluster, clusterAPI deploy.ClusterAPI) (reconcile.Result, error) {
 	cheTLSSecretName := checluster.Spec.K8s.TlsSecretName
 
-	// ===== Check Che TLS certificate ===== //
+	// ===== Check Che server TLS certificate ===== //
+
 	cheTLSSecret := &corev1.Secret{}
 	err := clusterAPI.Client.Get(context.TODO(), types.NamespacedName{Namespace: checluster.Namespace, Name: cheTLSSecretName}, cheTLSSecret)
 	if err != nil {
@@ -94,7 +95,7 @@ func HandleCheTLSSecrets(checluster *orgv1.CheCluster, clusterAPI deploy.Cluster
 			"CHE_SERVER_TLS_SECRET_NAME":     cheTLSSecretName,
 			"CHE_CA_CERTIFICATE_SECRET_NAME": CheTLSSelfSignedCertificateSecretName,
 		}
-		job, err := deploy.SyncJobToCluster(checluster, CheTLSJobName, CheTlsJobComponentName, cheTLSSecretsCreationJobImage, CheTLSJobServiceAccountName, jobEnvVars, clusterAPI)
+		job, err := deploy.SyncJobToCluster(checluster, CheTLSJobName, CheTLSJobComponentName, cheTLSSecretsCreationJobImage, CheTLSJobServiceAccountName, jobEnvVars, clusterAPI)
 		if err != nil {
 			logrus.Error(err)
 			return reconcile.Result{RequeueAfter: time.Second}, err
@@ -163,55 +164,41 @@ func HandleCheTLSSecrets(checluster *orgv1.CheCluster, clusterAPI deploy.Cluster
 			return reconcile.Result{RequeueAfter: time.Second}, err
 		}
 		// Che CA self-signed cetificate secret doesn't exist.
-		// Such situation could happen between reconcile loops, when CA cert is deleted.
-		// However the certificates should be created together,
-		// so it is mandatory to remove Che TLS secret now and recreate the pair.
-		cheTLSSecret = &corev1.Secret{}
-		err = clusterAPI.Client.Get(context.TODO(), types.NamespacedName{Namespace: checluster.Namespace, Name: cheTLSSecretName}, cheTLSSecret)
-		if err != nil { // No need to check for not found error as the secret already exists at this point
-			logrus.Errorf("Error getting Che TLS secert \"%s\": %v", cheTLSSecretName, err)
-			return reconcile.Result{RequeueAfter: time.Second}, err
-		}
-		if err = clusterAPI.Client.Delete(context.TODO(), cheTLSSecret); err != nil {
-			logrus.Errorf("Error deleting Che TLS secret \"%s\": %v", cheTLSSecretName, err)
-			return reconcile.Result{RequeueAfter: time.Second}, err
-		}
-		// Invalid secrets cleaned up, recreate them now
-		return reconcile.Result{RequeueAfter: time.Second}, err
-	}
-
-	// Che CA self-signed certificate secret exists, check for required data fields
-	if !isCheCASecretValid(cheTLSSelfSignedCertificateSecret) {
-		logrus.Infof("Che self-signed certificate secret \"%s\" is invalid. Recrating...", CheTLSSelfSignedCertificateSecretName)
-		// Che CA self-signed certificate secret is invalid, delete it
-		if err = clusterAPI.Client.Delete(context.TODO(), cheTLSSelfSignedCertificateSecret); err != nil {
-			logrus.Errorf("Error deleting Che self-signed certificate secret \"%s\": %v", CheTLSSelfSignedCertificateSecretName, err)
-			return reconcile.Result{RequeueAfter: time.Second}, err
-		}
-		// Also delete Che TLS as the certificates should be created together
-		// Here it is not mandatory to check Che TLS secret existence as it is handled above
-		if err = clusterAPI.Client.Delete(context.TODO(), cheTLSSecret); err != nil {
-			logrus.Errorf("Error deleting Che TLS secret \"%s\": %v", cheTLSSecretName, err)
-			return reconcile.Result{RequeueAfter: time.Second}, err
+		// This means that commonly trusted certificate is used.
+	} else {
+		// Che CA self-signed certificate secret exists, check for required data fields
+		if !isCheCASecretValid(cheTLSSelfSignedCertificateSecret) {
+			logrus.Infof("Che self-signed certificate secret \"%s\" is invalid. Recrating...", CheTLSSelfSignedCertificateSecretName)
+			// Che CA self-signed certificate secret is invalid, delete it
+			if err = clusterAPI.Client.Delete(context.TODO(), cheTLSSelfSignedCertificateSecret); err != nil {
+				logrus.Errorf("Error deleting Che self-signed certificate secret \"%s\": %v", CheTLSSelfSignedCertificateSecretName, err)
+				return reconcile.Result{RequeueAfter: time.Second}, err
+			}
+			// Also delete Che TLS as the certificates should be created together
+			// Here it is not mandatory to check Che TLS secret existence as it is handled above
+			if err = clusterAPI.Client.Delete(context.TODO(), cheTLSSecret); err != nil {
+				logrus.Errorf("Error deleting Che TLS secret \"%s\": %v", cheTLSSecretName, err)
+				return reconcile.Result{RequeueAfter: time.Second}, err
+			}
+			// Regenerate Che TLS certicates and recreate secrets
+			return reconcile.Result{RequeueAfter: time.Second}, nil
 		}
-		// Regenerate Che TLS certicates and recreate secrets
-		return reconcile.Result{RequeueAfter: time.Second}, nil
-	}
 
-	// Check owner reference
-	if cheTLSSelfSignedCertificateSecret.ObjectMeta.OwnerReferences == nil {
-		// Set owner Che cluster as Che TLS secret owner
-		if err := controllerutil.SetControllerReference(checluster, cheTLSSelfSignedCertificateSecret, clusterAPI.Scheme); err != nil {
-			logrus.Errorf("Failed to set owner for Che self-signed certificate secret \"%s\". Error: %s", CheTLSSelfSignedCertificateSecretName, err)
-			return reconcile.Result{RequeueAfter: time.Second}, err
-		}
-		if err := clusterAPI.Client.Update(context.TODO(), cheTLSSelfSignedCertificateSecret); err != nil {
-			logrus.Errorf("Failed to update owner for Che self-signed certificate secret \"%s\". Error: %s", CheTLSSelfSignedCertificateSecretName, err)
-			return reconcile.Result{RequeueAfter: time.Second}, err
+		// Check owner reference
+		if cheTLSSelfSignedCertificateSecret.ObjectMeta.OwnerReferences == nil {
+			// Set owner Che cluster as Che TLS secret owner
+			if err := controllerutil.SetControllerReference(checluster, cheTLSSelfSignedCertificateSecret, clusterAPI.Scheme); err != nil {
+				logrus.Errorf("Failed to set owner for Che self-signed certificate secret \"%s\". Error: %s", CheTLSSelfSignedCertificateSecretName, err)
+				return reconcile.Result{RequeueAfter: time.Second}, err
+			}
+			if err := clusterAPI.Client.Update(context.TODO(), cheTLSSelfSignedCertificateSecret); err != nil {
+				logrus.Errorf("Failed to update owner for Che self-signed certificate secret \"%s\". Error: %s", CheTLSSelfSignedCertificateSecretName, err)
+				return reconcile.Result{RequeueAfter: time.Second}, err
+			}
 		}
 	}
 
-	// Both secrets are ok, go further in reconcile loop
+	// TLS configuration is ok, go further in reconcile loop
 	return reconcile.Result{}, nil
 }
 

pkg/deploy/deployment_che.go

@@ -54,10 +54,6 @@ func getSpecCheDeployment(checluster *orgv1.CheCluster, cmResourceVersion string
 	labels := GetLabels(checluster, cheFlavor)
 	optionalEnv := true
 	memRequest := util.GetValue(checluster.Spec.Server.ServerMemoryRequest, DefaultServerMemoryRequest)
-	selfSignedCertEnv := corev1.EnvVar{
-		Name:  "CHE_SELF__SIGNED__CERT",
-		Value: "",
-	}
 	customPublicCertsVolumeSource := corev1.VolumeSource{}
 	if checluster.Spec.Server.ServerTrustStoreConfigMapName != "" {
 		customPublicCertsVolumeSource = corev1.VolumeSource{
@@ -84,19 +80,19 @@ func getSpecCheDeployment(checluster *orgv1.CheCluster, cmResourceVersion string
 		Name:  "CHE_GIT_SELF__SIGNED__CERT__HOST",
 		Value: "",
 	}
-	if checluster.Spec.Server.SelfSignedCert {
-		selfSignedCertEnv = corev1.EnvVar{
-			Name: "CHE_SELF__SIGNED__CERT",
-			ValueFrom: &corev1.EnvVarSource{
-				SecretKeyRef: &corev1.SecretKeySelector{
-					Key: "ca.crt",
-					LocalObjectReference: corev1.LocalObjectReference{
-						Name: "self-signed-certificate",
-					},
-					Optional: &optionalEnv,
+	selfSignedCertEnv := corev1.EnvVar{
+		// Propagete router certificate anyway. This allows to avoid reqesting flag from user.
+		// TODO rename to CHE_CERT
+		Name: "CHE_SELF__SIGNED__CERT",
+		ValueFrom: &corev1.EnvVarSource{
+			SecretKeyRef: &corev1.SecretKeySelector{
+				Key: "ca.crt",
+				LocalObjectReference: corev1.LocalObjectReference{
+					Name: "self-signed-certificate",
 				},
+				Optional: &optionalEnv,
 			},
-		}
+		},
 	}
 	if checluster.Spec.Server.GitSelfSignedCert {
 		gitSelfSignedCertEnv = corev1.EnvVar{