Fixes according to review

Signed-off-by: Mykola Morhun <mmorhun@redhat.com>

README.md

@@ -231,7 +231,7 @@ OPTIONS
       show CLI help
 
   -i, --cheimage=cheimage
-      [default: quay.io/eclipse/che-server:nightly] Che server container image
+      [default: eclipse/che-server:nightly] Che server container image
 
   -m, --multiuser
       Starts che in multi-user mode
@@ -243,24 +243,25 @@ OPTIONS
       (required) [default: 40000] Che server bootstrap timeout (in milliseconds)
 
   -p, --platform=minikube|minishift|k8s|openshift|microk8s|docker-desktop|crc
-      Type of Kubernetes platform. Valid values are "minikube", "minishift", "k8s (for kubernetes)", "openshift", "crc
+      Type of Kubernetes platform. Valid values are "minikube", "minishift", "k8s (for kubernetes)", "openshift", "crc 
       (for CodeReady Containers)", "microk8s".
 
   -s, --tls
       Enable TLS encryption.
-      Note, that this option is turned on by default for kubernetes infrastructure.
-      If it is needed to provide own certificate, 'che-tls' secret with TLS certificate must be created in the configured namespace. Otherwise, it will be automatically generated.
-      For OpenShift, router will use default cluster certificates.
+                           Note, that this option is turned on by default for kubernetes infrastructure.
+                           If it is needed to provide own certificate, 'che-tls' secret with TLS certificate must be 
+      created in the configured namespace. Otherwise, it will be automatically generated.
+                           For OpenShift, router will use default cluster certificates.
 
   -t, --templates=templates
       [default: templates] Path to the templates folder
 
   --che-operator-cr-yaml=che-operator-cr-yaml
-      Path to a yaml file that defines a CheCluster used by the operator. This parameter is used only when the installer
+      Path to a yaml file that defines a CheCluster used by the operator. This parameter is used only when the installer 
       is the operator.
 
   --che-operator-image=che-operator-image
-      [default: quay.io/eclipse/che-operator:nightly] Container image of the operator. This parameter is used only when
+      [default: quay.io/eclipse/che-operator:nightly] Container image of the operator. This parameter is used only when 
       the installer is the operator
 
   --deployment-name=deployment-name
@@ -288,9 +289,9 @@ OPTIONS
       persistent volume storage class name to use to store Eclipse Che Postgres database
 
   --self-signed-cert
-      Indicates that self signed certificates is used for encryption.
-      This is the flag for 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.
+      Authorize usage of self signed certificates for encryption.
+                           This is the flag for 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.
 
   --workspace-pvc-storage-class-name=workspace-pvc-storage-class-name
       persistent volume(s) storage class name to use to store Eclipse Che workspaces data

src/api/che.ts

@@ -8,11 +8,12 @@
  * SPDX-License-Identifier: EPL-2.0
  **********************************************************************/
 import { CoreV1Api, KubeConfig } from '@kubernetes/client-node'
-import axios from 'axios'
+import axios, { AxiosInstance } from 'axios'
 import * as cp from 'child_process'
 import { cli } from 'cli-ux'
 import * as commandExists from 'command-exists'
 import * as fs from 'fs-extra'
+import * as https from 'https'
 import * as yaml from 'js-yaml'
 import * as path from 'path'
 import { setInterval } from 'timers'
