diff --git a/.changeset/strong-students-beg.md b/.changeset/strong-students-beg.md new file mode 100644 index 0000000000..5b3653d389 --- /dev/null +++ b/.changeset/strong-students-beg.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-node': patch +--- + +Deprecate the `logStream` option in `executeShellCommand`, replacing it with a logger instance. diff --git a/plugins/scaffolder-node/report.api.md b/plugins/scaffolder-node/report.api.md index 1141f0ada0..0afd6dc202 100644 --- a/plugins/scaffolder-node/report.api.md +++ b/plugins/scaffolder-node/report.api.md @@ -9,6 +9,7 @@ import { BackstageCredentials } from '@backstage/backend-plugin-api'; import { JsonObject } from '@backstage/types'; import { JsonValue } from '@backstage/types'; import { Logger } from 'winston'; +import { LoggerService } from '@backstage/backend-plugin-api'; import { Observable } from '@backstage/types'; import { Schema } from 'jsonschema'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -194,6 +195,7 @@ export type ExecuteShellCommandOptions = { command: string; args: string[]; options?: SpawnOptionsWithoutStdio; + logger?: LoggerService; logStream?: Writable; }; diff --git a/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts new file mode 100644 index 0000000000..67951f915c --- /dev/null +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts @@ -0,0 +1,141 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { LoggerService } from '@backstage/backend-plugin-api'; +import { spawn } from 'child_process'; +import { PassThrough, Writable } from 'stream'; +import { executeShellCommand } from './executeShellCommand'; + +jest.mock('child_process', () => ({ + spawn: jest.fn(), +})); + +describe('executeShellCommand', () => { + let mockSpawn: jest.Mock; + let mockProcess: any; + let mockLogger: jest.Mocked; + let mockLogStream: Writable; + + beforeEach(() => { + mockSpawn = spawn as jest.Mock; + mockProcess = { + stdout: new PassThrough(), + stderr: new PassThrough(), + on: jest.fn((event: string, callback: (code: number) => void) => { + if (event === 'close') { + callback(0); + } + }), + }; + mockSpawn.mockReturnValue(mockProcess); + + mockLogStream = new PassThrough(); + + mockLogger = { + info: jest.fn(), + error: jest.fn(), + } as unknown as jest.Mocked; + jest.clearAllMocks(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should execute without logger or logStream', async () => { + await executeShellCommand({ + command: 'echo', + args: ['Hello World'], + }); + + expect(mockSpawn).toHaveBeenCalledWith('echo', ['Hello World'], undefined); + }); + + it('should execute with logger but no logStream', async () => { + const logStreamSpy = jest.spyOn(mockLogStream, 'write'); + await executeShellCommand({ + command: 'echo', + args: ['Hello World'], + logger: mockLogger, + }); + + // Simulate command output + mockProcess.stdout.emit('data', Buffer.from('Hello World\n')); + + mockProcess.on('close', (code: any) => { + expect(code).toBe(0); + expect(logStreamSpy).not.toHaveBeenCalled(); + expect(mockLogger.info).toHaveBeenCalledWith('Hello World'); + expect(mockLogger.error).not.toHaveBeenCalledWith(expect.anything()); + }); + }); + + it('should execute with logStream but no logger', async () => { + const logStreamSpy = jest.spyOn(mockLogStream, 'write'); + + await executeShellCommand({ + command: 'echo', + args: ['Hello World'], + logStream: mockLogStream, + }); + + mockProcess.stdout.emit('data', Buffer.from('Hello World\n')); + mockProcess.stderr.emit('data', Buffer.from('Command not found\n')); + + mockProcess.on('close', () => { + expect(logStreamSpy).toHaveBeenCalledWith(expect.any(Buffer)); + expect(logStreamSpy).toHaveBeenCalledTimes(2); + expect(mockLogger.info).not.toHaveBeenCalled(); + }); + }); + + it('should execute with both logger and logStream', async () => { + const logStreamSpy = jest.spyOn(mockLogStream, 'write'); + + await executeShellCommand({ + command: 'echo', + args: ['Hello World'], + logger: mockLogger, + logStream: mockLogStream, + }); + + mockProcess.stdout.emit('data', Buffer.from('Hello World\n')); + mockProcess.stderr.emit('data', Buffer.from('Command not found\n')); + + mockProcess.on('close', () => { + expect(mockLogger.info).toHaveBeenCalledWith('Hello World'); + expect(mockLogger.error).toHaveBeenCalledWith('Command not found'); + expect(logStreamSpy).toHaveBeenCalledTimes(2); + }); + }); + + it('should handle non-zero exit code', async () => { + mockProcess.on.mockImplementation( + (event: string, callback: (arg0: number) => void) => { + if (event === 'close') { + callback(1); // Simulate command failing + } + }, + ); + + await expect( + executeShellCommand({ + command: 'echo', + args: ['Hello World'], + logger: mockLogger, + }), + ).rejects.toThrow('Command echo failed, exit code: 1'); + }); +}); diff --git a/plugins/scaffolder-node/src/actions/executeShellCommand.ts b/plugins/scaffolder-node/src/actions/executeShellCommand.ts index ce54989589..1d45d74327 100644 --- a/plugins/scaffolder-node/src/actions/executeShellCommand.ts +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { LoggerService } from '@backstage/backend-plugin-api'; import { spawn, SpawnOptionsWithoutStdio } from 'child_process'; import { PassThrough, Writable } from 'stream'; @@ -29,7 +30,12 @@ export type ExecuteShellCommandOptions = { args: string[]; /** options to pass to spawn */ options?: SpawnOptionsWithoutStdio; - /** stream to capture stdout and stderr output */ + /** logger to capture stdout and stderr output */ + logger?: LoggerService; + /** + * stream to capture stdout and stderr output + * @deprecated please provide a logger instead. + */ logStream?: Writable; }; @@ -45,20 +51,25 @@ export async function executeShellCommand( command, args, options: spawnOptions, + logger, logStream = new PassThrough(), } = options; await new Promise((resolve, reject) => { const process = spawn(command, args, spawnOptions); - process.stdout.on('data', stream => { - logStream.write(stream); + process.stdout.on('data', chunk => { + logStream?.write(chunk); + logger?.info( + Buffer.isBuffer(chunk) ? chunk.toString('utf8').trim() : chunk.trim(), + ); }); - - process.stderr.on('data', stream => { - logStream.write(stream); + process.stderr.on('data', chunk => { + logStream?.write(chunk); + logger?.error( + Buffer.isBuffer(chunk) ? chunk.toString('utf8').trim() : chunk.trim(), + ); }); - process.on('error', error => { return reject(error); });