From 90c3701523e13f9e5177e4e2c64830815fd0ce2d Mon Sep 17 00:00:00 2001 From: Luca Stocchi <49404737+lstocchi@users.noreply.github.com> Date: Wed, 22 May 2024 16:52:55 +0200 Subject: [PATCH] fix: update podman api and replace local runCliCommand with process.exec (#44) Signed-off-by: lstocchi --- package.json | 4 +- src/create-cluster.spec.ts | 21 +++--- src/create-cluster.ts | 5 +- src/extension.ts | 4 +- src/image-handler.spec.ts | 12 ++-- src/image-handler.ts | 4 +- src/minikube-installer.spec.ts | 4 -- src/util.spec.ts | 122 +++------------------------------ src/util.ts | 96 +------------------------- yarn.lock | 8 +-- 10 files changed, 43 insertions(+), 237 deletions(-) diff --git a/package.json b/package.json index 0c0228d..1ab2fa1 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "publisher": "podman-desktop", "license": "Apache-2.0", "engines": { - "podman-desktop": "^1.0.0" + "podman-desktop": ">=1.10.0" }, "main": "./dist/extension.js", "contributes": { @@ -80,7 +80,7 @@ }, "devDependencies": { "7zip-min": "^1.4.4", - "@podman-desktop/api": "^1.6.4", + "@podman-desktop/api": "^1.10.0", "@typescript-eslint/eslint-plugin": "^6.19.1", "@typescript-eslint/parser": "^6.19.1", "@vitest/coverage-v8": "^1.2.1", diff --git a/src/create-cluster.spec.ts b/src/create-cluster.spec.ts index b82da64..fa28162 100644 --- a/src/create-cluster.spec.ts +++ b/src/create-cluster.spec.ts @@ -19,7 +19,6 @@ import { beforeEach, expect, test, vi } from 'vitest'; import type { Mock } from 'vitest'; import { createCluster } from './create-cluster'; -import { runCliCommand } from './util'; import type { TelemetryLogger } from '@podman-desktop/api'; import * as extensionApi from '@podman-desktop/api'; @@ -29,12 +28,14 @@ vi.mock('@podman-desktop/api', async () => { kubernetes: { createResources: vi.fn(), }, + process: { + exec: vi.fn(), + }, }; }); vi.mock('./util', async () => { return { - runCliCommand: vi.fn(), getMinikubePath: vi.fn(), }; }); @@ -52,7 +53,7 @@ const telemetryLoggerMock = { test('expect error is cli returns non zero exit code', async () => { try { - (runCliCommand as Mock).mockReturnValue({ exitCode: -1, error: 'error' }); + (extensionApi.process.exec as Mock).mockReturnValue({ exitCode: -1, error: 'error' }); await createCluster({}, undefined, '', telemetryLoggerMock, undefined); } catch (err) { expect(err).to.be.a('Error'); @@ -62,7 +63,7 @@ test('expect error is cli returns non zero exit code', async () => { }); test('expect cluster to be created', async () => { - (runCliCommand as Mock).mockReturnValue({ exitCode: 0 }); + (extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 }); await createCluster({}, undefined, '', telemetryLoggerMock, undefined); expect(telemetryLogUsageMock).toHaveBeenNthCalledWith( 1, @@ -75,7 +76,7 @@ test('expect cluster to be created', async () => { test('expect error if Kubernetes reports error', async () => { try { - (runCliCommand as Mock).mockReturnValue({ exitCode: 0 }); + (extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 }); const logger = { log: vi.fn(), error: vi.fn(), @@ -95,7 +96,7 @@ test('expect error if Kubernetes reports error', async () => { }); test('expect cluster to be created with custom base image', async () => { - (runCliCommand as Mock).mockReturnValue({ exitCode: 0 }); + (extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 }); await createCluster( { 'minikube.cluster.creation.base-image': 'myCustomImage' }, undefined, @@ -110,7 +111,7 @@ test('expect cluster to be created with custom base image', async () => { ); expect(telemetryLogErrorMock).not.toBeCalled(); expect(extensionApi.kubernetes.createResources).not.toBeCalled(); - expect(runCliCommand).toBeCalledWith( + expect(extensionApi.process.exec).toBeCalledWith( '', [ 'start', @@ -124,12 +125,11 @@ test('expect cluster to be created with custom base image', async () => { 'myCustomImage', ], expect.anything(), - undefined, ); }); test('expect cluster to be created with custom mount', async () => { - (runCliCommand as Mock).mockReturnValue({ exitCode: 0 }); + (extensionApi.process.exec as Mock).mockReturnValue({ exitCode: 0 }); await createCluster( { 'minikube.cluster.creation.mount-string': '/foo:/bar' }, undefined, @@ -144,7 +144,7 @@ test('expect cluster to be created with custom mount', async () => { ); expect(telemetryLogErrorMock).not.toBeCalled(); expect(extensionApi.kubernetes.createResources).not.toBeCalled(); - expect(runCliCommand).toBeCalledWith( + expect(extensionApi.process.exec).toBeCalledWith( '', [ 'start', @@ -159,6 +159,5 @@ test('expect cluster to be created with custom mount', async () => { '/foo:/bar', ], expect.anything(), - undefined, ); }); diff --git a/src/create-cluster.ts b/src/create-cluster.ts index 64b273c..91716c0 100644 --- a/src/create-cluster.ts +++ b/src/create-cluster.ts @@ -15,9 +15,10 @@ * * SPDX-License-Identifier: Apache-2.0 ***********************************************************************/ -import { getMinikubePath, runCliCommand } from './util'; +import { getMinikubePath } from './util'; import type { Logger, TelemetryLogger, CancellationToken } from '@podman-desktop/api'; +import { process as processApi } from '@podman-desktop/api'; export async function createCluster( // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -52,7 +53,7 @@ export async function createCluster( // now execute the command to create the cluster try { - await runCliCommand(minikubeCli, startArgs, { env, logger }, token); + await processApi.exec(minikubeCli, startArgs, { env, logger, token }); telemetryLogger.logUsage('createCluster', { driver, runtime }); } catch (error) { const errorMessage = error instanceof Error ? error.message : error; diff --git a/src/extension.ts b/src/extension.ts index d78db4f..0a2bf7f 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -17,7 +17,7 @@ ***********************************************************************/ import * as extensionApi from '@podman-desktop/api'; -import { detectMinikube, getMinikubePath, runCliCommand } from './util'; +import { detectMinikube, getMinikubePath } from './util'; import { MinikubeInstaller } from './minikube-installer'; import type { CancellationToken, Logger } from '@podman-desktop/api'; import { window } from '@podman-desktop/api'; @@ -123,7 +123,7 @@ async function updateClusters(provider: extensionApi.Provider, containers: exten delete: async (logger): Promise => { const env = Object.assign({}, process.env); env.PATH = getMinikubePath(); - await runCliCommand(minikubeCli, ['delete', '--profile', cluster.name], { env, logger }); + await extensionApi.process.exec(minikubeCli, ['delete', '--profile', cluster.name], { env, logger }); }, }; // create a new connection diff --git a/src/image-handler.spec.ts b/src/image-handler.spec.ts index adea2a4..60efa03 100644 --- a/src/image-handler.spec.ts +++ b/src/image-handler.spec.ts @@ -21,7 +21,7 @@ import type { Mock } from 'vitest'; import { ImageHandler } from './image-handler'; import * as extensionApi from '@podman-desktop/api'; import * as fs from 'node:fs'; -import { getMinikubePath, runCliCommand } from './util'; +import { getMinikubePath } from './util'; let imageHandler: ImageHandler; vi.mock('@podman-desktop/api', async () => { @@ -29,6 +29,9 @@ vi.mock('@podman-desktop/api', async () => { containerEngine: { saveImage: vi.fn(), }, + process: { + exec: vi.fn(), + }, window: { showNotification: vi.fn(), showInformationMessage: vi.fn(), @@ -38,7 +41,6 @@ vi.mock('@podman-desktop/api', async () => { vi.mock('./util', async () => { return { - runCliCommand: vi.fn().mockReturnValue({ exitCode: 0 }), getMinikubePath: vi.fn(), }; }); @@ -119,9 +121,9 @@ test('expect cli is called with right PATH', async () => { ); expect(getMinikubePath).toBeCalled(); - expect(runCliCommand).toBeCalledTimes(1); - // grab the env parameter of the first call to runCliCommand - const props = (runCliCommand as Mock).mock.calls[0][2]; + expect(extensionApi.process.exec).toBeCalledTimes(1); + // grab the env parameter of the first call to process.Exec + const props = (extensionApi.process.exec as Mock).mock.calls[0][2]; expect(props).to.have.property('env'); const env = props.env; expect(env).to.have.property('PATH'); diff --git a/src/image-handler.ts b/src/image-handler.ts index f9f312f..01bedd4 100644 --- a/src/image-handler.ts +++ b/src/image-handler.ts @@ -18,7 +18,7 @@ import type { MinikubeCluster } from './extension'; import * as extensionApi from '@podman-desktop/api'; import { tmpName } from 'tmp-promise'; -import { getMinikubePath, runCliCommand } from './util'; +import { getMinikubePath } from './util'; import * as fs from 'node:fs'; type ImageInfo = { engineId: string; name?: string; tag?: string }; @@ -71,7 +71,7 @@ export class ImageHandler { await extensionApi.containerEngine.saveImage(image.engineId, name, filename); // Run the minikube image load command to push the image to the cluster - await runCliCommand(minikubeCli, ['-p', selectedCluster.label, 'image', 'load', filename], { + await extensionApi.process.exec(minikubeCli, ['-p', selectedCluster.label, 'image', 'load', filename], { env: env, }); diff --git a/src/minikube-installer.spec.ts b/src/minikube-installer.spec.ts index 68f2c89..f27453f 100644 --- a/src/minikube-installer.spec.ts +++ b/src/minikube-installer.spec.ts @@ -60,10 +60,6 @@ const telemetryLoggerMock = { logError: telemetryLogErrorMock, } as unknown as extensionApi.TelemetryLogger; -vi.mock('runCliCommand', async () => { - return vi.fn(); -}); - beforeEach(() => { installer = new MinikubeInstaller('.', telemetryLoggerMock); vi.clearAllMocks(); diff --git a/src/util.spec.ts b/src/util.spec.ts index 3687b4d..585125e 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -20,8 +20,7 @@ import * as extensionApi from '@podman-desktop/api'; import { afterEach, beforeEach, expect, test, vi } from 'vitest'; -import { detectMinikube, getMinikubePath, installBinaryToSystem, runCliCommand } from './util'; -import * as childProcess from 'node:child_process'; +import { detectMinikube, getMinikubePath, installBinaryToSystem } from './util'; import type { MinikubeInstaller } from './minikube-installer'; import * as fs from 'node:fs'; import * as path from 'node:path'; @@ -86,126 +85,27 @@ test('getMinikubePath on macOS with existing PATH', async () => { expect(computedPath).toEqual(`${existingPATH}:/usr/local/bin:/opt/homebrew/bin:/opt/local/bin:/opt/podman/bin`); }); -test.each([ - ['macOS', true, false], - ['windows', false, true], -])('detectMinikube on %s', async (operatingSystem, isMac, isWindows) => { - vi.mocked(extensionApi.env).isMac = isMac; - vi.mocked(extensionApi.env).isWindows = isWindows; - - // spy on runCliCommand - const spawnSpy = vi.spyOn(childProcess, 'spawn'); - - const onEventMock = vi.fn(); - - onEventMock.mockImplementation((event: string, callback: (data: string) => void) => { - // delay execution - if (event === 'close') { - setTimeout(() => { - callback(0 as unknown as string); - }, 500); - } - }); - - spawnSpy.mockReturnValue({ - on: onEventMock, - stdout: { setEncoding: vi.fn(), on: vi.fn() }, - stderr: { setEncoding: vi.fn(), on: vi.fn() }, - } as unknown as childProcess.ChildProcessWithoutNullStreams); - +test('detectMinikube', async () => { const fakeMinikubeInstaller = { getAssetInfo: vi.fn(), } as unknown as MinikubeInstaller; + const execMock = vi.spyOn(extensionApi.process, 'exec').mockResolvedValue({ + command: '', + stderr: '', + stdout: '', + }); + const result = await detectMinikube('', fakeMinikubeInstaller); expect(result).toEqual('minikube'); // expect not called getAssetInfo expect(fakeMinikubeInstaller.getAssetInfo).not.toBeCalled(); - expect(spawnSpy).toBeCalled(); + expect(execMock).toBeCalled(); // expect right parameters - if (isMac) { - expect(spawnSpy.mock.calls[0][0]).toEqual('minikube'); - } else if (isWindows) { - expect(spawnSpy.mock.calls[0][0]).toEqual('"minikube"'); - } - expect(spawnSpy.mock.calls[0][1]).toEqual(['version']); -}); - -test('runCliCommand/killProcess on macOS', async () => { - vi.mocked(extensionApi.env).isMac = true; - vi.mocked(extensionApi.env).isWindows = false; - - // spy on runCliCommand - const spawnSpy = vi.spyOn(childProcess, 'spawn'); - - const killMock = vi.fn(); - - spawnSpy.mockReturnValue({ - kill: killMock, - on: vi.fn(), - stdout: { setEncoding: vi.fn(), on: vi.fn() }, - stderr: { setEncoding: vi.fn(), on: vi.fn() }, - } as unknown as childProcess.ChildProcessWithoutNullStreams); - - const fakeToken = { - onCancellationRequested: vi.fn(), - } as unknown as extensionApi.CancellationToken; - - vi.mocked(fakeToken.onCancellationRequested).mockImplementation((callback: any): extensionApi.Disposable => { - // abort execution after 500ms - setTimeout(() => { - callback(); - }, 500); - - return extensionApi.Disposable.from({ dispose: vi.fn() }); - }); - - await expect(runCliCommand('fooCommand', [], undefined, fakeToken)).rejects.toThrow('Execution cancelled'); - - expect(spawnSpy.mock.calls[0][0]).toEqual('fooCommand'); - - expect(killMock).toBeCalled(); -}); - -test('runCliCommand/killProcess on Windows', async () => { - vi.mocked(extensionApi.env).isMac = false; - vi.mocked(extensionApi.env).isWindows = true; - - // spy on runCliCommand - const spawnSpy = vi.spyOn(childProcess, 'spawn'); - - const killMock = vi.fn(); - - spawnSpy.mockReturnValue({ - kill: killMock, - pid: 'pid123', - on: vi.fn(), - stdout: { setEncoding: vi.fn(), on: vi.fn() }, - stderr: { setEncoding: vi.fn(), on: vi.fn() }, - } as unknown as childProcess.ChildProcessWithoutNullStreams); - - const fakeToken = { - onCancellationRequested: vi.fn(), - } as unknown as extensionApi.CancellationToken; - - vi.mocked(fakeToken.onCancellationRequested).mockImplementation((callback: any): extensionApi.Disposable => { - // abort execution after 500ms - setTimeout(() => { - callback(); - }, 500); - - return extensionApi.Disposable.from({ dispose: vi.fn() }); - }); - - await expect(runCliCommand('fooCommand', [], undefined, fakeToken)).rejects.toThrow('Execution cancelled'); - - expect(spawnSpy.mock.calls[0][0]).toEqual('"fooCommand"'); - // on windows we don't use killProcess but run taskkill - expect(killMock).not.toBeCalled(); - - expect(spawnSpy.mock.calls[1]).toEqual(['taskkill', ['/pid', 'pid123', '/f', '/t']]); + expect(execMock.mock.calls[0][0]).toEqual('minikube'); + expect(execMock.mock.calls[0][1]).toEqual(['version']); }); test('error: expect installBinaryToSystem to fail with a non existing binary', async () => { diff --git a/src/util.ts b/src/util.ts index 8861452..b5aae01 100644 --- a/src/util.ts +++ b/src/util.ts @@ -18,8 +18,6 @@ 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 extensionApi from '@podman-desktop/api'; import type { MinikubeInstaller } from './minikube-installer'; import * as fs from 'node:fs'; @@ -53,7 +51,7 @@ export function getMinikubePath(): string { // search if minikube is available in the path export async function detectMinikube(pathAddition: string, installer: MinikubeInstaller): Promise { try { - await runCliCommand('minikube', ['version'], { env: { PATH: getMinikubePath() } }); + await extensionApi.process.exec('minikube', ['version'], { env: { PATH: getMinikubePath() } }); return 'minikube'; } catch (e) { // ignore and try another way @@ -62,7 +60,7 @@ export async function detectMinikube(pathAddition: string, installer: MinikubeIn const assetInfo = await installer.getAssetInfo(); if (assetInfo) { try { - await runCliCommand(assetInfo.name, ['version'], { + await extensionApi.process.exec(assetInfo.name, ['version'], { env: { PATH: getMinikubePath().concat(path.delimiter).concat(pathAddition) }, }); return pathAddition @@ -75,88 +73,6 @@ export async function detectMinikube(pathAddition: string, installer: MinikubeIn return undefined; } -export function runCliCommand( - command: string, - args: string[], - options?: RunOptions, - token?: extensionApi.CancellationToken, -): Promise { - return new Promise((resolve, reject) => { - let stdOut = ''; - let stdErr = ''; - let err = ''; - let env = Object.assign({}, process.env); // clone original env object - - // In production mode, applications don't have access to the 'user' path like brew - if (extensionApi.env.isMac || extensionApi.env.isWindows) { - env.PATH = getMinikubePath(); - if (extensionApi.env.isWindows) { - // Escape any whitespaces in command - command = `"${command}"`; - } - } else if (env.FLATPAK_ID) { - // need to execute the command on the host - args = ['--host', command, ...args]; - command = 'flatpak-spawn'; - } - - if (options?.env) { - env = Object.assign(env, options.env); - } - - const spawnProcess = spawn(command, args, { shell: extensionApi.env.isWindows, env }); - // if the token is cancelled, kill the process and reject the promise - token?.onCancellationRequested(() => { - killProcess(spawnProcess); - options?.logger?.error('Execution cancelled'); - // reject the promise - reject(new Error('Execution cancelled')); - }); - // do not reject as we want to store exit code in the result - spawnProcess.on('error', error => { - if (options?.logger) { - options.logger.error(error); - } - stdErr += error; - err += error; - }); - - spawnProcess.stdout.setEncoding('utf8'); - spawnProcess.stdout.on('data', data => { - if (options?.logger) { - options.logger.log(data); - } - stdOut += data; - }); - spawnProcess.stderr.setEncoding('utf8'); - spawnProcess.stderr.on('data', data => { - if (args?.[0] === 'create' || args?.[0] === 'delete') { - if (options?.logger) { - options.logger.log(data); - } - if (typeof data === 'string' && data.indexOf('error') >= 0) { - stdErr += data; - } else { - stdOut += data; - } - } else { - stdErr += data; - } - }); - - spawnProcess.on('close', exitCode => { - if (exitCode == 0) { - resolve({ stdOut, stdErr, error: err }); - } else { - if (options?.logger) { - options.logger.error(stdErr); - } - reject(new Error(stdErr)); - } - }); - }); -} - // Takes a binary path (e.g. /tmp/minikube) and installs it to the system. Renames it based on binaryName // supports Windows, Linux and macOS // If using Windows or Mac, we will use sudo-prompt in order to elevate the privileges @@ -211,11 +127,3 @@ export async function installBinaryToSystem(binaryPath: string, binaryName: stri throw error; } } - -function killProcess(spawnProcess: ChildProcess) { - if (extensionApi.env.isWindows) { - spawn('taskkill', ['/pid', spawnProcess.pid?.toString(), '/f', '/t']); - } else { - spawnProcess.kill(); - } -} diff --git a/yarn.lock b/yarn.lock index 99d544f..baf9b35 100644 --- a/yarn.lock +++ b/yarn.lock @@ -394,10 +394,10 @@ dependencies: esquery "^1.4.0" -"@podman-desktop/api@^1.6.4": - version "1.6.4" - resolved "https://registry.yarnpkg.com/@podman-desktop/api/-/api-1.6.4.tgz#f6da8228523e787f408d366f1d99d12a7b9e6924" - integrity sha512-8sxPcFvepxVM0iANq9h+QbnxAPAEE03KhrDTUp8AEzMPHZhascBSi11xhaLaDUujXVaKHUysEt0hh+0ccL479w== +"@podman-desktop/api@^1.10.0": + version "1.10.2" + resolved "https://registry.yarnpkg.com/@podman-desktop/api/-/api-1.10.2.tgz#5e528318848d901beeeb3eaf3105806e2b72f9ac" + integrity sha512-QcfzlQXBXA2p3wAHpt0m2ss5x4vQVx8H5i+u0aAgdYm3BHoJBr/Xq7Hvnd+Vqypz5AogUPYjejJ9o3UMx1OS3w== "@rollup/rollup-android-arm-eabi@4.9.6": version "4.9.6"