@@ -33,8 +34,17 @@ export class CheHelper {
   kube: KubeHelper
   oc = new OpenShiftHelper()
 
+  private readonly axios: AxiosInstance
+
   constructor(flags: any) {
     this.kube = new KubeHelper(flags)
+
+    // Make axios ignore untrusted certificate error for self-signed certificate case.
+    const httpsAgent = new https.Agent({ rejectUnauthorized: false })
+
+    this.axios = axios.create({
+      httpsAgent
+    })
   }
 
   /**
@@ -136,7 +146,7 @@ export class CheHelper {
     const endpoint = `${cheURL}/api/system/state`
     let response = null
     try {
-      response = await axios.get(endpoint, { timeout: responseTimeoutMs })
+      response = await this.axios.get(endpoint, { timeout: responseTimeoutMs })
     } catch (error) {
       throw this.getCheApiError(error, endpoint)
     }
@@ -151,7 +161,7 @@ export class CheHelper {
     const headers = accessToken ? { Authorization: `${accessToken}` } : null
     let response = null
     try {
-      response = await axios.post(endpoint, null, { headers, timeout: responseTimeoutMs })
+      response = await this.axios.post(endpoint, null, { headers, timeout: responseTimeoutMs })
     } catch (error) {
       if (error.response && error.response.status === 409) {
         return
@@ -177,19 +187,19 @@ export class CheHelper {
   }
 
   async isCheServerReady(cheURL: string, responseTimeoutMs = this.defaultCheResponseTimeoutMs): Promise<boolean> {
-    const id = await axios.interceptors.response.use(response => response, async (error: any) => {
+    const id = await this.axios.interceptors.response.use(response => response, async (error: any) => {
       if (error.config && error.response && (error.response.status === 404 || error.response.status === 503)) {
-        return axios.request(error.config)
+        return this.axios.request(error.config)
       }
       return Promise.reject(error)
     })
 
     try {
-      await axios.get(`${cheURL}/api/system/state`, { timeout: responseTimeoutMs })
-      await axios.interceptors.response.eject(id)
+      await this.axios.get(`${cheURL}/api/system/state`, { timeout: responseTimeoutMs })
+      await this.axios.interceptors.response.eject(id)
       return true
     } catch {
-      await axios.interceptors.response.eject(id)
+      await this.axios.interceptors.response.eject(id)
       return false
     }
   }
@@ -214,7 +224,7 @@ export class CheHelper {
         json.metadata.name = workspaceName
         devfile = yaml.dump(json)
       }
-      response = await axios.post(endpoint, devfile, { headers })
+      response = await this.axios.post(endpoint, devfile, { headers })
     } catch (error) {
       if (!devfile) { throw new Error(`E_NOT_FOUND_DEVFILE - ${devfilePath} - ${error.message}`) }
       if (error.response && error.response.status === 400) {
@@ -232,7 +242,7 @@ export class CheHelper {
 
   async parseDevfile(devfilePath = ''): Promise<string> {
     if (devfilePath.startsWith('http')) {
-      const response = await axios.get(devfilePath)
+      const response = await this.axios.get(devfilePath)
       return response.data
     } else {
       return fs.readFileSync(devfilePath, 'utf8')
@@ -254,7 +264,7 @@ export class CheHelper {
 
     try {
       let workspaceConfig = fs.readFileSync(workspaceConfigPath, 'utf8')
-      response = await axios.post(endpoint, workspaceConfig, { headers })
+      response = await this.axios.post(endpoint, workspaceConfig, { headers })
     } catch (error) {
       if (!workspaceConfig) { throw new Error(`E_NOT_FOUND_WORKSPACE_CONFIG_FILE - ${workspaceConfigPath} - ${error.message}`) }
       if (error.response && error.response.status === 400) {
@@ -274,7 +284,7 @@ export class CheHelper {
     const endpoint = `${cheURL}/api/keycloak/settings`
     let response = null
     try {
-      response = await axios.get(endpoint, { timeout: responseTimeoutMs })
+      response = await this.axios.get(endpoint, { timeout: responseTimeoutMs })
     } catch (error) {
       if (error.response && (error.response.status === 404 || error.response.status === 503)) {
         return false

src/commands/server/delete.ts

@@ -33,7 +33,7 @@ export default class Delete extends Command {
     const notifier = require('node-notifier')
 
     const k8sTasks = new K8sTasks()
-    const helmTasks = new HelmTasks()
+    const helmTasks = new HelmTasks(flags)
     const msAddonTasks = new MinishiftAddonTasks()
     const operatorTasks = new OperatorTasks()
     const cheTasks = new CheTasks(flags)

src/commands/server/start.ts

@@ -74,12 +74,15 @@ export default class Start extends Command {
     tls: flags.boolean({
       char: 's',
       description: `Enable TLS encryption.
-                    Note that for kubernetes 'che-tls' with TLS certificate must be created in the configured namespace.
+                    Note, that this option is turned on by default for kubernetes infrastructure.
+                    If it is needed to provide own certificate, 'che-tls' secret with TLS certificate must be created in the configured namespace. Otherwise, it will be automatically generated.
                     For OpenShift, router will use default cluster certificates.`,
       default: false
     }),
     'self-signed-cert': flags.boolean({
-      description: 'Authorize usage of self signed certificates for encryption. Note that `self-signed-cert` secret with CA certificate must be created in the configured namespace.',
+      description: `Authorize usage of self signed certificates for encryption.
+                    This is the flag for 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.`,
       default: false
     }),
     platform: string({
@@ -202,8 +205,7 @@ export default class Start extends Command {
     const listrOptions: Listr.ListrOptions = { renderer: (flags['listr-renderer'] as any), collapse: false, showSubtasks: true } as Listr.ListrOptions
     ctx.listrOptions = listrOptions
 
-    // TODO temporary workaround.
-    // When tls by default is implemented for all platforms, delete `tls` flag or make it turned on by default.
+    // TODO when tls by default is implemented for all platforms, make `tls` flag turned on by default.
     if (flags.platform === 'k8s' || flags.platform === 'minikube' || flags.platform === 'microk8s') {
       flags.tls = true
     }

src/tasks/component-installers/cert-manager.ts

@@ -8,7 +8,6 @@
  * SPDX-License-Identifier: EPL-2.0
  **********************************************************************/
 
-import { Command } from '@oclif/command'
 import * as fs from 'fs'
 import * as Listr from 'listr'
 import * as os from 'os'
@@ -29,7 +28,7 @@ export class CertManagerTasks {
   /**
    * Returns list of tasks which perform cert-manager checks and deploy and requests self-signed certificate for Che.
    */
-  getTasks(flags: any, command: Command): ReadonlyArray<Listr.ListrTask> {
+  getTasks(flags: any): ReadonlyArray<Listr.ListrTask> {
     return [
       {
         title: 'Check Cert Manager deployment',
@@ -40,34 +39,33 @@ export class CertManagerTasks {
             task.title = `${task.title}...already deployed`
           } else {
             task.title = `${task.title}...not deployed`
+          }
+        }
+      },
+      {
+        title: 'Deploy cert-manager',
+        enabled: ctx => !ctx.certManagerInstalled,
+        task: async (ctx: any, task: any) => {
+          const yamlPath = path.join(flags.templates, 'cert-manager', 'cert-manager.yml')
+          await this.kubeHelper.applyResource(yamlPath)
+          ctx.certManagerInstalled = true
 
-            return new Listr([
-              {
-                title: 'Deploy cert-manager',
-                task: async (ctx: any, task: any) => {
-                  const yamlPath = path.join(flags.templates, 'cert-manager', 'cert-manager.yml')
-                  await this.kubeHelper.applyResource(yamlPath)
-                  ctx.certManagerInstalled = true
-
-                  task.title = `${task.title}...done`
-                }
-              },
-              {
-                title: 'Wait for cert-manager',
-                task: async (ctx: any, task: any) => {
-                  if (!ctx.certManagerInstalled) {
-                    throw new Error('Cert Manager should be deployed before.')
-                  }
-
-                  await this.kubeHelper.waitForPodReady('app.kubernetes.io/name=cert-manager', CERT_MANAGER_NAMESPACE_NAME)
-                  await this.kubeHelper.waitForPodReady('app.kubernetes.io/name=webhook', CERT_MANAGER_NAMESPACE_NAME)
-                  await this.kubeHelper.waitForPodReady('app.kubernetes.io/name=cainjector', CERT_MANAGER_NAMESPACE_NAME)
-
-                  task.title = `${task.title}...ready`
-                }
-              }
-            ], ctx.listrOptions)
+          task.title = `${task.title}...done`
+        }
+      },
+      {
+        title: 'Wait for cert-manager',
+        enabled: ctx => ctx.certManagerInstalled,
+        task: async (ctx: any, task: any) => {
+          if (!ctx.certManagerInstalled) {
+            throw new Error('Cert Manager should be deployed before.')
           }
+
+          await this.kubeHelper.waitForPodReady('app.kubernetes.io/name=cert-manager', CERT_MANAGER_NAMESPACE_NAME)
+          await this.kubeHelper.waitForPodReady('app.kubernetes.io/name=webhook', CERT_MANAGER_NAMESPACE_NAME)
+          await this.kubeHelper.waitForPodReady('app.kubernetes.io/name=cainjector', CERT_MANAGER_NAMESPACE_NAME)
+
+          task.title = `${task.title}...ready`
         }
       },
       {
@@ -100,7 +98,7 @@ export class CertManagerTasks {
                 await this.kubeHelper.createJob(CA_CERT_GENERATION_JOB_NAME, CA_CERT_GENERATION_JOB_IMAGE, CA_CERT_GENERATION_SERVICE_ACCOUNT_NAME, CERT_MANAGER_NAMESPACE_NAME)
                 await this.kubeHelper.waitJob(CA_CERT_GENERATION_JOB_NAME, CERT_MANAGER_NAMESPACE_NAME)
               } catch {
-                command.error('Failed to generate self-signed CA certificate: generating job failed.')
+                throw new Error('Failed to generate self-signed CA certificate: generating job failed.')
               }
             } finally {
               // Clean up resources

src/tasks/installers/helm.ts

@@ -23,11 +23,9 @@ import { CertManagerTasks } from '../../tasks/component-installers/cert-manager'
 
 export class HelmTasks {
   protected kubeHelper: KubeHelper
-  protected command: Command
 
-  constructor(flags: any, command: Command) {
+  constructor(flags: any) {
     this.kubeHelper = new KubeHelper(flags)
-    this.command = command
   }
 
   /**
@@ -88,7 +86,7 @@ export class HelmTasks {
             task.title = `${task.title}...going to generate self-signed one`
 
             const certManagerTasks = new CertManagerTasks(flags)
-            return new Listr(certManagerTasks.getTasks(flags, this.command), ctx.listrOptions)
+            return new Listr(certManagerTasks.getTasks(flags), ctx.listrOptions)
           }
         }
       },

src/tasks/installers/installer.ts

@@ -66,7 +66,7 @@ export class InstallerTasks {
   }
 
   installTasks(flags: any, command: Command): ReadonlyArray<Listr.ListrTask> {
-    const helmTasks = new HelmTasks(flags, command)
+    const helmTasks = new HelmTasks(flags)
     const operatorTasks = new OperatorTasks()
     const minishiftAddonTasks = new MinishiftAddonTasks()
 

test/api/che.test.ts

@@ -119,7 +119,7 @@ describe('Che helper', () => {
   })
   describe('cheNamespaceExist', () => {
     fancy
-      .stub(kc, 'makeApiClient', () => k8sApi)
+      .stub(kube.kc, 'makeApiClient', () => k8sApi)
       .stub(k8sApi, 'readNamespace', jest.fn().mockImplementation(() => { throw new Error() }))
       .it('founds out that a namespace doesn\'t exist', async () => {
         const res = await ch.cheNamespaceExist(namespace)

test/tasks/installers/helm.test.ts

@@ -8,7 +8,6 @@
  * SPDX-License-Identifier: EPL-2.0
  **********************************************************************/
 
-import Command from '@oclif/command'
 import * as execa from 'execa'
 // tslint:disable:object-curly-spacing
 import { expect, fancy } from 'fancy-test'
@@ -17,21 +16,21 @@ import { HelmTasks } from '../../../src/tasks/installers/helm'
 
 jest.mock('execa')
 
-let helmTasks = new HelmTasks({}, {} as Command)
+let helmTasks = new HelmTasks({})
 describe('Helm helper', () => {
   fancy
     .it('check get v3 version', async () => {
       const helmVersionOutput = 'v3.0.0+ge29ce2a';
       (execa as any).mockResolvedValue({ exitCode: 0, stdout: helmVersionOutput })
-      const version = await helmTasks.getVersion();
+      const version = await helmTasks.getVersion()
       expect(version).to.equal('v3.0.0+ge29ce2a')
     })
 
   fancy
     .it('check get v2 version', async () => {
       const helmVersionOutput = 'Client: v2.13.0-rc.2+gb0d4c9e';
       (execa as any).mockResolvedValue({ exitCode: 0, stdout: helmVersionOutput })
-      const version = await helmTasks.getVersion();
+      const version = await helmTasks.getVersion()
       expect(version).to.equal('v2.13.0-rc.2+gb0d4c9e')
     })
 })