feat: Deprecate 'self-signed-cert' flag and implement autodetection f…

…or it (#734) * Deprecate and make autodetection for 'self-signed-cert' flag. Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
che-incubator · Jun 16, 2020 · 7e9b365 · 7e9b365
1 parent 07f741a
commit 7e9b365
Show file tree

Hide file tree

Showing 13 changed files with 82 additions and 84 deletions.
diff --git a/README.md b/README.md
@@ -338,12 +338,11 @@ OPTIONS
   -s, --tls
       Enable TLS encryption.
                            Note, this option is turned on by default.
-                           For Kubernetes infrastructure, it is required to provide own certificate: 'che-tls' secret with 
-      TLS certificate must be pre-created in the configured namespace.
-                           The only exception is Helm installer. In that case the secret will be generated automatically.
+                           To provide own certificate for Kubernetes infrastructure, 'che-tls' secret with TLS certificate 
+      must be pre-created in the configured namespace.
+                           In case of providing own self-signed certificate 'self-signed-certificate' secret should be 
+      also created.
                            For OpenShift, router will use default cluster certificates.
-                           If the certificate is self-signed, '--self-signed-cert' option should be provided, otherwise 
-      Che won't be able to start.
                            Please see docs for more details: 
       https://www.eclipse.org/che/docs/che-7/installing-che-in-tls-mode-with-self-signed-certificates/
 
@@ -414,10 +413,7 @@ OPTIONS
       persistent volume storage class name to use to store Eclipse Che postgres database
 
   --self-signed-cert
-      Authorize usage of self signed certificates for encryption.
-                           This is the flag for Eclipse Che to propagate the certificate to components, so they will trust 
-      it.
-                           Note that `che-tls` secret with CA certificate must be created in the configured namespace.
+      Deprecated. The flag is ignored. Self signed certificates usage is autodetected now.
 
   --skip-cluster-availability-check
       Skip cluster availability check. The check is a simple request to ensure the cluster is reachable.

diff --git a/src/api/che.ts b/src/api/che.ts
@@ -110,13 +110,19 @@ export class CheHelper {
     }
   }
 
+  async isSelfSignedCertificateSecretExist(namespace: string): Promise<boolean> {
+    const selfSignedCertSecret = await this.kube.getSecret(CHE_ROOT_CA_SECRET_NAME, namespace)
+    return !!selfSignedCertSecret
+  }
+
   /**
-   * Gets self-signed Che CA certificate from 'self-signed-certificate' secret. The secret should exist.
+   * Gets self-signed Che CA certificate from 'self-signed-certificate' secret.
+   * If secret doesn't exist, undefined is returned.
    */
