From ce9a13f5edf8f779e5db9f3136cc0db836abf02d Mon Sep 17 00:00:00 2001 From: Florent Benoit Date: Fri, 26 Jan 2024 12:58:07 +0100 Subject: [PATCH] chore: avoid the usage of sudo-prompt uses Podman Desktop API instead fixes https://github.com/containers/podman-desktop-extension-minikube/issues/26 Change-Id: I8e3705a2d924c6ba61667ab8d814f5d50304dd2d Signed-off-by: Florent Benoit Change-Id: I096dbd6dc0a07b11d6fbec07cd9f45ea9e328a0d --- package.json | 1 - src/minikube-installer.spec.ts | 21 ++++++++ src/util.spec.ts | 96 ++++++++++++++++++++++++++++++++-- src/util.ts | 61 ++++++++++----------- yarn.lock | 5 -- 5 files changed, 140 insertions(+), 44 deletions(-) diff --git a/package.json b/package.json index 137dea9..7a1a7c0 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,6 @@ "watch": "vite build -w" }, "dependencies": { - "sudo-prompt": "^9.2.1", "@octokit/rest": "^20.0.2", "@types/node": "^18", "tmp-promise": "^3.0.3" diff --git a/src/minikube-installer.spec.ts b/src/minikube-installer.spec.ts index 5e05678..68f2c89 100644 --- a/src/minikube-installer.spec.ts +++ b/src/minikube-installer.spec.ts @@ -32,6 +32,9 @@ vi.mock('@podman-desktop/api', async () => { withProgress: vi.fn(), showNotification: vi.fn(), }, + process: { + exec: vi.fn(), + }, env: { isWindows: vi.fn(), }, @@ -89,6 +92,24 @@ test('error: expect installBinaryToSystem to fail with a non existing binary', a value: 'linux', }); + vi.spyOn(extensionApi.process, 'exec').mockImplementation( + () => + new Promise((_, reject) => { + const error: extensionApi.RunError = { + name: '', + message: 'Command failed', + exitCode: 1603, + command: 'command', + stdout: 'stdout', + stderr: 'stderr', + cancelled: false, + killed: false, + }; + + reject(error); + }), + ); + // Run installBinaryToSystem with a non-binary file try { await installBinaryToSystem('test', 'tmpBinary'); diff --git a/src/util.spec.ts b/src/util.spec.ts index b3b244d..3687b4d 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -20,11 +20,11 @@ import * as extensionApi from '@podman-desktop/api'; import { afterEach, beforeEach, expect, test, vi } from 'vitest'; -import { detectMinikube, getMinikubePath, runCliCommand } from './util'; +import { detectMinikube, getMinikubePath, installBinaryToSystem, runCliCommand } from './util'; import * as childProcess from 'node:child_process'; import type { MinikubeInstaller } from './minikube-installer'; - -vi.mock('node:child_process'); +import * as fs from 'node:fs'; +import * as path from 'node:path'; vi.mock('@podman-desktop/api', async () => { return { @@ -34,6 +34,9 @@ vi.mock('@podman-desktop/api', async () => { withProgress: vi.fn(), showNotification: vi.fn(), }, + process: { + exec: vi.fn(), + }, env: { isMac: vi.fn(), isWindows: vi.fn(), @@ -48,9 +51,18 @@ vi.mock('@podman-desktop/api', async () => { }; }); +vi.mock('node:child_process'); + +// mock exists sync +vi.mock('node:fs', async () => { + return { + existsSync: vi.fn(), + }; +}); + const originalProcessEnv = process.env; beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); process.env = {}; }); @@ -195,3 +207,79 @@ test('runCliCommand/killProcess on Windows', async () => { expect(spawnSpy.mock.calls[1]).toEqual(['taskkill', ['/pid', 'pid123', '/f', '/t']]); }); + +test('error: expect installBinaryToSystem to fail with a non existing binary', async () => { + // Mock the platform to be linux + Object.defineProperty(process, 'platform', { + value: 'linux', + }); + + vi.spyOn(extensionApi.process, 'exec').mockImplementation( + () => + new Promise((_, reject) => { + const error: extensionApi.RunError = { + name: '', + message: 'Command failed', + exitCode: 1603, + command: 'command', + stdout: 'stdout', + stderr: 'stderr', + cancelled: false, + killed: false, + }; + + reject(error); + }), + ); + + // Expect await installBinaryToSystem to throw an error + await expect(installBinaryToSystem('test', 'tmpBinary')).rejects.toThrowError(); +}); + +test('success: installBinaryToSystem on mac with /usr/local/bin already created', async () => { + // Mock the platform to be darwin + Object.defineProperty(process, 'platform', { + value: 'darwin', + }); + + // Mock existsSync to be true since within the function it's doing: !fs.existsSync(localBinDir) + vi.spyOn(fs, 'existsSync').mockImplementation(() => { + return true; + }); + + // Run installBinaryToSystem which will trigger the spyOn mock + await installBinaryToSystem('test', 'tmpBinary'); + + // check called with admin being true + expect(extensionApi.process.exec).toBeCalledWith('chmod', expect.arrayContaining(['+x', 'test'])); + expect(extensionApi.process.exec).toHaveBeenNthCalledWith( + 2, + 'cp', + ['test', `${path.sep}usr${path.sep}local${path.sep}bin${path.sep}tmpBinary`], + { isAdmin: true }, + ); +}); + +test('expect: installBinaryToSystem on linux with /usr/local/bin NOT created yet (expect mkdir -p command)', async () => { + // Mock the platform to be darwin + Object.defineProperty(process, 'platform', { + value: 'linux', + }); + + // Mock existsSync to be false since within the function it's doing: !fs.existsSync(localBinDir) + vi.spyOn(fs, 'existsSync').mockImplementation(() => { + return false; + }); + + // Run installBinaryToSystem which will trigger the spyOn mock + await installBinaryToSystem('test', 'tmpBinary'); + + expect(extensionApi.process.exec).toBeCalledWith('chmod', expect.arrayContaining(['+x', 'test'])); + + // check called with admin being true + expect(extensionApi.process.exec).toBeCalledWith( + 'cp', + ['test', `mkdir -p /usr/local/bin && ${path.sep}usr${path.sep}local${path.sep}bin${path.sep}tmpBinary`], + expect.objectContaining({ isAdmin: true }), + ); +}); diff --git a/src/util.ts b/src/util.ts index 5b799c3..8861452 100644 --- a/src/util.ts +++ b/src/util.ts @@ -20,9 +20,9 @@ import * as os from 'node:os'; import * as path from 'node:path'; import type { ChildProcess } from 'node:child_process'; import { spawn } from 'node:child_process'; -import * as sudo from 'sudo-prompt'; import * as extensionApi from '@podman-desktop/api'; import type { MinikubeInstaller } from './minikube-installer'; +import * as fs from 'node:fs'; export interface SpawnResult { stdOut: string; @@ -168,7 +168,7 @@ export async function installBinaryToSystem(binaryPath: string, binaryName: stri // Before copying the file, make sure it's executable (chmod +x) for Linux and Mac if (system === 'linux' || system === 'darwin') { try { - await runCliCommand('chmod', ['+x', binaryPath]); + await extensionApi.process.exec('chmod', ['+x', binaryPath]); console.log(`Made ${binaryPath} executable`); } catch (error) { throw new Error(`Error making binary executable: ${error}`); @@ -178,45 +178,38 @@ export async function installBinaryToSystem(binaryPath: string, binaryName: stri // Create the appropriate destination path (Windows uses AppData/Local, Linux and Mac use /usr/local/bin) // and the appropriate command to move the binary to the destination path let destinationPath: string; - let command: string[]; - if (system == 'win32') { + let command: string; + let args: string[]; + if (system === 'win32') { destinationPath = path.join(os.homedir(), 'AppData', 'Local', 'Microsoft', 'WindowsApps', `${binaryName}.exe`); - command = ['copy', binaryPath, destinationPath]; + command = 'copy'; + args = [`"${binaryPath}"`, `"${destinationPath}"`]; } else { destinationPath = path.join('/usr/local/bin', binaryName); - command = ['cp', binaryPath, destinationPath]; + command = 'cp'; + args = [binaryPath, destinationPath]; } - // If windows or mac, use sudo-prompt to elevate the privileges - // if Linux, use sudo and polkit support - if (system === 'win32' || system === 'darwin') { - return new Promise((resolve, reject) => { - // Convert the command array to a string for sudo prompt - // the name is used for the prompt - const sudoOptions = { - name: `${binaryName} Binary Installation`, - }; - const sudoCommand = command.join(' '); - sudo.exec(sudoCommand, sudoOptions, error => { - if (error) { - console.error(`Failed to install '${binaryName}' binary: ${error}`); - reject(error); - } else { - console.log(`Successfully installed '${binaryName}' binary.`); - resolve(); - } - }); - }); - } else { - try { - // Use pkexec in order to elevate the prileges / ask for password for copying to /usr/local/bin - await runCliCommand('pkexec', command); - console.log(`Successfully installed '${binaryName}' binary.`); - } catch (error) { - console.error(`Failed to install '${binaryName}' binary: ${error}`); - throw error; + // If on macOS or Linux, check to see if the /usr/local/bin directory exists, + // if it does not, then add mkdir -p /usr/local/bin to the start of the command when moving the binary. + const localBinDir = '/usr/local/bin'; + if ((system === 'linux' || system === 'darwin') && !fs.existsSync(localBinDir)) { + if (system === 'darwin') { + args.unshift('mkdir', '-p', localBinDir, '&&'); + } else { + // add mkdir -p /usr/local/bin just after the first item or args array (so it'll be in the -c shell instruction) + args[args.length - 1] = `mkdir -p /usr/local/bin && ${args[args.length - 1]}`; } } + + try { + // Use admin prileges / ask for password for copying to /usr/local/bin + await extensionApi.process.exec(command, args, { isAdmin: true }); + console.log(`Successfully installed '${binaryName}' binary.`); + } catch (error) { + console.error(`Failed to install '${binaryName}' binary: ${error}`); + throw error; + } } function killProcess(spawnProcess: ChildProcess) { diff --git a/yarn.lock b/yarn.lock index 2c4cd51..99d544f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2659,11 +2659,6 @@ strip-literal@^1.3.0: dependencies: acorn "^8.10.0" -sudo-prompt@^9.2.1: - version "9.2.1" - resolved "https://registry.yarnpkg.com/sudo-prompt/-/sudo-prompt-9.2.1.tgz#77efb84309c9ca489527a4e749f287e6bdd52afd" - integrity sha512-Mu7R0g4ig9TUuGSxJavny5Rv0egCEtpZRNMrZaYS1vxkiIxGiGUwoezU3LazIQ+KE04hTrTfNPgxU5gzi7F5Pw== - supports-color@^7.1.0: version "7.2.0" resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-7.2.0.tgz#1b7dcdcb32b8138801b3e478ba6a51caa89648da"