From 5e3caa1c965121872484d7a0e6314ae3b27aa017 Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Wed, 27 May 2020 16:20:27 +0300 Subject: [PATCH] Remove self-signed certificate option. Always propogate router certificate into trust store. Signed-off-by: Mykola Morhun --- README.md | 9 ++-- deploy/crds/org_v1_che_cr.yaml | 5 +- deploy/crds/org_v1_che_crd.yaml | 14 ++--- e2e/create.go | 16 +++--- go.mod | 2 + pkg/controller/che/che_controller.go | 12 ++--- pkg/controller/che/tls-secrets.go | 81 ++++++++++++---------------- pkg/deploy/deployment_che.go | 26 ++++----- 8 files changed, 64 insertions(+), 101 deletions(-) diff --git a/README.md b/README.md index 4f8536082f..a6fde25d07 100644 --- a/README.md +++ b/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 - + diff --git a/deploy/crds/org_v1_che_cr.yaml b/deploy/crds/org_v1_che_cr.yaml index 96b9a0ff8f..76076f8b8d 100644 --- a/deploy/crds/org_v1_che_cr.yaml +++ b/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 diff --git a/deploy/crds/org_v1_che_crd.yaml b/deploy/crds/org_v1_che_crd.yaml index 5f06a7c026..13c09198c4 100644 --- a/deploy/crds/org_v1_che_crd.yaml +++ b/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 diff --git a/e2e/create.go b/e2e/create.go index 8bf2e83f22..6be05e603f 100644 --- a/e2e/create.go +++ b/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 diff --git a/go.mod b/go.mod index 5758bf9689..fe632c9c34 100644 --- a/go.mod +++ b/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 diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 1e4f7d5653..13bf4c2a11 100644 --- a/pkg/controller/che/che_controller.go +++ b/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 { diff --git a/pkg/controller/che/tls-secrets.go b/pkg/controller/che/tls-secrets.go index d4f3ce6b9f..2bdab621e5 100644 --- a/pkg/controller/che/tls-secrets.go +++ b/pkg/controller/che/tls-secrets.go @@ -33,7 +33,7 @@ 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" ) @@ -41,7 +41,8 @@ const ( 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 } diff --git a/pkg/deploy/deployment_che.go b/pkg/deploy/deployment_che.go index b7f96efdf3..04dd681272 100644 --- a/pkg/deploy/deployment_che.go +++ b/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{