From e9f6330830dbf4b0042390b055e47628709d5b85 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 6 Dec 2021 15:29:03 -0500 Subject: [PATCH 1/5] Replace all uses of promiseExec with promiseExecFile Signed-off-by: Sebastian Malton --- src/common/system-ca.ts | 22 ++- src/common/utils/promise-exec.ts | 3 +- src/main/helm/helm-release-manager.ts | 257 ++++++++++++++++---------- src/main/helm/helm-repo-manager.ts | 128 ++++++++----- src/main/helm/helm-service.ts | 16 +- src/main/kubectl.ts | 9 +- 6 files changed, 274 insertions(+), 161 deletions(-) diff --git a/src/common/system-ca.ts b/src/common/system-ca.ts index 22d85e3b4215..bc7231a5f60a 100644 --- a/src/common/system-ca.ts +++ b/src/common/system-ca.ts @@ -22,7 +22,7 @@ import { isMac, isWindows } from "./vars"; import wincaAPI from "win-ca/api"; import https from "https"; -import { promiseExec } from "./utils/promise-exec"; +import { promiseExecFile } from "./utils/promise-exec"; // DST Root CA X3, which was expired on 9.30.2021 export const DSTRootCAX3 = "-----BEGIN CERTIFICATE-----\nMIIDSjCCAjKgAwIBAgIQRK+wgNajJ7qJMDmGLvhAazANBgkqhkiG9w0BAQUFADA/\nMSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT\nDkRTVCBSb290IENBIFgzMB4XDTAwMDkzMDIxMTIxOVoXDTIxMDkzMDE0MDExNVow\nPzEkMCIGA1UEChMbRGlnaXRhbCBTaWduYXR1cmUgVHJ1c3QgQ28uMRcwFQYDVQQD\nEw5EU1QgUm9vdCBDQSBYMzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB\nAN+v6ZdQCINXtMxiZfaQguzH0yxrMMpb7NnDfcdAwRgUi+DoM3ZJKuM/IUmTrE4O\nrz5Iy2Xu/NMhD2XSKtkyj4zl93ewEnu1lcCJo6m67XMuegwGMoOifooUMM0RoOEq\nOLl5CjH9UL2AZd+3UWODyOKIYepLYYHsUmu5ouJLGiifSKOeDNoJjj4XLh7dIN9b\nxiqKqy69cK3FCxolkHRyxXtqqzTWMIn/5WgTe1QLyNau7Fqckh49ZLOMxt+/yUFw\n7BZy1SbsOFU5Q9D8/RhcQPGX69Wam40dutolucbY38EVAjqr2m7xPi71XAicPNaD\naeQQmxkqtilX4+U9m5/wAl0CAwEAAaNCMEAwDwYDVR0TAQH/BAUwAwEB/zAOBgNV\nHQ8BAf8EBAMCAQYwHQYDVR0OBBYEFMSnsaR7LHH62+FLkHX/xBVghYkQMA0GCSqG\nSIb3DQEBBQUAA4IBAQCjGiybFwBcqR7uKGY3Or+Dxz9LwwmglSBd49lZRNI+DT69\nikugdB/OEIKcdBodfpga3csTS7MgROSR6cz8faXbauX+5v3gTt23ADq1cEmv8uXr\nAvHRAosZy5Q6XkjEGB5YGV8eAlrwDPGxrancWYaLbumR9YbK+rlmM6pZW87ipxZz\nR8srzJmwN0jP41ZL9c8PDHIyh8bwRLtTcm1D9SZImlJnt1ir/md2cXjbDaJWFBM5\nJDGFoqgCWjBH4d1QB7wCCZAA62RjYJsWvIjJEubSfZGL+T0yjWW06XyxV3bqxbYo\nOb8VZRzI9neWagqNdwvYkQsEjgfbKbYK7p2CNTUQ\n-----END CERTIFICATE-----\n"; @@ -33,19 +33,25 @@ export function isCertActive(cert: string) { return !isExpired; } +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Cheatsheet#other_assertions +const certSplitPattern = /(?=-----BEGIN\sCERTIFICATE-----)/g; + +async function execSecurity(...args: string[]): Promise { + const { stdout } = await promiseExecFile("/usr/bin/security", args); + + return stdout.split(certSplitPattern); +} + /** * Get root CA certificate from MacOSX system keychain * Only return non-expred certificates. */ export async function getMacRootCA() { // inspired mac-ca https://github.com/jfromaniello/mac-ca - const args = "find-certificate -a -p"; - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Cheatsheet#other_assertions - const splitPattern = /(?=-----BEGIN\sCERTIFICATE-----)/g; - const systemRootCertsPath = "/System/Library/Keychains/SystemRootCertificates.keychain"; - const bin = "/usr/bin/security"; - const trusted = (await promiseExec(`${bin} ${args}`)).stdout.toString().split(splitPattern); - const rootCA = (await promiseExec(`${bin} ${args} ${systemRootCertsPath}`)).stdout.toString().split(splitPattern); + const [trusted, rootCA] = await Promise.all([ + execSecurity("find-certificate", "-a", "-p"), + execSecurity("find-certificate", "-a", "-p", "/System/Library/Keychains/SystemRootCertificates.keychain"), + ]); return [...new Set([...trusted, ...rootCA])].filter(isCertActive); } diff --git a/src/common/utils/promise-exec.ts b/src/common/utils/promise-exec.ts index 8b9d5b72cc5e..b2d39ee11017 100644 --- a/src/common/utils/promise-exec.ts +++ b/src/common/utils/promise-exec.ts @@ -20,7 +20,6 @@ */ import * as util from "util"; -import { exec, execFile } from "child_process"; +import { execFile } from "child_process"; -export const promiseExec = util.promisify(exec); export const promiseExecFile = util.promisify(execFile); diff --git a/src/main/helm/helm-release-manager.ts b/src/main/helm/helm-release-manager.ts index 4f96562e5d13..e9b2cfeab1fa 100644 --- a/src/main/helm/helm-release-manager.ts +++ b/src/main/helm/helm-release-manager.ts @@ -22,161 +22,224 @@ import * as tempy from "tempy"; import fse from "fs-extra"; import * as yaml from "js-yaml"; -import { promiseExec } from "../../common/utils/promise-exec"; +import { promiseExecFile } from "../../common/utils/promise-exec"; import { helmCli } from "./helm-cli"; -import type { Cluster } from "../cluster"; import { toCamelCase } from "../../common/utils/camelCase"; +import type { BaseEncodingOptions } from "fs"; +import { execFile, ExecFileOptions } from "child_process"; -export async function listReleases(pathToKubeconfig: string, namespace?: string) { - const helm = await helmCli.binaryPath(); - const namespaceFlag = namespace ? `-n ${namespace}` : "--all-namespaces"; +async function execHelm(args: string[], options?: BaseEncodingOptions & ExecFileOptions): Promise { + const helmCliPath = await helmCli.binaryPath(); try { - const { stdout } = await promiseExec(`"${helm}" ls --output json ${namespaceFlag} --kubeconfig ${pathToKubeconfig}`); - const output = JSON.parse(stdout); - - if (output.length == 0) { - return output; - } - output.forEach((release: any, index: number) => { - output[index] = toCamelCase(release); - }); + const { stdout } = await promiseExecFile(helmCliPath, args, options); - return output; + return stdout; } catch (error) { throw error?.stderr || error; } } +export async function listReleases(pathToKubeconfig: string, namespace?: string): Promise[]> { + const args = [ + "ls", + "--output", "json", + ]; -export async function installChart(chart: string, values: any, name: string | undefined, namespace: string, version: string, pathToKubeconfig: string) { - const helm = await helmCli.binaryPath(); - const fileName = tempy.file({ name: "values.yaml" }); + if (namespace) { + args.push("-n", namespace); + } else { + args.push("--all-namespaces"); + } - await fse.writeFile(fileName, yaml.dump(values)); + args.push("--kubeconfig", pathToKubeconfig); - try { - let generateName = ""; + const output = JSON.parse(await execHelm(args)); - if (!name) { - generateName = "--generate-name"; - name = ""; - } - const { stdout } = await promiseExec(`"${helm}" install ${name} ${chart} --version ${version} -f ${fileName} --namespace ${namespace} --kubeconfig ${pathToKubeconfig} ${generateName}`); - const releaseName = stdout.split("\n")[0].split(" ")[1].trim(); + if (!Array.isArray(output) || output.length == 0) { + return []; + } + + return output.map(toCamelCase); +} + + +export async function installChart(chart: string, values: any, name: string | undefined = "", namespace: string, version: string, kubeconfigPath: string) { + const valuesFilePath = tempy.file({ name: "values.yaml" }); + + await fse.writeFile(valuesFilePath, yaml.dump(values)); + + const args = [ + "install", + name, + chart, + "--version", version, + "--values", valuesFilePath, + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ]; + + if (!name) { + args.push("--generate-name"); + } + + try { + const output = await execHelm(args); + const releaseName = output.split("\n")[0].split(" ")[1].trim(); return { - log: stdout, + log: output, release: { name: releaseName, namespace, }, }; - } catch (error) { - throw error?.stderr || error; } finally { - await fse.unlink(fileName); + await fse.unlink(valuesFilePath); } } -export async function upgradeRelease(name: string, chart: string, values: any, namespace: string, version: string, cluster: Cluster) { - const helm = await helmCli.binaryPath(); - const fileName = tempy.file({ name: "values.yaml" }); +export async function upgradeRelease(name: string, chart: string, values: any, namespace: string, version: string, kubeconfigPath: string, kubectlPath: string) { + const valuesFilePath = tempy.file({ name: "values.yaml" }); + + await fse.writeFile(valuesFilePath, yaml.dump(values)); - await fse.writeFile(fileName, yaml.dump(values)); + const args = [ + "upgrade", + name, + chart, + "--version", version, + "--values", valuesFilePath, + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ]; try { - const proxyKubeconfig = await cluster.getProxyKubeconfigPath(); - const { stdout } = await promiseExec(`"${helm}" upgrade ${name} ${chart} --version ${version} -f ${fileName} --namespace ${namespace} --kubeconfig ${proxyKubeconfig}`); + const output = await execHelm(args); return { - log: stdout, - release: getRelease(name, namespace, cluster), + log: output, + release: getRelease(name, namespace, kubeconfigPath, kubectlPath), }; - } catch (error) { - throw error?.stderr || error; } finally { - await fse.unlink(fileName); + await fse.unlink(valuesFilePath); } } -export async function getRelease(name: string, namespace: string, cluster: Cluster) { - try { - const helm = await helmCli.binaryPath(); - const proxyKubeconfig = await cluster.getProxyKubeconfigPath(); +export async function getRelease(name: string, namespace: string, kubeconfigPath: string, kubectlPath: string) { + const args = [ + "status", + name, + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + "--output", "json", + ]; - const { stdout } = await promiseExec(`"${helm}" status ${name} --output json --namespace ${namespace} --kubeconfig ${proxyKubeconfig}`, { - maxBuffer: 32 * 1024 * 1024 * 1024, // 32 MiB - }); - const release = JSON.parse(stdout); + const release = JSON.parse(await execHelm(args, { + maxBuffer: 32 * 1024 * 1024 * 1024, // 32 MiB + })); - release.resources = await getResources(name, namespace, cluster); + release.resources = await getResources(name, namespace, kubeconfigPath, kubectlPath); - return release; - } catch (error) { - throw error?.stderr || error; - } + return release; } -export async function deleteRelease(name: string, namespace: string, pathToKubeconfig: string) { - try { - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" delete ${name} --namespace ${namespace} --kubeconfig ${pathToKubeconfig}`); - - return stdout; - } catch (error) { - throw error?.stderr || error; - } +export async function deleteRelease(name: string, namespace: string, kubeconfigPath: string) { + return execHelm([ + "delete", + name, + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ]); } interface GetValuesOptions { namespace: string; all?: boolean; - pathToKubeconfig: string; + kubeconfigPath: string; } -export async function getValues(name: string, { namespace, all = false, pathToKubeconfig }: GetValuesOptions) { - try { - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" get values ${name} ${all ? "--all" : ""} --output yaml --namespace ${namespace} --kubeconfig ${pathToKubeconfig}`); +export async function getValues(name: string, { namespace, all = false, kubeconfigPath }: GetValuesOptions) { + const args = [ + "get", + "values", + name, + ]; - return stdout; - } catch (error) { - throw error?.stderr || error; + if (all) { + args.push("--all"); } -} -export async function getHistory(name: string, namespace: string, pathToKubeconfig: string) { - try { - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" history ${name} --output json --namespace ${namespace} --kubeconfig ${pathToKubeconfig}`); + args.push( + "--output", "yaml", + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ); - return JSON.parse(stdout); - } catch (error) { - throw error?.stderr || error; - } + return execHelm(args); } -export async function rollback(name: string, namespace: string, revision: number, pathToKubeconfig: string) { - try { - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" rollback ${name} ${revision} --namespace ${namespace} --kubeconfig ${pathToKubeconfig}`); +export async function getHistory(name: string, namespace: string, kubeconfigPath: string) { + return JSON.parse(await execHelm([ + "history", + name, + "--output", "json", + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ])); +} - return stdout; - } catch (error) { - throw error?.stderr || error; - } +export async function rollback(name: string, namespace: string, revision: number, kubeconfigPath: string) { + return JSON.parse(await execHelm([ + "rollback", + name, + "--output", "json", + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ])); } -async function getResources(name: string, namespace: string, cluster: Cluster) { - try { - const helm = await helmCli.binaryPath(); - const kubectl = await cluster.ensureKubectl(); - const kubectlPath = await kubectl.getPath(); - const pathToKubeconfig = await cluster.getProxyKubeconfigPath(); - const { stdout } = await promiseExec(`"${helm}" get manifest ${name} --namespace ${namespace} --kubeconfig ${pathToKubeconfig} | "${kubectlPath}" get -n ${namespace} --kubeconfig ${pathToKubeconfig} -f - -o=json`); +async function getResources(name: string, namespace: string, kubeconfigPath: string, kubectlPath: string) { + const helmArgs = [ + "get", + "manifest", + name, + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + ]; + const kubectlArgs = [ + "get", + "--namespace", namespace, + "--kubeconfig", kubeconfigPath, + "-f", "-", + "--output", "json", + ]; - return JSON.parse(stdout).items; + try { + const helmOutput = await execHelm(helmArgs); + + return new Promise((resolve, reject) => { + let stdout = ""; + const kubectl = execFile(kubectlPath, kubectlArgs); + + kubectl.stdout.on("data", output => stdout += output); + kubectl.stdin.write(helmOutput); + kubectl.stdin.end(); + + kubectl + .on("exit", (code, signal) => { + if (typeof code === "number") { + if (code) { + reject(new Error(`Kubectl exited with code ${code}`)); + } else { + resolve(JSON.parse(stdout).items); + } + } else { + reject(new Error(`Kubectl exited with signal ${signal}`)); + } + }) + .on("error", reject); + }); } catch { return []; } diff --git a/src/main/helm/helm-repo-manager.ts b/src/main/helm/helm-repo-manager.ts index 1c36bef2bef6..184cf648679f 100644 --- a/src/main/helm/helm-repo-manager.ts +++ b/src/main/helm/helm-repo-manager.ts @@ -20,13 +20,14 @@ */ import yaml from "js-yaml"; -import { readFile } from "fs-extra"; -import { promiseExec } from "../../common/utils/promise-exec"; +import { BaseEncodingOptions, readFile } from "fs-extra"; +import { promiseExecFile } from "../../common/utils/promise-exec"; import { helmCli } from "./helm-cli"; import { Singleton } from "../../common/utils/singleton"; import { customRequestPromise } from "../../common/request"; import orderBy from "lodash/orderBy"; import logger from "../logger"; +import type { ExecFileOptions } from "child_process"; export type HelmEnv = Record & { HELM_REPOSITORY_CACHE?: string; @@ -49,6 +50,18 @@ export interface HelmRepo { password?: string, } +async function execHelm(args: string[], options?: BaseEncodingOptions & ExecFileOptions): Promise { + const helmCliPath = await helmCli.binaryPath(); + + try { + const { stdout } = await promiseExecFile(helmCliPath, args, options); + + return stdout; + } catch (error) { + throw error?.stderr || error; + } +} + export class HelmRepoManager extends Singleton { protected repos: HelmRepo[]; protected helmEnv: HelmEnv; @@ -77,22 +90,21 @@ export class HelmRepoManager extends Singleton { } protected static async parseHelmEnv() { - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" env`).catch((error) => { - throw(error.stderr); - }); - const lines = stdout.split(/\r?\n/); // split by new line feed - const env: HelmEnv = {}; + const output = await execHelm(["env"]); + const env = output + .split("\n") + .map(line => { + const match = line.match(/^(?[^=]*)=(?.*)$/); - lines.forEach((line: string) => { - const [key, value] = line.split("="); + if (match) { + return [match.groups.key, match.groups.value]; + } - if (key && value) { - env[key] = value.replace(/"/g, ""); // strip quotas - } - }); + return null; + }) + .filter(Boolean); - return env; + return Object.fromEntries(env); } public async repo(name: string): Promise { @@ -116,7 +128,7 @@ export class HelmRepoManager extends Singleton { }; } - public async repositories(): Promise { + public async repositories(previousAttempt = false): Promise { try { if (!this.initialized) { await this.init(); @@ -125,9 +137,13 @@ export class HelmRepoManager extends Singleton { const { repositories } = await this.readConfig(); if (!repositories.length) { + if (previousAttempt) { + throw new Error("Previous add repo called did not add repo"); + } + await HelmRepoManager.addRepo({ name: "bitnami", url: "https://charts.bitnami.com/bitnami" }); - return await this.repositories(); + return await this.repositories(true); } return repositories.map(repo => ({ @@ -135,57 +151,71 @@ export class HelmRepoManager extends Singleton { cacheFilePath: `${this.helmEnv.HELM_REPOSITORY_CACHE}/${repo.name}-index.yaml`, })); } catch (error) { - logger.error(`[HELM]: repositories listing error "${error}"`); + logger.error(`[HELM]: repositories listing error`, error); return []; } } public static async update() { - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" repo update`).catch((error) => { - return { stdout: error.stdout }; - }); - - return stdout; + return execHelm([ + "repo", + "update", + ]); } public static async addRepo({ name, url }: HelmRepo) { logger.info(`[HELM]: adding repo "${name}" from ${url}`); - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" repo add ${name} ${url}`).catch((error) => { - throw(error.stderr); - }); - return stdout; + return execHelm([ + "repo", + "add", + name, + url, + ]); } - public static async addCustomRepo(repoAttributes : HelmRepo) { - logger.info(`[HELM]: adding repo "${repoAttributes.name}" from ${repoAttributes.url}`); - const helm = await helmCli.binaryPath(); + public static async addCustomRepo({ name, url, insecureSkipTlsVerify, username, password, caFile, keyFile, certFile }: HelmRepo) { + logger.info(`[HELM]: adding repo ${name} from ${url}`); + const args = [ + "repo", + "add", + ]; - const insecureSkipTlsVerify = repoAttributes.insecureSkipTlsVerify ? " --insecure-skip-tls-verify" : ""; - const username = repoAttributes.username ? ` --username "${repoAttributes.username}"` : ""; - const password = repoAttributes.password ? ` --password "${repoAttributes.password}"` : ""; - const caFile = repoAttributes.caFile ? ` --ca-file "${repoAttributes.caFile}"` : ""; - const keyFile = repoAttributes.keyFile ? ` --key-file "${repoAttributes.keyFile}"` : ""; - const certFile = repoAttributes.certFile ? ` --cert-file "${repoAttributes.certFile}"` : ""; + if (insecureSkipTlsVerify) { + args.push("--insecure-skip-tls-verify"); + } - const addRepoCommand = `"${helm}" repo add ${repoAttributes.name} ${repoAttributes.url}${insecureSkipTlsVerify}${username}${password}${caFile}${keyFile}${certFile}`; - const { stdout } = await promiseExec(addRepoCommand).catch((error) => { - throw(error.stderr); - }); + if (username) { + args.push("--username", username); + } - return stdout; + if (password) { + args.push("--password", password); + } + + if (caFile) { + args.push("--ca-file", caFile); + } + + if (keyFile) { + args.push("--key-file", keyFile); + } + + if (certFile) { + args.push("--cert-file", certFile); + } + + return execHelm(args); } public static async removeRepo({ name, url }: HelmRepo): Promise { - logger.info(`[HELM]: removing repo "${name}" from ${url}`); - const helm = await helmCli.binaryPath(); - const { stdout } = await promiseExec(`"${helm}" repo remove ${name}`).catch((error) => { - throw(error.stderr); - }); + logger.info(`[HELM]: removing repo ${name} (${url})`); - return stdout; + return execHelm([ + "repo", + "remove", + name, + ]); } } diff --git a/src/main/helm/helm-service.ts b/src/main/helm/helm-service.ts index 2e86e3651300..4d4637b3b1da 100644 --- a/src/main/helm/helm-service.ts +++ b/src/main/helm/helm-service.ts @@ -65,13 +65,19 @@ class HelmService { public async listReleases(cluster: Cluster, namespace: string = null) { const proxyKubeconfig = await cluster.getProxyKubeconfigPath(); + logger.debug("list releases"); + return listReleases(proxyKubeconfig, namespace); } public async getRelease(cluster: Cluster, releaseName: string, namespace: string) { + const kubeconfigPath = await cluster.getProxyKubeconfigPath(); + const kubectl = await cluster.ensureKubectl(); + const kubectlPath = await kubectl.getPath(); + logger.debug("Fetch release"); - return getRelease(releaseName, namespace, cluster); + return getRelease(releaseName, namespace, kubeconfigPath, kubectlPath); } public async getReleaseValues(releaseName: string, { cluster, namespace, all }: GetReleaseValuesArgs) { @@ -79,7 +85,7 @@ class HelmService { logger.debug("Fetch release values"); - return getValues(releaseName, { namespace, all, pathToKubeconfig }); + return getValues(releaseName, { namespace, all, kubeconfigPath: pathToKubeconfig }); } public async getReleaseHistory(cluster: Cluster, releaseName: string, namespace: string) { @@ -99,9 +105,13 @@ class HelmService { } public async updateRelease(cluster: Cluster, releaseName: string, namespace: string, data: { chart: string; values: {}; version: string }) { + const proxyKubeconfig = await cluster.getProxyKubeconfigPath(); + const kubectl = await cluster.ensureKubectl(); + const kubectlPath = await kubectl.getPath(); + logger.debug("Upgrade release"); - return upgradeRelease(releaseName, data.chart, data.values, namespace, data.version, cluster); + return upgradeRelease(releaseName, data.chart, data.values, namespace, data.version, proxyKubeconfig, kubectlPath); } public async rollback(cluster: Cluster, releaseName: string, namespace: string, revision: number) { diff --git a/src/main/kubectl.ts b/src/main/kubectl.ts index dbe450608293..2ac1831c41a0 100644 --- a/src/main/kubectl.ts +++ b/src/main/kubectl.ts @@ -21,7 +21,7 @@ import path from "path"; import fs from "fs"; -import { promiseExec } from "../common/utils/promise-exec"; +import { promiseExecFile } from "../common/utils/promise-exec"; import logger from "./logger"; import { ensureDir, pathExists } from "fs-extra"; import * as lockFile from "proper-lockfile"; @@ -199,7 +199,12 @@ export class Kubectl { if (exists) { try { - const { stdout } = await promiseExec(`"${path}" version --client=true -o json`); + const args = [ + "version", + "--client", "true", + "--output", "json", + ]; + const { stdout } = await promiseExecFile(path, args); const output = JSON.parse(stdout); if (!checkVersion) { From 844b7be954ff46baf58a12f04934943e12a3c8de Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 8 Dec 2021 15:53:04 -0500 Subject: [PATCH 2/5] review comments Signed-off-by: Sebastian Malton --- src/main/helm/helm-release-manager.ts | 21 ++++++++++++--------- src/main/helm/helm-repo-manager.ts | 20 +++++++++----------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/main/helm/helm-release-manager.ts b/src/main/helm/helm-release-manager.ts index e9b2cfeab1fa..824341023ed1 100644 --- a/src/main/helm/helm-release-manager.ts +++ b/src/main/helm/helm-release-manager.ts @@ -69,15 +69,19 @@ export async function installChart(chart: string, values: any, name: string | un await fse.writeFile(valuesFilePath, yaml.dump(values)); - const args = [ - "install", - name, + const args = ["install"]; + + if (name) { + args.push(name); + } + + args.push( chart, "--version", version, "--values", valuesFilePath, "--namespace", namespace, "--kubeconfig", kubeconfigPath, - ]; + ); if (!name) { args.push("--generate-name"); @@ -193,7 +197,6 @@ export async function rollback(name: string, namespace: string, revision: number return JSON.parse(await execHelm([ "rollback", name, - "--output", "json", "--namespace", namespace, "--kubeconfig", kubeconfigPath, ])); @@ -222,10 +225,6 @@ async function getResources(name: string, namespace: string, kubeconfigPath: str let stdout = ""; const kubectl = execFile(kubectlPath, kubectlArgs); - kubectl.stdout.on("data", output => stdout += output); - kubectl.stdin.write(helmOutput); - kubectl.stdin.end(); - kubectl .on("exit", (code, signal) => { if (typeof code === "number") { @@ -239,6 +238,10 @@ async function getResources(name: string, namespace: string, kubeconfigPath: str } }) .on("error", reject); + + kubectl.stdout.on("data", output => stdout += output); + kubectl.stdin.write(helmOutput); + kubectl.stdin.end(); }); } catch { return []; diff --git a/src/main/helm/helm-repo-manager.ts b/src/main/helm/helm-repo-manager.ts index 184cf648679f..69dfd3bd62a4 100644 --- a/src/main/helm/helm-repo-manager.ts +++ b/src/main/helm/helm-repo-manager.ts @@ -91,20 +91,18 @@ export class HelmRepoManager extends Singleton { protected static async parseHelmEnv() { const output = await execHelm(["env"]); - const env = output - .split("\n") - .map(line => { - const match = line.match(/^(?[^=]*)=(?.*)$/); + const lines = output.split(/\r?\n/); // split by new line feed + const env: HelmEnv = {}; - if (match) { - return [match.groups.key, match.groups.value]; - } + lines.forEach((line: string) => { + const [key, value] = line.split("="); - return null; - }) - .filter(Boolean); + if (key && value) { + env[key] = value.replace(/"/g, ""); // strip quotas + } + }); - return Object.fromEntries(env); + return env; } public async repo(name: string): Promise { From e7045727d40722661cc500ebefaf268bc3abc6f9 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Thu, 9 Dec 2021 11:38:06 -0500 Subject: [PATCH 3/5] Fix addCustomRepo getResources error output Signed-off-by: Sebastian Malton --- src/main/helm/helm-release-manager.ts | 8 +++++--- src/main/helm/helm-repo-manager.ts | 2 ++ .../components/+preferences/add-helm-repo-dialog.tsx | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/helm/helm-release-manager.ts b/src/main/helm/helm-release-manager.ts index 824341023ed1..5a5f1e6ef282 100644 --- a/src/main/helm/helm-release-manager.ts +++ b/src/main/helm/helm-release-manager.ts @@ -223,15 +223,16 @@ async function getResources(name: string, namespace: string, kubeconfigPath: str return new Promise((resolve, reject) => { let stdout = ""; + let stderr = ""; const kubectl = execFile(kubectlPath, kubectlArgs); kubectl .on("exit", (code, signal) => { if (typeof code === "number") { - if (code) { - reject(new Error(`Kubectl exited with code ${code}`)); - } else { + if (code === 0) { resolve(JSON.parse(stdout).items); + } else { + reject(stderr); } } else { reject(new Error(`Kubectl exited with signal ${signal}`)); @@ -239,6 +240,7 @@ async function getResources(name: string, namespace: string, kubeconfigPath: str }) .on("error", reject); + kubectl.stderr.on("data", output => stderr += output); kubectl.stdout.on("data", output => stdout += output); kubectl.stdin.write(helmOutput); kubectl.stdin.end(); diff --git a/src/main/helm/helm-repo-manager.ts b/src/main/helm/helm-repo-manager.ts index 69dfd3bd62a4..ec5b61565ae5 100644 --- a/src/main/helm/helm-repo-manager.ts +++ b/src/main/helm/helm-repo-manager.ts @@ -178,6 +178,8 @@ export class HelmRepoManager extends Singleton { const args = [ "repo", "add", + name, + url, ]; if (insecureSkipTlsVerify) { diff --git a/src/renderer/components/+preferences/add-helm-repo-dialog.tsx b/src/renderer/components/+preferences/add-helm-repo-dialog.tsx index d6760fa561a8..608c33bad27d 100644 --- a/src/renderer/components/+preferences/add-helm-repo-dialog.tsx +++ b/src/renderer/components/+preferences/add-helm-repo-dialog.tsx @@ -121,7 +121,7 @@ export class AddHelmRepoDialog extends React.Component {
this.setFilepath(fileType, v)} @@ -172,7 +172,7 @@ export class AddHelmRepoDialog extends React.Component { close={this.close} > - {this.addCustomRepo();}}> + this.addCustomRepo()}>
Date: Tue, 14 Dec 2021 12:02:10 -0500 Subject: [PATCH 4/5] Skip integrationt test, will turn it on again in next PR Signed-off-by: Sebastian Malton --- integration/__tests__/app-preferences.tests.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/__tests__/app-preferences.tests.ts b/integration/__tests__/app-preferences.tests.ts index 47dffd6a9066..8cdf19a5488a 100644 --- a/integration/__tests__/app-preferences.tests.ts +++ b/integration/__tests__/app-preferences.tests.ts @@ -71,7 +71,8 @@ describe("preferences page tests", () => { } }, 10*60*1000); - utils.itIf(process.platform !== "win32")("ensures helm repos", async () => { + // Skipping, but will turn it on again in the follow up PR + it.skip("ensures helm repos", async () => { await window.click("[data-testid=kubernetes-tab]"); await window.waitForSelector("[data-testid=repository-name]", { timeout: 140_000, From 07d829af33ceee9c96aba2b790025b9a86b68160 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Fri, 17 Dec 2021 09:42:49 -0500 Subject: [PATCH 5/5] Remove non-security bug fix Signed-off-by: Sebastian Malton --- src/main/helm/helm-repo-manager.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/helm/helm-repo-manager.ts b/src/main/helm/helm-repo-manager.ts index ec5b61565ae5..f8edc12e008d 100644 --- a/src/main/helm/helm-repo-manager.ts +++ b/src/main/helm/helm-repo-manager.ts @@ -126,7 +126,7 @@ export class HelmRepoManager extends Singleton { }; } - public async repositories(previousAttempt = false): Promise { + public async repositories(): Promise { try { if (!this.initialized) { await this.init(); @@ -135,13 +135,9 @@ export class HelmRepoManager extends Singleton { const { repositories } = await this.readConfig(); if (!repositories.length) { - if (previousAttempt) { - throw new Error("Previous add repo called did not add repo"); - } - await HelmRepoManager.addRepo({ name: "bitnami", url: "https://charts.bitnami.com/bitnami" }); - return await this.repositories(true); + return await this.repositories(); } return repositories.map(repo => ({