-  async retrieveCheCaCert(cheNamespace: string): Promise<string> {
+  async retrieveCheCaCert(cheNamespace: string): Promise<string | undefined> {
     const cheCaSecret = await this.kube.getSecret(CHE_ROOT_CA_SECRET_NAME, cheNamespace)
     if (!cheCaSecret) {
-      throw new Error('Che CA self-signed certificate not found. Are you using self-signed certificate?')
+      return
     }
 
     if (cheCaSecret.data && cheCaSecret.data['ca.crt']) {

diff --git a/src/api/kube.ts b/src/api/kube.ts
@@ -1146,7 +1146,6 @@ export class KubeHelper {
           yamlCr.spec.k8s.tlsSecretName = 'che-tls'
         }
       }
-      yamlCr.spec.server.selfSignedCert = flags['self-signed-cert']
       if (flags.domain) {
         yamlCr.spec.k8s.ingressDomain = flags.domain
       }
@@ -1794,6 +1793,26 @@ export class KubeHelper {
     }
   }
 
+  /**
+   * Creates a secret with given name and data.
+   * Data should not be base64 encoded.
+   */
+  async createSecret(name: string, data: {[key: string]: string}, namespace: string): Promise<V1Secret | undefined> {
+    const k8sCoreApi = KubeHelper.KUBE_CONFIG.makeApiClient(CoreV1Api)
+
+    const secret = new V1Secret()
+    secret.metadata = new V1ObjectMeta()
+    secret.metadata.name = name
+    secret.metadata.namespace = namespace
+    secret.stringData = data
+
+    try {
+      return (await k8sCoreApi.createNamespacedSecret(namespace, secret)).body
+    } catch {
+      return
+    }
+  }
+
   /**
    * Awaits secret to be present and contain non-empty data fields specified in dataKeys parameter.
    */

diff --git a/src/commands/cacert/export.ts b/src/commands/cacert/export.ts
@@ -53,8 +53,12 @@ export default class Export extends Command {
     try {
       await tasks.run(ctx)
       const cheCaCert = await cheHelper.retrieveCheCaCert(flags.chenamespace)
-      const targetFile = await cheHelper.saveCheCaCert(cheCaCert, this.getTargetFile(flags.destination))
-      this.log(`Eclipse Che self-signed CA certificate is exported to ${targetFile}`)
+      if (cheCaCert) {
+        const targetFile = await cheHelper.saveCheCaCert(cheCaCert, this.getTargetFile(flags.destination))
+        this.log(`Eclipse Che self-signed CA certificate is exported to ${targetFile}`)
+      } else {
+        this.log('Seems commonly trusted certificate is used.')
+      }
     } catch (error) {
       this.error(error)
     }

diff --git a/src/commands/server/start.ts b/src/commands/server/start.ts
@@ -80,16 +80,13 @@ export default class Start extends Command {
       char: 's',
       description: `Enable TLS encryption.
                     Note, this option is turned on by default.
-                    For Kubernetes infrastructure, it is required to provide own certificate: 'che-tls' secret with TLS certificate must be pre-created in the configured namespace.
-                    The only exception is Helm installer. In that case the secret will be generated automatically.
+                    To provide own certificate for Kubernetes infrastructure, 'che-tls' secret with TLS certificate must be pre-created in the configured namespace.
+                    In case of providing own self-signed certificate 'self-signed-certificate' secret should be also created.
                     For OpenShift, router will use default cluster certificates.
-                    If the certificate is self-signed, '--self-signed-cert' option should be provided, otherwise Che won't be able to start.
                     Please see docs for more details: ${DOCS_LINK_INSTALL_TLS_WITH_SELF_SIGNED_CERT}`
     }),
     'self-signed-cert': flags.boolean({
-      description: `Authorize usage of self signed certificates for encryption.
-                    This is the flag for Eclipse Che to propagate the certificate to components, so they will trust it.
-                    Note that \`che-tls\` secret with CA certificate must be created in the configured namespace.`,
+      description: 'Deprecated. The flag is ignored. Self signed certificates usage is autodetected now.',
       default: false
     }),
     platform: string({
@@ -253,7 +250,6 @@ export default class Start extends Command {
       flags['devfile-registry-url'] && ignoredFlags.push('--devfile-registry-url')
       flags['postgres-pvc-storage-class-name'] && ignoredFlags.push('--postgres-pvc-storage-class-name')
       flags['workspace-pvc-storage-class-name'] && ignoredFlags.push('--workspace-pvc-storage-class-name')
-      flags['self-signed-cert'] && ignoredFlags.push('--self-signed-cert')
       flags['os-oauth'] && ignoredFlags.push('--os-oauth')
       flags.tls && ignoredFlags.push('--tls')
       flags.cheimage && ignoredFlags.push('--cheimage')
@@ -327,6 +323,10 @@ export default class Start extends Command {
     // Holds messages which should be printed at the end of chectl log
     ctx.highlightedMessages = [] as string[]
 
+    if (flags['self-signed-cert']) {
+      this.warn('"self-signed-cert" flag is deprecated and has no effect. Autodetection is used instead.')
+    }
+
     const cheTasks = new CheTasks(flags)
     const platformTasks = new PlatformTasks()
     const installerTasks = new InstallerTasks()

diff --git a/src/tasks/component-installers/cert-manager.ts b/src/tasks/component-installers/cert-manager.ts
@@ -13,7 +13,7 @@ import * as path from 'path'
 
 import { CheHelper } from '../../api/che'
 import { KubeHelper } from '../../api/kube'
-import { CA_CERT_GENERATION_JOB_IMAGE, CERT_MANAGER_NAMESPACE_NAME, CHE_TLS_SECRET_NAME } from '../../constants'
+import { CA_CERT_GENERATION_JOB_IMAGE, CERT_MANAGER_NAMESPACE_NAME, CHE_ROOT_CA_SECRET_NAME, CHE_TLS_SECRET_NAME } from '../../constants'
 import { base64Decode } from '../../util'
 import { getMessageImportCaCertIntoBrowser } from '../installers/common-tasks'
 
@@ -171,6 +171,9 @@ export class CertManagerTasks {
             const cheCaCrt = base64Decode(cheSecret.data['ca.crt'])
             const cheCaCertPath = await this.cheHelper.saveCheCaCert(cheCaCrt)
 
+            // We need to put self-signed CA certificate seprately into CHE_ROOT_CA_SECRET_NAME secret
+            await this.kubeHelper.createSecret(CHE_ROOT_CA_SECRET_NAME, { 'ca.crt': cheCaCrt }, flags.chenamespace)
+
             ctx.highlightedMessages.push(getMessageImportCaCertIntoBrowser(cheCaCertPath))
             task.title = `${task.title}... is exported to ${cheCaCertPath}`
           } else {

diff --git a/src/tasks/installers/common-tasks.ts b/src/tasks/installers/common-tasks.ts
@@ -9,7 +9,6 @@
  **********************************************************************/
 
 import ansi = require('ansi-colors')
-import { cli } from 'cli-ux'
 import * as execa from 'execa'
 import { copy, mkdirp, remove } from 'fs-extra'
 import * as Listr from 'listr'
@@ -99,52 +98,23 @@ export function createEclipseCheCluster(flags: any, kube: KubeHelper): Listr.Lis
   }
 }
 
-export function checkTlsCertificate(flags: any): Listr.ListrTask {
-  return {
-    title: 'Checking certificate',
-    // It makes sense to check whether self-signed certificate is used only if TLS mode is on
-    enabled: () => flags.tls,
-    // If the flag is set no need to check if it is required
-    skip: () => flags['self-signed-cert'],
-    task: async (_: any, task: any) => {
-      const warningMessage = 'Self-signed certificate is used, so "--self-signed-cert" option is required. Added automatically.'
-
-      const platform = flags.platform
-      if (platform === 'minikube' || platform === 'crc' || platform === 'minishift') {
-        // There is no way to use real certificate on listed above platforms
-        cli.warn(warningMessage)
-        flags['self-signed-cert'] = true
-        task.title = `${task.title}... self-signed`
-        return
-      }
-
-      if (flags.domain && (flags.domain.endsWith('nip.io') || flags.domain.endsWith('xip.io'))) {
-        // It is not possible to use real certificate with *.nip.io and similar services
-        cli.warn(warningMessage)
-        flags['self-signed-cert'] = true
-        task.title = `${task.title}... self-signed`
-        return
-      }
-
-      // TODO check the secret certificate if it is commonly trusted.
-      cli.info('TLS mode is turned on, however we failed to determine whether self-signed certificate is used. \n\
-               Please rerun chectl with "--self-signed-cert" option if it is the case, otherwise Eclipse Che will fail to start.')
-    }
-  }
-}
-
 export function retrieveCheCaCertificateTask(flags: any): Listr.ListrTask {
   return {
     title: 'Retrieving Che self-signed CA certificate',
     // It makes sense to retrieve CA certificate only if self-signed certificate is used.
-    enabled: () => flags.tls && flags['self-signed-cert'] && flags.installer !== 'helm',
+    enabled: () => flags.tls && flags.installer !== 'helm',
     task: async (ctx: any, task: any) => {
       const che = new CheHelper(flags)
       const cheCaCert = await che.retrieveCheCaCert(flags.chenamespace)
-      const targetFile = await che.saveCheCaCert(cheCaCert)
+      if (cheCaCert) {
+        const targetFile = await che.saveCheCaCert(cheCaCert)
+
+        task.title = `${task.title }... is exported to ${targetFile}`
+        ctx.highlightedMessages.push(getMessageImportCaCertIntoBrowser(targetFile))
+      } else {
+        task.title = `${task.title }... commonly trusted certificate is used.`
+      }
 
-      task.title = `${task.title }... is exported to ${targetFile}`
-      ctx.highlightedMessages.push(getMessageImportCaCertIntoBrowser(targetFile))
     }
   }
 }

diff --git a/src/tasks/installers/helm.ts b/src/tasks/installers/helm.ts
@@ -19,7 +19,7 @@ import * as path from 'path'
 import { CheHelper } from '../../api/che'
 import { KubeHelper } from '../../api/kube'
 import { VersionHelper } from '../../api/version'
-import { CHE_TLS_SECRET_NAME } from '../../constants'
+import { CHE_ROOT_CA_SECRET_NAME, CHE_TLS_SECRET_NAME } from '../../constants'
 import { CertManagerTasks } from '../../tasks/component-installers/cert-manager'
 import { generatePassword, isStableVersion } from '../../util'
 
@@ -78,25 +78,28 @@ export class HelmTasks {
       {
         title: 'Check Eclipse Che TLS certificate',
         task: async (ctx: any, task: any) => {
-          const cheTlsSecret = await this.kubeHelper.getSecret(CHE_TLS_SECRET_NAME, flags.chenamespace)
-
           const fixErrorMessage = 'Helm installer generates secrets automatically. To fix the problem delete existed secrets in dedicated for Eclispe Che namespace and rerun the command.'
-          if (cheTlsSecret && cheTlsSecret.data) {
-            if (!cheTlsSecret.data['tls.crt'] || !cheTlsSecret.data['tls.key']) {
-              throw new Error('"che-tls" secret is found but it is invalid. The valid self-signed certificate should contain "tls.crt" and "tls.key" entries. ' + fixErrorMessage)
+
+          const cheTlsSecret = await this.kubeHelper.getSecret(CHE_TLS_SECRET_NAME, flags.chenamespace)
+          if (cheTlsSecret) {
+            if (!cheTlsSecret.data || !cheTlsSecret.data['tls.crt'] || !cheTlsSecret.data['tls.key']) {
+              throw new Error('"che-tls" secret is found but it is invalid. The valid secret should contain "tls.crt" and "tls.key" entries. ' + fixErrorMessage)
             }
-            if (flags['self-signed-cert'] && !cheTlsSecret.data['ca.crt']) {
-              throw new Error(`"ca.crt" should be present in ${CHE_TLS_SECRET_NAME} secret in case of using self-signed certificate with helm installer. ${fixErrorMessage}`)
+            const selfSignedCertSecret = await this.kubeHelper.getSecret(CHE_ROOT_CA_SECRET_NAME, flags.chenamespace)
+            if (selfSignedCertSecret && (!selfSignedCertSecret.data || !selfSignedCertSecret.data['ca.crt'])) {
+              throw new Error(`"ca.crt" should be present in ${CHE_ROOT_CA_SECRET_NAME} secret in case of using self-signed certificate with helm installer. ${fixErrorMessage}`)
             }
 
             ctx.cheCertificateExists = true
 
-            task.title = `${task.title}...self-signed certificate secret found`
+            if (selfSignedCertSecret) {
+              task.title = `${task.title}...self-signed TLS certificate secret found`
+            } else {
+              task.title = `${task.title}...TLS certificate secret found`
+            }
           } else {
             // TLS certificate for Eclipse Che hasn't been added into the cluster manually, so we need to take care about it automatically
             ctx.cheCertificateExists = false
-            // Set self-signed certificate flag to true as we are going to generate one
-            flags['self-signed-cert'] = true
 
             task.title = `${task.title}...going to generate self-signed one`
 
@@ -312,9 +315,8 @@ error: E_COMMAND_FAILED`)
       tlsFlag = `-f ${destDir}values/tls.yaml`
     }
 
-    if (flags['self-signed-cert']) {
-      setOptions.push('--set global.tls.useSelfSignedCerts=true')
-    }
+    const selfSignedCertSecretExists = !! await this.kubeHelper.getSecret(CHE_TLS_SECRET_NAME, flags.chenamespace)
+    setOptions.push(`--set global.tls.useSelfSignedCerts=${selfSignedCertSecretExists}`)
 
     if (flags['plugin-registry-url']) {
       setOptions.push(`--set che.workspace.pluginRegistryUrl=${flags['plugin-registry-url']} --set chePluginRegistry.deploy=false`)

diff --git a/src/tasks/installers/olm.ts b/src/tasks/installers/olm.ts
@@ -17,7 +17,7 @@ import { CatalogSource, Subscription } from '../../api/typings/olm'
 import { CUSTOM_CATALOG_SOURCE_NAME, CVS_PREFIX, DEFAULT_CHE_IMAGE, DEFAULT_CHE_OLM_PACKAGE_NAME, DEFAULT_OLM_KUBERNETES_NAMESPACE, DEFAULT_OPENSHIFT_MARKET_PLACE_NAMESPACE, KUBERNETES_OLM_CATALOG, OLM_STABLE_CHANNEL_NAME, OPENSHIFT_OLM_CATALOG, OPERATOR_GROUP_NAME, SUBSCRIPTION_NAME } from '../../constants'
 import { isKubernetesPlatformFamily } from '../../util'
 
-import { checkTlsCertificate, copyOperatorResources, createEclipseCheCluster, createNamespaceTask } from './common-tasks'
+import { copyOperatorResources, createEclipseCheCluster, createNamespaceTask } from './common-tasks'
 
 export class OLMTasks {
   /**
@@ -32,7 +32,6 @@ export class OLMTasks {
       this.isOlmPreInstalledTask(command, kube),
       copyOperatorResources(flags, command.config.cacheDir),
       createNamespaceTask(flags),
-      checkTlsCertificate(flags),
       {
         title: 'Create operator group',
         task: async (_ctx: any, task: any) => {

diff --git a/src/tasks/installers/operator.ts b/src/tasks/installers/operator.ts
@@ -18,7 +18,7 @@ import { KubeHelper } from '../../api/kube'
 import { CHE_CLUSTER_CRD } from '../../constants'
 import { isStableVersion } from '../../util'
 
-import { checkTlsCertificate, copyOperatorResources, createEclipseCheCluster, createNamespaceTask } from './common-tasks'
+import { copyOperatorResources, createEclipseCheCluster, createNamespaceTask } from './common-tasks'
 
 export class OperatorTasks {
   operatorServiceAccount = 'che-operator'
@@ -40,7 +40,6 @@ export class OperatorTasks {
     return new Listr([
       copyOperatorResources(flags, command.config.cacheDir),
       createNamespaceTask(flags),
-      checkTlsCertificate(flags),
       {
         title: `Create ServiceAccount ${this.operatorServiceAccount} in namespace ${flags.chenamespace}`,
         task: async (ctx: any, task: any) => {

diff --git a/test/e2e/minikube.test.ts b/test/e2e/minikube.test.ts
@@ -22,7 +22,7 @@ describe('Eclipse Che deploy test suite', () => {
     test
       .stdout({ print: true })
       .stderr({ print: true })
-      .command(['server:start', '--platform=minikube', '--che-operator-cr-patch-yaml=test/e2e/util/cr-test.yaml', '--tls', '--self-signed-cert', '--installer=operator', '--skip-cluster-availability-check'])
+      .command(['server:start', '--platform=minikube', '--che-operator-cr-patch-yaml=test/e2e/util/cr-test.yaml', '--tls', '--installer=operator', '--skip-cluster-availability-check'])
       .exit(0)
       .it('uses minikube as platform, operator as installer and auth is enabled')
     test

diff --git a/test/e2e/minishift.test.ts b/test/e2e/minishift.test.ts
@@ -23,7 +23,7 @@ describe('Eclipse Che deploy test suite', () => {
     test
       .stdout({ print: true })
       .stderr({ print: true })
-      .command(['server:start', '--platform=minishift', '--che-operator-cr-patch-yaml=test/e2e/util/cr-test.yaml', '--tls', '--self-signed-cert', '--installer=operator'])
+      .command(['server:start', '--platform=minishift', '--che-operator-cr-patch-yaml=test/e2e/util/cr-test.yaml', '--tls', '--installer=operator'])
       .exit(0)
       .it('uses minishift as platform, operator as installer and auth is enabled')
     test

diff --git a/yarn.lock b/yarn.lock
@@ -1525,11 +1525,11 @@ ecc-jsbn@~0.1.1:
 
 "eclipse-che-operator@git://github.com/eclipse/che-operator#master":
   version "0.0.0"
-  resolved "git://github.com/eclipse/che-operator#e0663e1f6b5c84fb84043191d32b0a9da9c0b9fe"
+  resolved "git://github.com/eclipse/che-operator#912fd52fc438a1917130e1f1d7c74e64c232762a"
 
 "eclipse-che@git://github.com/eclipse/che#master":
   version "0.0.0"
-  resolved "git://github.com/eclipse/che#a46766cec2c7365d03d499d6bdbc7a663658bbe3"
+  resolved "git://github.com/eclipse/che#307a780c64745627d046068fb57fbaab980ed468"
 
 editorconfig@^0.15.0:
   version "0.15.3"