some more cleaning up

Signed-off-by: Sebastian Malton <smalton@mirantis.com>

spec/src/main/kubectl_spec.ts

@@ -1,17 +1,17 @@
 jest.mock("electron")
 jest.mock("../../../src/common/user-store")
 
-import { Kubectl, bundledKubectl } from "../../../src/main/kubectl"
+import { Kubectl } from "../../../src/main/kubectl"
 
 describe("kubectlVersion", () => {
 
   it("returns bundled version if exactly same version used", async () => {
-    const kubectl = new Kubectl(bundledKubectl.kubectlVersion)
+    const kubectl = new Kubectl(Kubectl.bundled().kubectlVersion)
-    expect(kubectl.kubectlVersion).toBe(bundledKubectl.kubectlVersion)
+    expect(kubectl.kubectlVersion).toBe(Kubectl.bundled().kubectlVersion)
   })
 
   it("returns bundled version if same major.minor version is used", async () => {
     const kubectl = new Kubectl("1.17.0")
-    expect(kubectl.kubectlVersion).toBe(bundledKubectl.kubectlVersion)
+    expect(kubectl.kubectlVersion).toBe(Kubectl.bundled().kubectlVersion)
   })
 })

src/common/request.ts

@@ -1,7 +1,7 @@
-import * as request from "request"
+import { Options } from "request"
 import { UserStore } from "../common/user-store"
 
