From d10337b93529cd81eb5118451c480f5321958dbf Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Mon, 7 Oct 2024 23:12:55 +0200 Subject: [PATCH 01/14] feat: deprecate logStream, add logger Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/executeShellCommand.ts | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-node/src/actions/executeShellCommand.ts b/plugins/scaffolder-node/src/actions/executeShellCommand.ts index ce54989589..cb87330f9a 100644 --- a/plugins/scaffolder-node/src/actions/executeShellCommand.ts +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.ts @@ -16,6 +16,7 @@ import { spawn, SpawnOptionsWithoutStdio } from 'child_process'; import { PassThrough, Writable } from 'stream'; +import { Logger } from 'winston'; /** * Options for {@link executeShellCommand}. @@ -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?: Logger; + /** + * stream to capture stdout and stderr output + * @deprecated please provide a logger instead. + */ logStream?: Writable; }; @@ -45,20 +51,27 @@ 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?.log( + '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?.log( + 'error', + Buffer.isBuffer(chunk) ? chunk.toString('utf8').trim() : chunk.trim(), + ); }); - process.on('error', error => { return reject(error); }); From a771c791d6f4ea7c941f8354213756397a2c7acd Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Mon, 7 Oct 2024 23:14:20 +0200 Subject: [PATCH 02/14] feat: fix actions Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/fetch/cookiecutter.test.ts | 18 ++++---- .../src/actions/fetch/cookiecutter.ts | 29 +++++------- .../src/actions/fetch/rails/index.test.ts | 14 +++--- .../src/actions/fetch/rails/index.ts | 18 +++----- .../fetch/rails/railsNewRunner.test.ts | 44 +++++++++---------- .../src/actions/fetch/rails/railsNewRunner.ts | 16 +++---- 6 files changed, 64 insertions(+), 75 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts index 52946ce7fd..c88c7cde54 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts @@ -15,16 +15,16 @@ */ import { ContainerRunner } from '@backstage/backend-common'; -import { ConfigReader } from '@backstage/config'; -import { JsonObject } from '@backstage/types'; -import { ScmIntegrations } from '@backstage/integration'; +import { UrlReaderService } from '@backstage/backend-plugin-api'; import { createMockDirectory } from '@backstage/backend-test-utils'; -import { createFetchCookiecutterAction } from './cookiecutter'; -import { join } from 'path'; +import { ConfigReader } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; import type { ActionContext } from '@backstage/plugin-scaffolder-node'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; -import { Writable } from 'stream'; -import { UrlReaderService } from '@backstage/backend-plugin-api'; +import { JsonObject } from '@backstage/types'; +import { join } from 'path'; +import { Logger } from 'winston'; +import { createFetchCookiecutterAction } from './cookiecutter'; const executeShellCommand = jest.fn(); const commandExists = jest.fn(); @@ -168,7 +168,7 @@ describe('fetch:cookiecutter', () => { join(mockTmpDir, 'template'), '--verbose', ], - logStream: expect.any(Writable), + logger: expect.any(Logger), }), ); }); @@ -189,7 +189,7 @@ describe('fetch:cookiecutter', () => { }, workingDir: '/input', envVars: { HOME: '/tmp' }, - logStream: expect.any(Writable), + logger: expect.any(Logger), }), ); }); diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts index 6e540d7fca..1b61376a31 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts @@ -19,18 +19,18 @@ import { UrlReaderService, resolveSafeChildPath, } from '@backstage/backend-plugin-api'; -import { JsonObject, JsonValue } from '@backstage/types'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; +import { + createTemplateAction, + executeShellCommand, + fetchContents, +} from '@backstage/plugin-scaffolder-node'; +import { JsonObject, JsonValue } from '@backstage/types'; import commandExists from 'command-exists'; import fs from 'fs-extra'; import path, { resolve as resolvePath } from 'path'; -import { PassThrough, Writable } from 'stream'; -import { - createTemplateAction, - fetchContents, - executeShellCommand, -} from '@backstage/plugin-scaffolder-node'; +import { Logger } from 'winston'; import { examples } from './cookiecutter.examples'; export class CookiecutterRunner { @@ -57,14 +57,14 @@ export class CookiecutterRunner { public async run({ workspacePath, values, - logStream, + logger, imageName, templateDir, templateContentsDir, }: { workspacePath: string; values: JsonObject; - logStream: Writable; + logger: Logger; imageName?: string; templateDir: string; templateContentsDir: string; @@ -99,7 +99,7 @@ export class CookiecutterRunner { await executeShellCommand({ command: 'cookiecutter', args: ['--no-input', '-o', intermediateDir, templateDir, '--verbose'], - logStream, + logger, }); } else { if (this.containerRunner === undefined) { @@ -116,7 +116,7 @@ export class CookiecutterRunner { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, - logStream, + logger, }); } @@ -247,15 +247,10 @@ export function createFetchCookiecutterAction(options: { _extensions: ctx.input.extensions, }; - const logStream = new PassThrough(); - logStream.on('data', chunk => { - ctx.logger.info(chunk.toString()); - }); - // Will execute the template in ./template and put the result in ./result await cookiecutter.run({ workspacePath: workDir, - logStream, + logger: ctx.logger, values: values, imageName: ctx.input.imageName, templateDir: templateDir, diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts index 616c90e0de..3ea3e01bef 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts @@ -28,15 +28,15 @@ jest.mock('./railsNewRunner', () => { }); import { ContainerRunner } from '@backstage/backend-common'; +import { UrlReaderService } from '@backstage/backend-plugin-api'; +import { createMockDirectory } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; -import { resolve as resolvePath } from 'path'; -import { createFetchRailsAction } from './index'; import { fetchContents } from '@backstage/plugin-scaffolder-node'; -import { createMockDirectory } from '@backstage/backend-test-utils'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; -import { Writable } from 'stream'; -import { UrlReaderService } from '@backstage/backend-plugin-api'; +import { resolve as resolvePath } from 'path'; +import { Logger } from 'winston'; +import { createFetchRailsAction } from './index'; describe('fetch:rails', () => { const mockDir = createMockDirectory(); @@ -106,7 +106,7 @@ describe('fetch:rails', () => { expect(mockRailsTemplater.run).toHaveBeenCalledWith({ workspacePath: mockContext.workspacePath, - logStream: expect.any(Writable), + logger: expect.any(Logger), values: mockContext.input.values, }); }); @@ -122,7 +122,7 @@ describe('fetch:rails', () => { expect(mockRailsTemplater.run).toHaveBeenCalledWith({ workspacePath: mockContext.workspacePath, - logStream: expect.any(Writable), + logger: expect.any(Logger), values: { ...mockContext.input.values, imageName: 'foo/rails-custom-image', diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts index 4d5db0f3ef..223aeb19a2 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts @@ -15,20 +15,19 @@ */ import { ContainerRunner } from '@backstage/backend-common'; -import { JsonObject } from '@backstage/types'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; -import fs from 'fs-extra'; import { createTemplateAction, fetchContents, } from '@backstage/plugin-scaffolder-node'; +import { JsonObject } from '@backstage/types'; +import fs from 'fs-extra'; -import { resolve as resolvePath } from 'path'; -import { RailsNewRunner } from './railsNewRunner'; -import { PassThrough } from 'stream'; -import { examples } from './index.examples'; import { UrlReaderService } from '@backstage/backend-plugin-api'; +import { resolve as resolvePath } from 'path'; +import { examples } from './index.examples'; +import { RailsNewRunner } from './railsNewRunner'; /** * Creates the `fetch:rails` Scaffolder action. @@ -219,15 +218,10 @@ export function createFetchRailsAction(options: { throw new Error(`Image ${imageName} is not allowed`); } - const logStream = new PassThrough(); - logStream.on('data', chunk => { - ctx.logger.info(chunk.toString()); - }); - // Will execute the template in ./template and put the result in ./result await templateRunner.run({ workspacePath: workDir, - logStream, + logger: ctx.logger, values: { ...ctx.input.values, imageName }, }); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts index 47104c66e1..2fd1b954c2 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts @@ -28,10 +28,10 @@ jest.mock( ); import { ContainerRunner } from '@backstage/backend-common'; -import path from 'path'; -import { PassThrough } from 'stream'; -import { RailsNewRunner } from './railsNewRunner'; import { createMockDirectory } from '@backstage/backend-test-utils'; +import path from 'path'; +import { Logger } from 'winston'; +import { RailsNewRunner } from './railsNewRunner'; describe('Rails Templater', () => { const containerRunner: jest.Mocked = { @@ -47,7 +47,7 @@ describe('Rails Templater', () => { describe('when running on docker', () => { it('should run the correct bindings for the volumes', async () => { - const logStream = new PassThrough(); + const logger = new Logger(); const values = { owner: 'angeliski', storePath: 'https://github.com/angeliski/rails-project', @@ -65,7 +65,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logStream, + logger, }); expect(containerRunner.runContainer).toHaveBeenCalledWith({ @@ -78,12 +78,12 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', - logStream: logStream, + logger: logger, }); }); it('should use the provided imageName', async () => { - const logStream = new PassThrough(); + const logger = new Logger(); const values = { owner: 'angeliski', storePath: 'https://github.com/angeliski/rails-project', @@ -101,7 +101,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logStream, + logger, }); expect(containerRunner.runContainer).toHaveBeenCalledWith( @@ -112,7 +112,7 @@ describe('Rails Templater', () => { }); it('should pass through the streamer to the run docker helper', async () => { - const stream = new PassThrough(); + const logger = new Logger(); const values = { owner: 'angeliski', @@ -131,7 +131,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logStream: stream, + logger, }); expect(containerRunner.runContainer).toHaveBeenCalledWith({ @@ -144,12 +144,12 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', - logStream: stream, + logger, }); }); it('update the template path to correct location', async () => { - const logStream = new PassThrough(); + const logger = new Logger(); const values = { owner: 'angeliski', storePath: 'https://github.com/angeliski/rails-project', @@ -168,7 +168,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logStream, + logger, }); expect(containerRunner.runContainer).toHaveBeenCalledWith({ @@ -186,14 +186,14 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', - logStream: logStream, + logger, }); }); }); describe('when rails is available', () => { it('use the binary', async () => { - const stream = new PassThrough(); + const logger = new Logger(); const values = { owner: 'angeliski', @@ -213,7 +213,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logStream: stream, + logger, }); expect(executeShellCommand).toHaveBeenCalledWith({ @@ -222,12 +222,12 @@ describe('Rails Templater', () => { 'new', path.join(mockDir.path, 'intermediate', 'rails-project'), ]), - logStream: stream, + logger, }); }); it('update the template path to correct location', async () => { - const stream = new PassThrough(); + const logger = new Logger(); const values = { owner: 'angeliski', @@ -248,7 +248,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logStream: stream, + logger, }); expect(executeShellCommand).toHaveBeenCalledWith({ @@ -259,14 +259,14 @@ describe('Rails Templater', () => { '--template', path.join(mockDir.path, './something.rb'), ]), - logStream: stream, + logger, }); }); }); describe('when nothing was generated', () => { it('throws an error', async () => { - const stream = new PassThrough(); + const logger = new Logger(); mockDir.setContent({ intermediate: {}, @@ -282,7 +282,7 @@ describe('Rails Templater', () => { name: 'rails-project', imageName: 'foo/rails-custom-image', }, - logStream: stream, + logger, }), ).rejects.toThrow(/No data generated by rails/); }); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts index 31cfea4a76..386e7b427f 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts @@ -15,16 +15,16 @@ */ import { ContainerRunner } from '@backstage/backend-common'; +import { executeShellCommand } from '@backstage/plugin-scaffolder-node'; +import { JsonObject } from '@backstage/types'; +import commandExists from 'command-exists'; import fs from 'fs-extra'; import path from 'path'; -import { executeShellCommand } from '@backstage/plugin-scaffolder-node'; -import commandExists from 'command-exists'; +import { Logger } from 'winston'; import { railsArgumentResolver, RailsRunOptions, } from './railsArgumentResolver'; -import { JsonObject } from '@backstage/types'; -import { Writable } from 'stream'; export class RailsNewRunner { private readonly containerRunner?: ContainerRunner; @@ -36,11 +36,11 @@ export class RailsNewRunner { public async run({ workspacePath, values, - logStream, + logger, }: { workspacePath: string; values: JsonObject; - logStream: Writable; + logger: Logger; }): Promise { const intermediateDir = path.join(workspacePath, 'intermediate'); await fs.ensureDir(intermediateDir); @@ -71,7 +71,7 @@ export class RailsNewRunner { `${intermediateDir}${path.sep}${name}`, ...arrayExtraArguments, ], - logStream, + logger, }); } else { if (!imageName) { @@ -96,7 +96,7 @@ export class RailsNewRunner { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, - logStream, + logger, }); } From 16e339ea2665442daf1d14e8784d904748cfb644 Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Mon, 7 Oct 2024 23:14:59 +0200 Subject: [PATCH 03/14] feat: add api-report, changesets and package files Signed-off-by: ElaineDeMattosSilvaB --- .changeset/violet-seas-pretend.md | 7 +++++++ .../scaffolder-backend-module-cookiecutter/report.api.md | 2 -- plugins/scaffolder-backend-module-rails/package.json | 1 + plugins/scaffolder-node/report.api.md | 1 + yarn.lock | 1 + 5 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 .changeset/violet-seas-pretend.md diff --git a/.changeset/violet-seas-pretend.md b/.changeset/violet-seas-pretend.md new file mode 100644 index 0000000000..09774f0af8 --- /dev/null +++ b/.changeset/violet-seas-pretend.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-node': minor +'@backstage/plugin-scaffolder-backend-module-cookiecutter': patch +'@backstage/plugin-scaffolder-backend-module-rails': patch +--- + +Deprecate the `logStream` option in `executeShellCommand`, replacing it with a logger instance. diff --git a/plugins/scaffolder-backend-module-cookiecutter/report.api.md b/plugins/scaffolder-backend-module-cookiecutter/report.api.md index 19f43389dc..316dc6b4a8 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/report.api.md +++ b/plugins/scaffolder-backend-module-cookiecutter/report.api.md @@ -3,8 +3,6 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts -/// - import { BackendFeature } from '@backstage/backend-plugin-api'; import { ContainerRunner } from '@backstage/backend-common'; import { JsonObject } from '@backstage/types'; diff --git a/plugins/scaffolder-backend-module-rails/package.json b/plugins/scaffolder-backend-module-rails/package.json index 56cd2c0451..62929c41f7 100644 --- a/plugins/scaffolder-backend-module-rails/package.json +++ b/plugins/scaffolder-backend-module-rails/package.json @@ -52,6 +52,7 @@ "@backstage/types": "workspace:^", "command-exists": "^1.2.9", "fs-extra": "^11.0.0", + "winston": "^3.2.1", "yaml": "^2.0.0" }, "devDependencies": { diff --git a/plugins/scaffolder-node/report.api.md b/plugins/scaffolder-node/report.api.md index c6cabda02c..26735e18de 100644 --- a/plugins/scaffolder-node/report.api.md +++ b/plugins/scaffolder-node/report.api.md @@ -191,6 +191,7 @@ export type ExecuteShellCommandOptions = { command: string; args: string[]; options?: SpawnOptionsWithoutStdio; + logger?: Logger; logStream?: Writable; }; diff --git a/yarn.lock b/yarn.lock index cd65c170d6..7348496686 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7450,6 +7450,7 @@ __metadata: command-exists: ^1.2.9 fs-extra: ^11.0.0 jest-when: ^3.1.0 + winston: ^3.2.1 yaml: ^2.0.0 languageName: unknown linkType: soft From 9655c15815c7be7cd31efbfee522bb9a73c1890f Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 15 Oct 2024 13:32:55 +0200 Subject: [PATCH 04/14] fix: remove logger from runContainer Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/fetch/cookiecutter.test.ts | 1 - .../src/actions/fetch/cookiecutter.ts | 1 - .../src/actions/fetch/rails/railsNewRunner.test.ts | 3 --- .../src/actions/fetch/rails/railsNewRunner.ts | 1 - 4 files changed, 6 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts index c88c7cde54..bb47f3e19a 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts @@ -189,7 +189,6 @@ describe('fetch:cookiecutter', () => { }, workingDir: '/input', envVars: { HOME: '/tmp' }, - logger: expect.any(Logger), }), ); }); diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts index 1b61376a31..5f7f91daab 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts @@ -116,7 +116,6 @@ export class CookiecutterRunner { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, - logger, }); } diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts index 2fd1b954c2..3abbd24ace 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts @@ -78,7 +78,6 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', - logger: logger, }); }); @@ -144,7 +143,6 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', - logger, }); }); @@ -186,7 +184,6 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', - logger, }); }); }); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts index 386e7b427f..906c2e6404 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts @@ -96,7 +96,6 @@ export class RailsNewRunner { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, - logger, }); } From bfc374068ed6a5c011b5759c2695aa75a1528bba Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 15 Oct 2024 21:33:12 +0200 Subject: [PATCH 05/14] feat: add tests Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/executeShellCommand.test.ts | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 plugins/scaffolder-node/src/actions/executeShellCommand.test.ts 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..91ee2c52fd --- /dev/null +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts @@ -0,0 +1,147 @@ +/* + * 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. + */ + +// Copyright 2024 DB Systel GmbH +// Licensed under the DBISL, see the accompanying file LICENSE. + +import { spawn } from 'child_process'; +import { PassThrough, Writable } from 'stream'; +import { Logger } from 'winston'; +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 = { + log: 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.log).toHaveBeenCalledWith('info', 'Hello World'); + expect(mockLogger.log).not.toHaveBeenCalledWith( + 'error', + 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.log).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.log).toHaveBeenCalledWith('info', 'Hello World'); + expect(mockLogger.log).toHaveBeenCalledWith('error', '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'); + }); +}); From 346ba744896a2d28c7e7a9a968730049314b8349 Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 15 Oct 2024 21:53:08 +0200 Subject: [PATCH 06/14] fix: remove logger from cookiecutter and rails Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/fetch/cookiecutter.test.ts | 5 ++- .../src/actions/fetch/cookiecutter.ts | 16 +++++--- .../src/actions/fetch/rails/index.test.ts | 6 +-- .../src/actions/fetch/rails/index.ts | 8 +++- .../fetch/rails/railsNewRunner.test.ts | 37 ++++++++++--------- .../src/actions/fetch/rails/railsNewRunner.ts | 9 +++-- 6 files changed, 49 insertions(+), 32 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts index bb47f3e19a..ce8a34a6ed 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts @@ -23,7 +23,7 @@ import type { ActionContext } from '@backstage/plugin-scaffolder-node'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; import { JsonObject } from '@backstage/types'; import { join } from 'path'; -import { Logger } from 'winston'; +import { Writable } from 'stream'; import { createFetchCookiecutterAction } from './cookiecutter'; const executeShellCommand = jest.fn(); @@ -168,7 +168,7 @@ describe('fetch:cookiecutter', () => { join(mockTmpDir, 'template'), '--verbose', ], - logger: expect.any(Logger), + logStream: expect.any(Writable), }), ); }); @@ -189,6 +189,7 @@ describe('fetch:cookiecutter', () => { }, workingDir: '/input', envVars: { HOME: '/tmp' }, + logStream: expect.any(Writable), }), ); }); diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts index 5f7f91daab..269630817c 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts @@ -30,7 +30,7 @@ import { JsonObject, JsonValue } from '@backstage/types'; import commandExists from 'command-exists'; import fs from 'fs-extra'; import path, { resolve as resolvePath } from 'path'; -import { Logger } from 'winston'; +import { PassThrough, Writable } from 'stream'; import { examples } from './cookiecutter.examples'; export class CookiecutterRunner { @@ -57,14 +57,14 @@ export class CookiecutterRunner { public async run({ workspacePath, values, - logger, + logStream, imageName, templateDir, templateContentsDir, }: { workspacePath: string; values: JsonObject; - logger: Logger; + logStream: Writable; imageName?: string; templateDir: string; templateContentsDir: string; @@ -99,7 +99,7 @@ export class CookiecutterRunner { await executeShellCommand({ command: 'cookiecutter', args: ['--no-input', '-o', intermediateDir, templateDir, '--verbose'], - logger, + logStream, }); } else { if (this.containerRunner === undefined) { @@ -116,6 +116,7 @@ export class CookiecutterRunner { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, + logStream, }); } @@ -246,10 +247,15 @@ export function createFetchCookiecutterAction(options: { _extensions: ctx.input.extensions, }; + const logStream = new PassThrough(); + logStream.on('data', chunk => { + ctx.logger.info(chunk.toString()); + }); + // Will execute the template in ./template and put the result in ./result await cookiecutter.run({ workspacePath: workDir, - logger: ctx.logger, + logStream, values: values, imageName: ctx.input.imageName, templateDir: templateDir, diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts index 3ea3e01bef..c2d3ead6bb 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts @@ -35,7 +35,7 @@ import { ScmIntegrations } from '@backstage/integration'; import { fetchContents } from '@backstage/plugin-scaffolder-node'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; import { resolve as resolvePath } from 'path'; -import { Logger } from 'winston'; +import { Writable } from 'stream'; import { createFetchRailsAction } from './index'; describe('fetch:rails', () => { @@ -106,7 +106,7 @@ describe('fetch:rails', () => { expect(mockRailsTemplater.run).toHaveBeenCalledWith({ workspacePath: mockContext.workspacePath, - logger: expect.any(Logger), + logStream: expect.any(Writable), values: mockContext.input.values, }); }); @@ -122,7 +122,7 @@ describe('fetch:rails', () => { expect(mockRailsTemplater.run).toHaveBeenCalledWith({ workspacePath: mockContext.workspacePath, - logger: expect.any(Logger), + logStream: expect.any(Writable), values: { ...mockContext.input.values, imageName: 'foo/rails-custom-image', diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts index 223aeb19a2..a063ce0a5b 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts @@ -26,6 +26,7 @@ import fs from 'fs-extra'; import { UrlReaderService } from '@backstage/backend-plugin-api'; import { resolve as resolvePath } from 'path'; +import { PassThrough } from 'stream'; import { examples } from './index.examples'; import { RailsNewRunner } from './railsNewRunner'; @@ -218,10 +219,15 @@ export function createFetchRailsAction(options: { throw new Error(`Image ${imageName} is not allowed`); } + const logStream = new PassThrough(); + logStream.on('data', chunk => { + ctx.logger.info(chunk.toString()); + }); + // Will execute the template in ./template and put the result in ./result await templateRunner.run({ workspacePath: workDir, - logger: ctx.logger, + logStream, values: { ...ctx.input.values, imageName }, }); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts index 3abbd24ace..c53f368516 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts @@ -30,7 +30,7 @@ jest.mock( import { ContainerRunner } from '@backstage/backend-common'; import { createMockDirectory } from '@backstage/backend-test-utils'; import path from 'path'; -import { Logger } from 'winston'; +import { PassThrough } from 'stream'; import { RailsNewRunner } from './railsNewRunner'; describe('Rails Templater', () => { @@ -47,7 +47,7 @@ describe('Rails Templater', () => { describe('when running on docker', () => { it('should run the correct bindings for the volumes', async () => { - const logger = new Logger(); + const logStream = new PassThrough(); const values = { owner: 'angeliski', storePath: 'https://github.com/angeliski/rails-project', @@ -65,7 +65,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logger, + logStream, }); expect(containerRunner.runContainer).toHaveBeenCalledWith({ @@ -78,11 +78,12 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', + logStream: logStream, }); }); it('should use the provided imageName', async () => { - const logger = new Logger(); + const logStream = new PassThrough(); const values = { owner: 'angeliski', storePath: 'https://github.com/angeliski/rails-project', @@ -100,7 +101,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logger, + logStream, }); expect(containerRunner.runContainer).toHaveBeenCalledWith( @@ -111,7 +112,7 @@ describe('Rails Templater', () => { }); it('should pass through the streamer to the run docker helper', async () => { - const logger = new Logger(); + const stream = new PassThrough(); const values = { owner: 'angeliski', @@ -130,7 +131,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logger, + logStream: stream, }); expect(containerRunner.runContainer).toHaveBeenCalledWith({ @@ -143,11 +144,12 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', + logStream: stream, }); }); it('update the template path to correct location', async () => { - const logger = new Logger(); + const logStream = new PassThrough(); const values = { owner: 'angeliski', storePath: 'https://github.com/angeliski/rails-project', @@ -166,7 +168,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logger, + logStream, }); expect(containerRunner.runContainer).toHaveBeenCalledWith({ @@ -184,13 +186,14 @@ describe('Rails Templater', () => { [path.join(mockDir.path, 'intermediate')]: '/output', }, workingDir: '/input', + logStream: logStream, }); }); }); describe('when rails is available', () => { it('use the binary', async () => { - const logger = new Logger(); + const stream = new PassThrough(); const values = { owner: 'angeliski', @@ -210,7 +213,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logger, + logStream: stream, }); expect(executeShellCommand).toHaveBeenCalledWith({ @@ -219,12 +222,12 @@ describe('Rails Templater', () => { 'new', path.join(mockDir.path, 'intermediate', 'rails-project'), ]), - logger, + logStream: stream, }); }); it('update the template path to correct location', async () => { - const logger = new Logger(); + const stream = new PassThrough(); const values = { owner: 'angeliski', @@ -245,7 +248,7 @@ describe('Rails Templater', () => { await templater.run({ workspacePath: mockDir.path, values, - logger, + logStream: stream, }); expect(executeShellCommand).toHaveBeenCalledWith({ @@ -256,14 +259,14 @@ describe('Rails Templater', () => { '--template', path.join(mockDir.path, './something.rb'), ]), - logger, + logStream: stream, }); }); }); describe('when nothing was generated', () => { it('throws an error', async () => { - const logger = new Logger(); + const stream = new PassThrough(); mockDir.setContent({ intermediate: {}, @@ -279,7 +282,7 @@ describe('Rails Templater', () => { name: 'rails-project', imageName: 'foo/rails-custom-image', }, - logger, + logStream: stream, }), ).rejects.toThrow(/No data generated by rails/); }); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts index 906c2e6404..fe2470eef8 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts @@ -20,7 +20,7 @@ import { JsonObject } from '@backstage/types'; import commandExists from 'command-exists'; import fs from 'fs-extra'; import path from 'path'; -import { Logger } from 'winston'; +import { Writable } from 'stream'; import { railsArgumentResolver, RailsRunOptions, @@ -36,11 +36,11 @@ export class RailsNewRunner { public async run({ workspacePath, values, - logger, + logStream, }: { workspacePath: string; values: JsonObject; - logger: Logger; + logStream: Writable; }): Promise { const intermediateDir = path.join(workspacePath, 'intermediate'); await fs.ensureDir(intermediateDir); @@ -71,7 +71,7 @@ export class RailsNewRunner { `${intermediateDir}${path.sep}${name}`, ...arrayExtraArguments, ], - logger, + logStream, }); } else { if (!imageName) { @@ -96,6 +96,7 @@ export class RailsNewRunner { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, + logStream, }); } From a6795414641bd34bc0eb694e835e3fae9fe5759f Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 15 Oct 2024 22:01:49 +0200 Subject: [PATCH 07/14] fix: remove winston logger from the rails package.json Signed-off-by: ElaineDeMattosSilvaB --- .../report.api.md | 2 + .../package.json | 83 +++++++++---------- yarn.lock | 1 - 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/report.api.md b/plugins/scaffolder-backend-module-cookiecutter/report.api.md index 316dc6b4a8..19f43389dc 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/report.api.md +++ b/plugins/scaffolder-backend-module-cookiecutter/report.api.md @@ -3,6 +3,8 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +/// + import { BackendFeature } from '@backstage/backend-plugin-api'; import { ContainerRunner } from '@backstage/backend-common'; import { JsonObject } from '@backstage/types'; diff --git a/plugins/scaffolder-backend-module-rails/package.json b/plugins/scaffolder-backend-module-rails/package.json index 62f688b5c0..9b6e5b87be 100644 --- a/plugins/scaffolder-backend-module-rails/package.json +++ b/plugins/scaffolder-backend-module-rails/package.json @@ -1,46 +1,8 @@ { - "name": "@backstage/plugin-scaffolder-backend-module-rails", - "version": "0.5.1-next.2", - "description": "A module for the scaffolder backend that lets you template projects using Rails", "backstage": { - "role": "backend-plugin-module", "pluginId": "scaffolder", - "pluginPackage": "@backstage/plugin-scaffolder-backend" - }, - "publishConfig": { - "access": "public" - }, - "homepage": "https://backstage.io", - "repository": { - "type": "git", - "url": "https://github.com/backstage/backstage", - "directory": "plugins/scaffolder-backend-module-rails" - }, - "license": "Apache-2.0", - "exports": { - ".": "./src/index.ts", - "./package.json": "./package.json" - }, - "main": "src/index.ts", - "types": "src/index.ts", - "typesVersions": { - "*": { - "package.json": [ - "package.json" - ] - } - }, - "files": [ - "dist" - ], - "scripts": { - "build": "backstage-cli package build", - "clean": "backstage-cli package clean", - "lint": "backstage-cli package lint", - "prepack": "backstage-cli package prepack", - "postpack": "backstage-cli package postpack", - "start": "backstage-cli package start", - "test": "backstage-cli package test" + "pluginPackage": "@backstage/plugin-scaffolder-backend", + "role": "backend-plugin-module" }, "dependencies": { "@backstage/backend-common": "^0.25.0", @@ -52,9 +14,9 @@ "@backstage/types": "workspace:^", "command-exists": "^1.2.9", "fs-extra": "^11.0.0", - "winston": "^3.2.1", "yaml": "^2.0.0" }, + "description": "A module for the scaffolder backend that lets you template projects using Rails", "devDependencies": { "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", @@ -63,5 +25,42 @@ "@types/fs-extra": "^11.0.0", "@types/node": "^18.17.8", "jest-when": "^3.1.0" - } + }, + "exports": { + ".": "./src/index.ts", + "./package.json": "./package.json" + }, + "files": [ + "dist" + ], + "homepage": "https://backstage.io", + "license": "Apache-2.0", + "main": "src/index.ts", + "name": "@backstage/plugin-scaffolder-backend-module-rails", + "publishConfig": { + "access": "public" + }, + "repository": { + "directory": "plugins/scaffolder-backend-module-rails", + "type": "git", + "url": "https://github.com/backstage/backstage" + }, + "scripts": { + "build": "backstage-cli package build", + "clean": "backstage-cli package clean", + "lint": "backstage-cli package lint", + "postpack": "backstage-cli package postpack", + "prepack": "backstage-cli package prepack", + "start": "backstage-cli package start", + "test": "backstage-cli package test" + }, + "types": "src/index.ts", + "typesVersions": { + "*": { + "package.json": [ + "package.json" + ] + } + }, + "version": "0.5.1-next.2" } diff --git a/yarn.lock b/yarn.lock index 0d66938b6f..8bbf86bd74 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7498,7 +7498,6 @@ __metadata: command-exists: ^1.2.9 fs-extra: ^11.0.0 jest-when: ^3.1.0 - winston: ^3.2.1 yaml: ^2.0.0 languageName: unknown linkType: soft From 322de9395d425dc066a1d65989231e5f3b74b4d8 Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 15 Oct 2024 22:09:40 +0200 Subject: [PATCH 08/14] fix: package.sjon Signed-off-by: ElaineDeMattosSilvaB --- .../package.json | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/plugins/scaffolder-backend-module-rails/package.json b/plugins/scaffolder-backend-module-rails/package.json index 9b6e5b87be..188892b2e2 100644 --- a/plugins/scaffolder-backend-module-rails/package.json +++ b/plugins/scaffolder-backend-module-rails/package.json @@ -1,9 +1,47 @@ { + "name": "@backstage/plugin-scaffolder-backend-module-rails", + "version": "0.5.1", + "description": "A module for the scaffolder backend that lets you template projects using Rails", "backstage": { "pluginId": "scaffolder", "pluginPackage": "@backstage/plugin-scaffolder-backend", "role": "backend-plugin-module" }, + "publishConfig": { + "access": "public" + }, + "homepage": "https://backstage.io", + "repository": { + "type": "git", + "url": "https://github.com/backstage/backstage", + "directory": "plugins/scaffolder-backend-module-rails" + }, + "license": "Apache-2.0", + "exports": { + ".": "./src/index.ts", + "./package.json": "./package.json" + }, + "main": "src/index.ts", + "types": "src/index.ts", + "typesVersions": { + "*": { + "package.json": [ + "package.json" + ] + } + }, + "files": [ + "dist" + ], + "scripts": { + "build": "backstage-cli package build", + "clean": "backstage-cli package clean", + "lint": "backstage-cli package lint", + "prepack": "backstage-cli package prepack", + "postpack": "backstage-cli package postpack", + "start": "backstage-cli package start", + "test": "backstage-cli package test" + }, "dependencies": { "@backstage/backend-common": "^0.25.0", "@backstage/backend-plugin-api": "workspace:^", @@ -16,7 +54,6 @@ "fs-extra": "^11.0.0", "yaml": "^2.0.0" }, - "description": "A module for the scaffolder backend that lets you template projects using Rails", "devDependencies": { "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", @@ -25,42 +62,5 @@ "@types/fs-extra": "^11.0.0", "@types/node": "^18.17.8", "jest-when": "^3.1.0" - }, - "exports": { - ".": "./src/index.ts", - "./package.json": "./package.json" - }, - "files": [ - "dist" - ], - "homepage": "https://backstage.io", - "license": "Apache-2.0", - "main": "src/index.ts", - "name": "@backstage/plugin-scaffolder-backend-module-rails", - "publishConfig": { - "access": "public" - }, - "repository": { - "directory": "plugins/scaffolder-backend-module-rails", - "type": "git", - "url": "https://github.com/backstage/backstage" - }, - "scripts": { - "build": "backstage-cli package build", - "clean": "backstage-cli package clean", - "lint": "backstage-cli package lint", - "postpack": "backstage-cli package postpack", - "prepack": "backstage-cli package prepack", - "start": "backstage-cli package start", - "test": "backstage-cli package test" - }, - "types": "src/index.ts", - "typesVersions": { - "*": { - "package.json": [ - "package.json" - ] - } - }, - "version": "0.5.1-next.2" + } } From a3dbd2302fa16b783c1e6e7baa618500639f0bcf Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 22 Oct 2024 06:46:09 +0200 Subject: [PATCH 09/14] fix: change winston logger for loggerservice Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/executeShellCommand.test.ts | 22 +++++++++---------- .../src/actions/executeShellCommand.ts | 10 ++++----- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts index 91ee2c52fd..aa60f31df4 100644 --- a/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts @@ -17,9 +17,9 @@ // Copyright 2024 DB Systel GmbH // Licensed under the DBISL, see the accompanying file LICENSE. +import { LoggerService } from '@backstage/backend-plugin-api'; import { spawn } from 'child_process'; import { PassThrough, Writable } from 'stream'; -import { Logger } from 'winston'; import { executeShellCommand } from './executeShellCommand'; jest.mock('child_process', () => ({ @@ -29,7 +29,7 @@ jest.mock('child_process', () => ({ describe('executeShellCommand', () => { let mockSpawn: jest.Mock; let mockProcess: any; - let mockLogger: jest.Mocked; + let mockLogger: jest.Mocked; let mockLogStream: Writable; beforeEach(() => { @@ -48,8 +48,9 @@ describe('executeShellCommand', () => { mockLogStream = new PassThrough(); mockLogger = { - log: jest.fn(), - } as unknown as jest.Mocked; + info: jest.fn(), + error: jest.fn(), + } as unknown as jest.Mocked; jest.clearAllMocks(); }); @@ -80,11 +81,8 @@ describe('executeShellCommand', () => { mockProcess.on('close', (code: any) => { expect(code).toBe(0); expect(logStreamSpy).not.toHaveBeenCalled(); - expect(mockLogger.log).toHaveBeenCalledWith('info', 'Hello World'); - expect(mockLogger.log).not.toHaveBeenCalledWith( - 'error', - expect.anything(), - ); + expect(mockLogger.info).toHaveBeenCalledWith('Hello World'); + expect(mockLogger.error).not.toHaveBeenCalledWith(expect.anything()); }); }); @@ -103,7 +101,7 @@ describe('executeShellCommand', () => { mockProcess.on('close', () => { expect(logStreamSpy).toHaveBeenCalledWith(expect.any(Buffer)); expect(logStreamSpy).toHaveBeenCalledTimes(2); - expect(mockLogger.log).not.toHaveBeenCalled(); + expect(mockLogger.info).not.toHaveBeenCalled(); }); }); @@ -121,8 +119,8 @@ describe('executeShellCommand', () => { mockProcess.stderr.emit('data', Buffer.from('Command not found\n')); mockProcess.on('close', () => { - expect(mockLogger.log).toHaveBeenCalledWith('info', 'Hello World'); - expect(mockLogger.log).toHaveBeenCalledWith('error', 'Command not found'); + expect(mockLogger.info).toHaveBeenCalledWith('Hello World'); + expect(mockLogger.error).toHaveBeenCalledWith('Command not found'); expect(logStreamSpy).toHaveBeenCalledTimes(2); }); }); diff --git a/plugins/scaffolder-node/src/actions/executeShellCommand.ts b/plugins/scaffolder-node/src/actions/executeShellCommand.ts index cb87330f9a..1d45d74327 100644 --- a/plugins/scaffolder-node/src/actions/executeShellCommand.ts +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.ts @@ -14,9 +14,9 @@ * limitations under the License. */ +import { LoggerService } from '@backstage/backend-plugin-api'; import { spawn, SpawnOptionsWithoutStdio } from 'child_process'; import { PassThrough, Writable } from 'stream'; -import { Logger } from 'winston'; /** * Options for {@link executeShellCommand}. @@ -31,7 +31,7 @@ export type ExecuteShellCommandOptions = { /** options to pass to spawn */ options?: SpawnOptionsWithoutStdio; /** logger to capture stdout and stderr output */ - logger?: Logger; + logger?: LoggerService; /** * stream to capture stdout and stderr output * @deprecated please provide a logger instead. @@ -60,15 +60,13 @@ export async function executeShellCommand( process.stdout.on('data', chunk => { logStream?.write(chunk); - logger?.log( - 'info', + logger?.info( Buffer.isBuffer(chunk) ? chunk.toString('utf8').trim() : chunk.trim(), ); }); process.stderr.on('data', chunk => { logStream?.write(chunk); - logger?.log( - 'error', + logger?.error( Buffer.isBuffer(chunk) ? chunk.toString('utf8').trim() : chunk.trim(), ); }); From edf47addc41a4b7868a0a4d7f5e6da256d6d4c92 Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 22 Oct 2024 07:14:09 +0200 Subject: [PATCH 10/14] fix: update report.api doc Signed-off-by: ElaineDeMattosSilvaB --- plugins/scaffolder-node/report.api.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-node/report.api.md b/plugins/scaffolder-node/report.api.md index 75c964f6d6..0fa84d5407 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'; @@ -191,7 +192,7 @@ export type ExecuteShellCommandOptions = { command: string; args: string[]; options?: SpawnOptionsWithoutStdio; - logger?: Logger; + logger?: LoggerService; logStream?: Writable; }; From f847e9cb09c39651c90aeb4fb8cf0354fb108014 Mon Sep 17 00:00:00 2001 From: Elaine Mattos Date: Tue, 22 Oct 2024 11:51:31 +0200 Subject: [PATCH 11/14] fix: remove improper license Signed-off-by: ElaineDeMattosSilvaB --- .../scaffolder-node/src/actions/executeShellCommand.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts index aa60f31df4..67951f915c 100644 --- a/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts +++ b/plugins/scaffolder-node/src/actions/executeShellCommand.test.ts @@ -13,10 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -// Copyright 2024 DB Systel GmbH -// Licensed under the DBISL, see the accompanying file LICENSE. - import { LoggerService } from '@backstage/backend-plugin-api'; import { spawn } from 'child_process'; import { PassThrough, Writable } from 'stream'; From 4a8dbbd0c2c61d3380c13c21f4ddd4cd01d7e81a Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 10 Dec 2024 19:01:31 +0100 Subject: [PATCH 12/14] fix: remove unnecessary changes by the auto imports grupping Signed-off-by: ElaineDeMattosSilvaB --- .../src/actions/fetch/cookiecutter.test.ts | 10 +++++----- .../src/actions/fetch/cookiecutter.ts | 12 ++++++------ plugins/scaffolder-backend-module-rails/package.json | 8 ++++---- .../src/actions/fetch/rails/index.test.ts | 10 +++++----- .../src/actions/fetch/rails/index.ts | 8 ++++---- .../src/actions/fetch/rails/railsNewRunner.test.ts | 2 +- .../src/actions/fetch/rails/railsNewRunner.ts | 8 ++++---- 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts index ce8a34a6ed..52946ce7fd 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts @@ -15,16 +15,16 @@ */ import { ContainerRunner } from '@backstage/backend-common'; -import { UrlReaderService } from '@backstage/backend-plugin-api'; -import { createMockDirectory } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; +import { JsonObject } from '@backstage/types'; import { ScmIntegrations } from '@backstage/integration'; +import { createMockDirectory } from '@backstage/backend-test-utils'; +import { createFetchCookiecutterAction } from './cookiecutter'; +import { join } from 'path'; import type { ActionContext } from '@backstage/plugin-scaffolder-node'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; -import { JsonObject } from '@backstage/types'; -import { join } from 'path'; import { Writable } from 'stream'; -import { createFetchCookiecutterAction } from './cookiecutter'; +import { UrlReaderService } from '@backstage/backend-plugin-api'; const executeShellCommand = jest.fn(); const commandExists = jest.fn(); diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts index 269630817c..6e540d7fca 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts @@ -19,18 +19,18 @@ import { UrlReaderService, resolveSafeChildPath, } from '@backstage/backend-plugin-api'; +import { JsonObject, JsonValue } from '@backstage/types'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; -import { - createTemplateAction, - executeShellCommand, - fetchContents, -} from '@backstage/plugin-scaffolder-node'; -import { JsonObject, JsonValue } from '@backstage/types'; import commandExists from 'command-exists'; import fs from 'fs-extra'; import path, { resolve as resolvePath } from 'path'; import { PassThrough, Writable } from 'stream'; +import { + createTemplateAction, + fetchContents, + executeShellCommand, +} from '@backstage/plugin-scaffolder-node'; import { examples } from './cookiecutter.examples'; export class CookiecutterRunner { diff --git a/plugins/scaffolder-backend-module-rails/package.json b/plugins/scaffolder-backend-module-rails/package.json index 188892b2e2..fe0046f17f 100644 --- a/plugins/scaffolder-backend-module-rails/package.json +++ b/plugins/scaffolder-backend-module-rails/package.json @@ -1,11 +1,11 @@ { "name": "@backstage/plugin-scaffolder-backend-module-rails", - "version": "0.5.1", + "version": "0.5.4-next.2", "description": "A module for the scaffolder backend that lets you template projects using Rails", "backstage": { + "role": "backend-plugin-module", "pluginId": "scaffolder", - "pluginPackage": "@backstage/plugin-scaffolder-backend", - "role": "backend-plugin-module" + "pluginPackage": "@backstage/plugin-scaffolder-backend" }, "publishConfig": { "access": "public" @@ -60,7 +60,7 @@ "@backstage/plugin-scaffolder-node-test-utils": "workspace:^", "@types/command-exists": "^1.2.0", "@types/fs-extra": "^11.0.0", - "@types/node": "^18.17.8", + "@types/node": "^20.16.0", "jest-when": "^3.1.0" } } diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts index c2d3ead6bb..616c90e0de 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts @@ -28,15 +28,15 @@ jest.mock('./railsNewRunner', () => { }); import { ContainerRunner } from '@backstage/backend-common'; -import { UrlReaderService } from '@backstage/backend-plugin-api'; -import { createMockDirectory } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; -import { fetchContents } from '@backstage/plugin-scaffolder-node'; -import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; import { resolve as resolvePath } from 'path'; -import { Writable } from 'stream'; import { createFetchRailsAction } from './index'; +import { fetchContents } from '@backstage/plugin-scaffolder-node'; +import { createMockDirectory } from '@backstage/backend-test-utils'; +import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; +import { Writable } from 'stream'; +import { UrlReaderService } from '@backstage/backend-plugin-api'; describe('fetch:rails', () => { const mockDir = createMockDirectory(); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts index a063ce0a5b..4d5db0f3ef 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts @@ -15,20 +15,20 @@ */ import { ContainerRunner } from '@backstage/backend-common'; +import { JsonObject } from '@backstage/types'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; +import fs from 'fs-extra'; import { createTemplateAction, fetchContents, } from '@backstage/plugin-scaffolder-node'; -import { JsonObject } from '@backstage/types'; -import fs from 'fs-extra'; -import { UrlReaderService } from '@backstage/backend-plugin-api'; import { resolve as resolvePath } from 'path'; +import { RailsNewRunner } from './railsNewRunner'; import { PassThrough } from 'stream'; import { examples } from './index.examples'; -import { RailsNewRunner } from './railsNewRunner'; +import { UrlReaderService } from '@backstage/backend-plugin-api'; /** * Creates the `fetch:rails` Scaffolder action. diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts index c53f368516..47104c66e1 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.test.ts @@ -28,10 +28,10 @@ jest.mock( ); import { ContainerRunner } from '@backstage/backend-common'; -import { createMockDirectory } from '@backstage/backend-test-utils'; import path from 'path'; import { PassThrough } from 'stream'; import { RailsNewRunner } from './railsNewRunner'; +import { createMockDirectory } from '@backstage/backend-test-utils'; describe('Rails Templater', () => { const containerRunner: jest.Mocked = { diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts index fe2470eef8..31cfea4a76 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/railsNewRunner.ts @@ -15,16 +15,16 @@ */ import { ContainerRunner } from '@backstage/backend-common'; -import { executeShellCommand } from '@backstage/plugin-scaffolder-node'; -import { JsonObject } from '@backstage/types'; -import commandExists from 'command-exists'; import fs from 'fs-extra'; import path from 'path'; -import { Writable } from 'stream'; +import { executeShellCommand } from '@backstage/plugin-scaffolder-node'; +import commandExists from 'command-exists'; import { railsArgumentResolver, RailsRunOptions, } from './railsArgumentResolver'; +import { JsonObject } from '@backstage/types'; +import { Writable } from 'stream'; export class RailsNewRunner { private readonly containerRunner?: ContainerRunner; From 7dd0013c130fc1607a0d3e1ecd0423d1e6281db5 Mon Sep 17 00:00:00 2001 From: ElaineDeMattosSilvaB Date: Tue, 10 Dec 2024 19:06:27 +0100 Subject: [PATCH 13/14] feat: add changeset Signed-off-by: ElaineDeMattosSilvaB --- .changeset/{violet-seas-pretend.md => strong-students-beg.md} | 2 -- 1 file changed, 2 deletions(-) rename .changeset/{violet-seas-pretend.md => strong-students-beg.md} (54%) diff --git a/.changeset/violet-seas-pretend.md b/.changeset/strong-students-beg.md similarity index 54% rename from .changeset/violet-seas-pretend.md rename to .changeset/strong-students-beg.md index 09774f0af8..01fc24b996 100644 --- a/.changeset/violet-seas-pretend.md +++ b/.changeset/strong-students-beg.md @@ -1,7 +1,5 @@ --- '@backstage/plugin-scaffolder-node': minor -'@backstage/plugin-scaffolder-backend-module-cookiecutter': patch -'@backstage/plugin-scaffolder-backend-module-rails': patch --- Deprecate the `logStream` option in `executeShellCommand`, replacing it with a logger instance. From 288611a3dcc22e690b0d02635f1ced235795965c Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Wed, 11 Dec 2024 08:19:51 +0100 Subject: [PATCH 14/14] Update plugin-scaffolder-node version to patch Signed-off-by: blam --- .changeset/strong-students-beg.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/strong-students-beg.md b/.changeset/strong-students-beg.md index 01fc24b996..5b3653d389 100644 --- a/.changeset/strong-students-beg.md +++ b/.changeset/strong-students-beg.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder-node': minor +'@backstage/plugin-scaffolder-node': patch --- Deprecate the `logStream` option in `executeShellCommand`, replacing it with a logger instance.