From 0e9356436c13750810d1c407a68ced4cbfc57519 Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Mon, 3 Feb 2020 12:21:21 +0200 Subject: [PATCH] Fixes according to review Signed-off-by: Mykola Morhun --- README.md | 21 +++---- src/api/che.ts | 34 +++++++---- src/commands/server/delete.ts | 2 +- src/commands/server/start.ts | 10 ++-- .../component-installers/cert-manager.ts | 56 +++++++++---------- src/tasks/installers/helm.ts | 6 +- src/tasks/installers/installer.ts | 2 +- test/api/che.test.ts | 2 +- test/tasks/installers/helm.test.ts | 7 +-- 9 files changed, 74 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index ec013dab2..4ded81b75 100644 --- a/README.md +++ b/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 diff --git a/src/api/che.ts b/src/api/che.ts index 421dc30a6..ab32c62db 100644 --- a/src/api/che.ts +++ b/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 { - 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 { 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 diff --git a/src/commands/server/delete.ts b/src/commands/server/delete.ts index c0cf19fed..a738b73aa 100644 --- a/src/commands/server/delete.ts +++ b/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) diff --git a/src/commands/server/start.ts b/src/commands/server/start.ts index f3a0f139f..cca2285a7 100644 --- a/src/commands/server/start.ts +++ b/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 } diff --git a/src/tasks/component-installers/cert-manager.ts b/src/tasks/component-installers/cert-manager.ts index a755fbc50..1dd5f02b0 100644 --- a/src/tasks/component-installers/cert-manager.ts +++ b/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 { + getTasks(flags: any): ReadonlyArray { 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 diff --git a/src/tasks/installers/helm.ts b/src/tasks/installers/helm.ts index e462ab303..f1a19f196 100644 --- a/src/tasks/installers/helm.ts +++ b/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) } } }, diff --git a/src/tasks/installers/installer.ts b/src/tasks/installers/installer.ts index f1598dcb4..83f03abd9 100644 --- a/src/tasks/installers/installer.ts +++ b/src/tasks/installers/installer.ts @@ -66,7 +66,7 @@ export class InstallerTasks { } installTasks(flags: any, command: Command): ReadonlyArray { - const helmTasks = new HelmTasks(flags, command) + const helmTasks = new HelmTasks(flags) const operatorTasks = new OperatorTasks() const minishiftAddonTasks = new MinishiftAddonTasks() diff --git a/test/api/che.test.ts b/test/api/che.test.ts index 4d63a08f0..ac1cdbf20 100644 --- a/test/api/che.test.ts +++ b/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) diff --git a/test/tasks/installers/helm.test.ts b/test/tasks/installers/helm.test.ts index 5684eb3bb..f5764a76b 100644 --- a/test/tasks/installers/helm.test.ts +++ b/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,13 +16,13 @@ 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') }) @@ -31,7 +30,7 @@ describe('Helm helper', () => { .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') }) })