-export function globalRequestOpts(requestOpts: request.Options ) {
+export function globalRequestOpts(requestOpts: Options): Options {
   const userPrefs = UserStore.getInstance().getPreferences()
   if (userPrefs.httpsProxy) {
     requestOpts.proxy = userPrefs.httpsProxy

src/common/user-store.ts

@@ -54,17 +54,11 @@ export class UserStore {
 
   public getPreferences(): UserPreferences {
     const prefs = this.store.get("preferences", {})
-    if (!prefs.colorTheme) {
+    prefs.colorTheme = prefs.colorTheme || "dark";
-      prefs.colorTheme = "dark"
+    prefs.downloadMirror = prefs.downloadMirror || "default";
-    }
+    prefs.allowTelemetry = prefs.allowTelemetry ?? true;
-    if (!prefs.downloadMirror) {
-      prefs.downloadMirror = "default"
-    }
-    if (prefs.allowTelemetry === undefined) {
-      prefs.allowTelemetry = true
-    }
 
-    return prefs
+    return prefs;
   }
 
   static getInstance(): UserStore {

src/main/kubectl.ts

@@ -32,18 +32,19 @@ const packageMirrors: Map<string, string> = new Map([
   ["china", "https://mirror.azure.cn/kubernetes/kubectl"]
 ])
 
-const initScriptVersionString = "# lens-initscript v3\n"
+const initScriptVersionString = "# lens-initscript v3";
-
+const isDevelopment = process.env.NODE_ENV !== "production";
-const isDevelopment = process.env.NODE_ENV !== "production"
+const bundledPath = (
-let bundledPath: string = null
+  isDevelopment
-
+    ? path.join(process.cwd(), "binaries", "client", process.platform, process.arch, "kubectl")
-if(isDevelopment) {
+    : path.join(process.resourcesPath, process.arch, "kubectl")
-  bundledPath = path.join(process.cwd(), "binaries", "client", process.platform, process.arch, "kubectl")
+) 
-} else {
+  + (process.platform === "win32" ? ".exe" : "");
-  bundledPath = path.join(process.resourcesPath, process.arch, "kubectl")
+const archMap: Record<string, string> = {
-}
+  "x64": "amd64",
-
+  "x86": "386",
-if(process.platform === "win32") bundledPath = `${bundledPath}.exe`
+  "ia32": "386",
+};
 
 export class Kubectl {
 
@@ -77,21 +78,11 @@ export class Kubectl {
       logger.debug("Set kubectl version " + this.kubectlVersion + " for cluster version " + clusterVersion + " using fallback")
     }
 
-    let arch = null
+    const arch = archMap[process.arch] || process.arch;
-
-    if(process.arch == "x64") {
-      arch = "amd64"
-    } else if(process.arch == "x86" || process.arch == "ia32") {
-      arch = "386"
-    } else {
-      arch = process.arch
-    }
-
     const platformName = process.platform === "win32" ? "windows" : process.platform
     const binaryName = process.platform === "win32" ? "kubectl.exe" : "kubectl"
 
     this.url = `${this.getDownloadMirror()}/v${this.kubectlVersion}/bin/${platformName}/${arch}/${binaryName}`
-
     this.dirname = path.normalize(path.join(Kubectl.kubectlDir, this.kubectlVersion))
     this.path = path.join(this.dirname, binaryName)
   }
@@ -107,7 +98,7 @@ export class Kubectl {
     }
   }
 
-  public async binDir() {
+  public async binDir(): Promise<string> {
     try {
       await this.ensureKubectl()
       return this.dirname
@@ -117,70 +108,90 @@ export class Kubectl {
     }
   }
 
-  public async checkBinary(checkVersion = true) {
+  public async checkBinary(checkVersion = true): Promise<boolean> {
     const exists = await pathExists(this.path)
-    if (exists) {
+    if (!exists) {
-      if (!checkVersion) {
+      return false;
+    }
+
+    if (!checkVersion) {
+      return true
+    }
+
+    try {
+      const { stdout } = await promiseExec(`"${this.path}" version --client=true -o json`)
+      const output = JSON.parse(stdout)
+      let version: string = output.clientVersion.gitVersion
+      if (version[0] === 'v') {
+        version = version.slice(1)
+      }
+      if (version === this.kubectlVersion) {
+        logger.debug(`Local kubectl is version ${this.kubectlVersion}`)
         return true
       }
-
+      logger.error(`Local kubectl is version ${version}, expected ${this.kubectlVersion}, unlinking`)
-      try {
-        const { stdout, stderr } = await promiseExec(`"${this.path}" version --client=true -o json`)
-        const output = JSON.parse(stdout)
-        let version: string = output.clientVersion.gitVersion
-        if (version[0] === 'v') {
-          version = version.slice(1)
-        }
-        if (version === this.kubectlVersion) {
-          logger.debug(`Local kubectl is version ${this.kubectlVersion}`)
-          return true
-        }
-        logger.error(`Local kubectl is version ${version}, expected ${this.kubectlVersion}, unlinking`)
-      }
-      catch(err) {
-        logger.error(`Local kubectl failed to run properly (${err.message}), unlinking`)
-      }
-      await fs.promises.unlink(this.path)
     }
-    return false
+    catch(err) {
+      logger.error(`Local kubectl failed to run properly (${err.message}), unlinking`)
+    }
+
+    await fs.promises.unlink(this.path)
+    return false;
   }
 
   protected async checkBundled(): Promise<boolean> {
-    if(this.kubectlVersion === Kubectl.bundledKubectlVersion) {
+    if (this.kubectlVersion !== Kubectl.bundledKubectlVersion) {
-      try {
+      return false;
-        const exist = await pathExists(this.path)
+    }
-        if (!exist) {
+
-          await fs.promises.copyFile(Kubectl.bundledKubectlPath, this.path)
+    try {
-          await fs.promises.chmod(this.path, 0o755)
+      const exist = await pathExists(this.path)
-        }
+      if (!exist) {
-        return true
+        await fs.promises.copyFile(Kubectl.bundledKubectlPath, this.path)
-      } catch(err) {
+        await fs.promises.chmod(this.path, 0o755)
-        logger.error("Could not copy the bundled kubectl to app-data: " + err)
-        return false
       }
-    } else {
+      return true
+    } catch(err) {
+      logger.error("Could not copy the bundled kubectl to app-data: " + err)
       return false
     }
   }
 
   public async ensureKubectl(): Promise<boolean> {
-    await ensureDir(this.dirname, 0o755)
+    await ensureDir(this.dirname, 0o755);
-    return lockFile.lock(this.dirname).then(async (release) => {
+    let release: () => Promise<void>;
+
+    try {
+      release = await lockFile.lock(this.dirname);
       logger.debug(`Acquired a lock for ${this.kubectlVersion}`)
-      const bundled = await this.checkBundled()
+
-      const isValid = await this.checkBinary(!bundled)
+      try {
-      if(!isValid) {
+        if (!await this.checkBinary(!await this.checkBundled())) {
-        await this.downloadKubectl().catch((error) => { logger.error(error) });
+          await this.downloadKubectl()
+        }
+      } catch (e) {
+        logger.error(e)
       }
-      await this.writeInitScripts().catch((error) => { logger.error("Failed to write init scripts"); logger.error(error) })
+      
-      logger.debug(`Releasing lock for ${this.kubectlVersion}`)
+      try {
-      release()
+        await this.writeInitScripts();
+      } catch (e) {
+        logger.error("Failed to write init scripts"); 
+        logger.error(e)
+      }
+
       return true
-    }).catch((e) => {
+    } catch (e) {
-      logger.error(`Failed to get a lock for ${this.kubectlVersion}`)
       logger.error(e)
       return false
-    })
+    } finally {
+      if (release) {
+        logger.debug(`Releasing lock for ${this.kubectlVersion}`)
+        release();
+      } else {
+        logger.error(`Failed to get a lock for ${this.kubectlVersion}`)
+      }
+    }
   }
 
   public async downloadKubectl() {
@@ -229,52 +240,55 @@ export class Kubectl {
 
   protected async writeInitScripts() {
     const helmPath = helmCli.getBinaryDir()
-    const fsPromises = fs.promises;
     const bashScriptPath = path.join(this.dirname, '.bash_set_path')
-    const bashScriptIsLatest = await this.scriptIsLatest(bashScriptPath)
+
-    if(!bashScriptIsLatest) {
+    if (!await this.scriptIsLatest(bashScriptPath)) {
-      let bashScript = "" + initScriptVersionString
+      const bashScript = `
-      bashScript += "tempkubeconfig=\"$KUBECONFIG\"\n"
+${initScriptVersionString}
-      bashScript += "test -f \"/etc/profile\" && . \"/etc/profile\"\n"
+tempkubeconfig="$KUBECONFIG"
-      bashScript += "if test -f \"$HOME/.bash_profile\"; then\n"
+test -f "/etc/profile" && . "/etc/profile"
-      bashScript += "  . \"$HOME/.bash_profile\"\n"
+if test -f "$HOME/.bash_profile"; then
-      bashScript += "elif test -f \"$HOME/.bash_login\"; then\n"
+  . "$HOME/.bash_profile"
-      bashScript += "  . \"$HOME/.bash_login\"\n"
+elif test -f "$HOME/.bash_login"; then
-      bashScript += "elif test -f \"$HOME/.profile\"; then\n"
+  . "$HOME/.bash_login"
-      bashScript += "  . \"$HOME/.profile\"\n"
+elif test -f "$HOME/.profile"; then
-      bashScript += "fi\n"
+  . "$HOME/.profile"
-      bashScript += `export PATH="${this.dirname}:${helmPath}:$PATH"\n`
+fi
-      bashScript += "export KUBECONFIG=\"$tempkubeconfig\"\n"
+export PATH="${this.dirname}:${helmPath}:$PATH"
-      bashScript += "unset tempkubeconfig\n"
+export KUBECONFIG="$tempkubeconfig"
-      await fsPromises.writeFile(bashScriptPath, bashScript.toString(), { mode: 0o644 })
+unset tempkubeconfig
+`.trimLeft();
+
+      await fs.promises.writeFile(bashScriptPath, bashScript.toString(), { mode: 0o644 })
     }
 
     const zshScriptPath = path.join(this.dirname, '.zlogin')
-    const zshScriptIsLatest = await this.scriptIsLatest(zshScriptPath)
+    if (!await this.scriptIsLatest(zshScriptPath)) {
-    if(!zshScriptIsLatest) {
+      const zshScript = `
-      let zshScript = "" + initScriptVersionString
+${initScriptVersionString}
+tempkubeconfig="$KUBECONFIG"
 
-      zshScript += "tempkubeconfig=\"$KUBECONFIG\"\n"
+${/* restore previous ZDOTDIR */""}
-      // restore previous ZDOTDIR
+export ZDOTDIR="$OLD_ZDOTDIR"
-      zshScript += "export ZDOTDIR=\"$OLD_ZDOTDIR\"\n"
-      // source all the files
-      zshScript += "test -f \"$OLD_ZDOTDIR/.zshenv\" && . \"$OLD_ZDOTDIR/.zshenv\"\n"
-      zshScript += "test -f \"$OLD_ZDOTDIR/.zprofile\" && . \"$OLD_ZDOTDIR/.zprofile\"\n"
-      zshScript += "test -f \"$OLD_ZDOTDIR/.zlogin\" && . \"$OLD_ZDOTDIR/.zlogin\"\n"
-      zshScript += "test -f \"$OLD_ZDOTDIR/.zshrc\" && . \"$OLD_ZDOTDIR/.zshrc\"\n"
 
-      // voodoo to replace any previous occurences of kubectl path in the PATH
+${/* source all the files */""}
-      zshScript += `kubectlpath=\"${this.dirname}"\n`
+test -f "$OLD_ZDOTDIR/.zshenv" && . "$OLD_ZDOTDIR/.zshenv"
-      zshScript += `helmpath=\"${helmPath}"\n`
+test -f "$OLD_ZDOTDIR/.zprofile" && . "$OLD_ZDOTDIR/.zprofile"
-      zshScript += "p=\":$kubectlpath:\"\n"
+test -f "$OLD_ZDOTDIR/.zlogin" && . "$OLD_ZDOTDIR/.zlogin"
-      zshScript += "d=\":$PATH:\"\n"
+test -f "$OLD_ZDOTDIR/.zshrc" && . "$OLD_ZDOTDIR/.zshrc"
-      zshScript += "d=${d//$p/:}\n"
+
-      zshScript += "d=${d/#:/}\n"
+${/* voodoo to replace any previous occurences of kubectl path in the PATH */""}
-      zshScript += "export PATH=\"$kubectlpath:$helmpath:${d/%:/}\"\n"
+kubectlpath="${this.dirname}"
-      zshScript += "export KUBECONFIG=\"$tempkubeconfig\"\n"
+p=":$kubectlpath:"
-      zshScript += "unset tempkubeconfig\n"
+d=":$PATH:"
-      zshScript += "unset OLD_ZDOTDIR\n"
+d=\${d//$p/:}
-      await fsPromises.writeFile(zshScriptPath, zshScript.toString(), { mode: 0o644 })
+export PATH="$kubectlpath:$helmpath:\${d/%:/}"
+export KUBECONFIG="$tempkubeconfig"
+unset tempkubeconfig
+unset OLD_ZDOTDIR
+`.trimLeft();
+
+      await fs.promises.writeFile(zshScriptPath, zshScript.toString(), { mode: 0o644 })
     }
   }
 
@@ -285,15 +299,12 @@ export class Kubectl {
   }
 
   protected getDownloadMirror() {
-    if (process.platform == "darwin") {
+    if (process.platform != "darwin") {
-      return packageMirrors.get("default") // MacOS packages are only available from default
+      // MacOS packages are only available from default, so don't try
+      const mirror = packageMirrors.get(UserStore.getInstance().getPreferences().downloadMirror)
+      if (mirror) { return mirror }
     }
-    const mirror = packageMirrors.get(UserStore.getInstance().getPreferences().downloadMirror)
-    if (mirror) { return mirror }
 
     return packageMirrors.get("default")
   }
 }
-
-const bundledKubectl = Kubectl.bundled()
-export { bundledKubectl }