From f42a541ade0b5bdce148b559ef0b1530e9abe488 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 11:18:58 +0200 Subject: [PATCH 01/22] chore(backend-common): Fixing the headers sent issue so we dont send the response twice --- .../src/middleware/errorHandler.test.ts | 23 +++++++++++++++++++ .../src/middleware/errorHandler.ts | 3 +++ 2 files changed, 26 insertions(+) diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts index 022802dff8..e8a5a7f47a 100644 --- a/packages/backend-common/src/middleware/errorHandler.test.ts +++ b/packages/backend-common/src/middleware/errorHandler.test.ts @@ -34,6 +34,29 @@ describe('errorHandler', () => { expect(response.text).toBe('some message'); }); + it('doesnt try to send the response again if its already been sent', async () => { + const app = express(); + const mockSend = jest.fn(); + + app.use('/works_with_async_fail', (_, res) => { + res.status(200).send('hello'); + + // mutate the response object to test the middlware. + // it's hard to catch errors inside middleware from the outside. + // @ts-ignore + res.send = mockSend; + throw new Error('some message'); + }); + + app.use(errorHandler()); + const response = await request(app).get('/works_with_async_fail'); + + expect(response.status).toBe(200); + expect(response.text).toBe('hello'); + + expect(mockSend).not.toHaveBeenCalled(); + }); + it('takes code from http-errors library errors', async () => { const app = express(); app.use('/breaks', () => { diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts index 14f6cfa8d7..ad8f170dcd 100644 --- a/packages/backend-common/src/middleware/errorHandler.ts +++ b/packages/backend-common/src/middleware/errorHandler.ts @@ -53,7 +53,10 @@ export function errorHandler( next: NextFunction, ) => { if (response.headersSent) { + // If the headers have already been sent, do not send the response again + // as this will throw an error in the backend. next(error); + return; } const status = getStatusCode(error); From 73c8c69c1ce88aa83848fc3f283392d586b22443 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 13:56:25 +0200 Subject: [PATCH 02/22] feat(scaffolder): starting to write a job processor --- plugins/scaffolder-backend/package.json | 1 + .../src/scaffolder/index.ts | 1 + .../src/scaffolder/templater/index.ts | 3 +- .../scaffolder-backend/src/service/router.ts | 44 ++++++++++++------- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 061f6ca814..5a8ebddce5 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -36,6 +36,7 @@ "helmet": "^3.22.0", "morgan": "^1.10.0", "nodegit": "0.26.5", + "uuid": "^8.2.0", "winston": "^3.2.1" }, "devDependencies": { diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index 4cc3f21a9d..f2f7d3dad3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -16,3 +16,4 @@ export * from './templater'; export * from './prepare'; export * from './templater/cookiecutter'; +export * from './jobs'; diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts b/plugins/scaffolder-backend/src/scaffolder/templater/index.ts index 8885e65308..d0bbb0aa6c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/index.ts @@ -16,6 +16,7 @@ import type { Writable } from 'stream'; import Docker from 'dockerode'; +import { JsonValue } from '@backstage/config'; export interface RequiredTemplateValues { component_id: string; @@ -23,7 +24,7 @@ export interface RequiredTemplateValues { export interface TemplaterRunOptions { directory: string; - values: RequiredTemplateValues & object; + values: RequiredTemplateValues & Record; logStream?: Writable; dockerClient: Docker; } diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index ab39864e76..a0e31718aa 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -17,10 +17,10 @@ import { Logger } from 'winston'; import Router from 'express-promise-router'; import express from 'express'; -import { PreparerBuilder, TemplaterBase } from '../scaffolder'; +import { PreparerBuilder, TemplaterBase, JobProcessor } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; - +import {} from '@backstage/backend-common'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; @@ -35,10 +35,32 @@ export async function createRouter( const { preparers, templater, logger: parentLogger, dockerClient } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); + const jobProcessor = new JobProcessor({ + preparers, + templater, + logger, + dockerClient, + }); + + router.get('/v1/job/:jobId', ({ params }, res) => { + const job = jobProcessor.get(params.jobId); + + if (!job) { + return res.status(404).send({ error: 'job not found' }); + } + + res.send({ + id: job.id, + metadata: job.metadata, + status: job.status, + log: job.log, + error: job.error, + }); + }); + router.post('/v1/jobs', async (_, res) => { // TODO(blam): Create a unique job here and return the ID so that // The end user can poll for updates on the current job - res.status(201).json({ accepted: true }); // TODO(blam): Take this entity from the post body sent from the frontend const mockEntity: TemplateEntityV1alpha1 = { @@ -64,20 +86,12 @@ export async function createRouter( }, }; - // Get the preparer for the mock entity - const preparer = preparers.get(mockEntity); + const job = jobProcessor.create(mockEntity, { component_id: 'test' }); + res.status(201).json({ jobId: job.id }); - // Run the preparer for the mock entity to produce a temporary directory with template in - const skeletonPath = await preparer.prepare(mockEntity); + jobProcessor.run(job); - // Run the templater on the mock directory with values from the post body - const templatedPath = await templater.run({ - directory: skeletonPath, - values: { component_id: 'test' }, - dockerClient, - }); - - console.warn(templatedPath); + // console.warn(templatedPath); }); const app = express(); From 7787eb99865ef163aadb142957518477c282d258 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 13:56:43 +0200 Subject: [PATCH 03/22] feat(scaffolder): implementing a basic API for the processor to mutate the entity state --- .../src/scaffolder/jobs/index.ts | 16 +++ .../src/scaffolder/jobs/processor.ts | 110 ++++++++++++++++++ .../src/scaffolder/jobs/types.ts | 57 +++++++++ 3 files changed, 183 insertions(+) create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/index.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/types.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts new file mode 100644 index 0000000000..303987c5b1 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2020 Spotify AB + * + * 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. + */ +export * from './processor'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts new file mode 100644 index 0000000000..bc9fe4698f --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -0,0 +1,110 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { Processor, Job, ProcessorContstructorArgs } from './types'; +import { JsonValue } from '@backstage/config'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { PassThrough, Writable } from 'stream'; +import uuid from 'uuid'; +import winston from 'winston'; +import { RequiredTemplateValues } from '../templater'; +import { createNewRootLogger } from '@backstage/backend-common'; + +export class JobProcessor implements Processor { + private preparers: ProcessorContstructorArgs['preparers']; + private templater: ProcessorContstructorArgs['templater']; + private dockerClient: ProcessorContstructorArgs['dockerClient']; + private jobs = new Map(); + + constructor({ + preparers, + templater, + dockerClient, + }: ProcessorContstructorArgs) { + this.preparers = preparers; + this.templater = templater; + this.dockerClient = dockerClient; + return this; + } + + create( + entity: TemplateEntityV1alpha1, + values: RequiredTemplateValues & Record, + ): Job { + const id = uuid.v4(); + const log: string[] = []; + const logStream = new PassThrough(); + logStream.on('data', chunk => log.push(chunk.toString())); + + const logger = createNewRootLogger(); + logger.add(new winston.transports.Stream({ stream: logStream })); + + const job: Job = { + id, + logStream, + logger, + log, + status: 'PENDING', + metadata: { + entity, + values, + }, + }; + + this.jobs.set(job.id, job); + return job; + } + get(id: string): Job | undefined { + return this.jobs.get(id); + } + async run(job: Job) { + if (job.status !== 'PENDING') { + throw new Error('Job is not in pending state'); + } + + const { logger, logStream } = job; + + try { + logger.debug('Prepare started'); + job.status = 'PREPARING'; + const entity = job.metadata.entity; + const preparer = this.preparers.get(entity); + const skeletonPath = await preparer.prepare(entity); + logger.debug('Prepare finished', { + skeletonPath, + }); + + logger.debug('Templating started'); + job.status = 'TEMPLATING'; + // Run the templater on the mock directory with values from the post body + const templatedPath = await this.templater.run({ + directory: skeletonPath, + values: job.metadata.values, + dockerClient: this.dockerClient, + logStream, + }); + logger.debug('Template finished', { templatedPath }); + + job.status = 'STORING'; + // TODO(blam): Implement VCS Push here + + job.status = 'COMPLETE'; + } catch (ex) { + job.error = ex; + job.status = 'FAILED'; + logger.error(`job ${job.id} failed with reason`, { ex }); + } + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts new file mode 100644 index 0000000000..05630e2c63 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -0,0 +1,57 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 type { Writable } from 'stream'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { JsonValue } from '@backstage/config'; +import { PreparerBuilder } from '../prepare'; +import Docker from 'dockerode'; +import { TemplaterBase, RequiredTemplateValues } from '../templater'; +import { Logger } from 'winston'; + +export type Job = { + id: string; + metadata: { + entity: TemplateEntityV1alpha1; + values: RequiredTemplateValues & Record; + }; + status: + | 'PENDING' + | 'PREPARING' + | 'TEMPLATING' + | 'STORING' + | 'COMPLETE' + | 'FAILED'; + logStream: Writable; + log: string[]; + logger: Logger; + error?: Error; +}; + +export type ProcessorContstructorArgs = { + preparers: PreparerBuilder; + templater: TemplaterBase; + logger: Logger; + dockerClient: Docker; +}; + +export type Processor = { + create( + entity: TemplateEntityV1alpha1, + values: RequiredTemplateValues & Record, + ): Job; + + get(id: string): Job | undefined; +}; From 85d1fbaf3c8c4642e3dc587cc070bd3cbe9be482 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 13:57:19 +0200 Subject: [PATCH 04/22] chore(backend-common): expose a create logger function --- .../backend-common/src/logging/rootLogger.ts | 40 ++++++++++--------- yarn.lock | 5 +++ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 11db57c1ed..47379b027c 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -16,24 +16,28 @@ import * as winston from 'winston'; -let rootLogger: winston.Logger = winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: - process.env.NODE_ENV === 'production' - ? winston.format.json() - : winston.format.combine( - winston.format.colorize(), - winston.format.timestamp(), - winston.format.simple(), - ), - defaultMeta: { service: 'backstage' }, - transports: [ - new winston.transports.Console({ - silent: - process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, - }), - ], -}); +export function createNewRootLogger(): winston.Logger { + return winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: + process.env.NODE_ENV === 'production' + ? winston.format.json() + : winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: { service: 'backstage' }, + transports: [ + new winston.transports.Console({ + silent: + process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, + }), + ], + }); +} + +let rootLogger: winston.Logger = createNewRootLogger(); export function getRootLogger(): winston.Logger { return rootLogger; diff --git a/yarn.lock b/yarn.lock index c9128c5350..ba53ee7513 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18918,6 +18918,11 @@ uuid@^8.0.0: resolved "https://registry.npmjs.org/uuid/-/uuid-8.1.0.tgz#6f1536eb43249f473abc6bd58ff983da1ca30d8d" integrity sha512-CI18flHDznR0lq54xBycOVmphdCYnQLKn8abKn7PXUiKUGdEd+/l9LWNJmugXel4hXq7S+RMNl34ecyC9TntWg== +uuid@^8.2.0: + version "8.2.0" + resolved "https://registry.npmjs.org/uuid/-/uuid-8.2.0.tgz#cb10dd6b118e2dada7d0cd9730ba7417c93d920e" + integrity sha512-CYpGiFTUrmI6OBMkAdjSDM0k5h8SkkiTP4WAjQgDgNB1S3Ou9VBEvr6q0Kv2H1mMk7IWfxYGpMH5sd5AvcIV2Q== + v8-compile-cache@^2.0.3: version "2.1.0" resolved "https://registry.npmjs.org/v8-compile-cache/-/v8-compile-cache-2.1.0.tgz#e14de37b31a6d194f5690d67efc4e7f6fc6ab30e" From bca0ebf3c9e08577ddaec2d6e5b1680b97e4fc6f Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Thu, 25 Jun 2020 00:15:58 +0200 Subject: [PATCH 05/22] fix(scaffolder): stringify error --- plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index bc9fe4698f..29b67b5e14 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -104,7 +104,7 @@ export class JobProcessor implements Processor { } catch (ex) { job.error = ex; job.status = 'FAILED'; - logger.error(`job ${job.id} failed with reason`, { ex }); + logger.error(`job ${job.id} failed with reason: ${ex}`); } } } From 1dd56d28a9fc8245f97c4bcfd51f4a1d8b2d17dd Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Thu, 25 Jun 2020 00:16:26 +0200 Subject: [PATCH 06/22] feat(scaffolder): create new tempdir for result --- .../src/scaffolder/templater/cookiecutter.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts index a95eee7fb4..cb11bf04af 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts @@ -48,10 +48,7 @@ export class CookieCutter implements TemplaterBase { await fs.writeJSON(`${options.directory}/cookiecutter.json`, cookieInfo); const templateDir = options.directory; - - // TODO(blam): This should be an entirely different directory on the host machine - // not in the template directory - const resultDir = `${templateDir}/result`; + const resultDir = await fs.promises.mkdtemp(`${options.directory}-result`); await runDockerContainer({ imageName: 'backstage/cookiecutter', From 82321cb4002e5adc850bec3988957e7bf9140212 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jun 2020 04:21:43 +0200 Subject: [PATCH 07/22] chore(scaffolder): fixing issues with scaffolder --- .../src/scaffolder/jobs/processor.test.ts | 16 +++++++++++++++ .../src/scaffolder/jobs/processor.ts | 20 ++++++++++++++----- 2 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts new file mode 100644 index 0000000000..1db114f597 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2020 Spotify AB + * + * 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. + */ +describe('JobProcessor', () => {}); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index bc9fe4698f..0c2a6ceae2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -16,7 +16,7 @@ import { Processor, Job, ProcessorContstructorArgs } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { PassThrough, Writable } from 'stream'; +import { PassThrough } from 'stream'; import uuid from 'uuid'; import winston from 'winston'; import { RequiredTemplateValues } from '../templater'; @@ -45,9 +45,16 @@ export class JobProcessor implements Processor { ): Job { const id = uuid.v4(); const log: string[] = []; + + // Create an empty stream to collect all the log lines into + // one variable for the API. const logStream = new PassThrough(); logStream.on('data', chunk => log.push(chunk.toString())); + // TODO(blam): Maybe this is not the right way to build the logger + // Maybe we want to be more ux specific and drop the json support. + // Child loggers can not have specific transports which sucks, so we have to + // create another here. const logger = createNewRootLogger(); logger.add(new winston.transports.Stream({ stream: logStream })); @@ -64,6 +71,7 @@ export class JobProcessor implements Processor { }; this.jobs.set(job.id, job); + return job; } get(id: string): Job | undefined { @@ -77,6 +85,7 @@ export class JobProcessor implements Processor { const { logger, logStream } = job; try { + // Prepare a folder for the templater to run in logger.debug('Prepare started'); job.status = 'PREPARING'; const entity = job.metadata.entity; @@ -86,9 +95,9 @@ export class JobProcessor implements Processor { skeletonPath, }); + // Run the templater on the directory with values passed in logger.debug('Templating started'); job.status = 'TEMPLATING'; - // Run the templater on the mock directory with values from the post body const templatedPath = await this.templater.run({ directory: skeletonPath, values: job.metadata.values, @@ -97,14 +106,15 @@ export class JobProcessor implements Processor { }); logger.debug('Template finished', { templatedPath }); + // Store the template somewhere when finished job.status = 'STORING'; // TODO(blam): Implement VCS Push here job.status = 'COMPLETE'; - } catch (ex) { - job.error = ex; + } catch (error) { + job.error = error; job.status = 'FAILED'; - logger.error(`job ${job.id} failed with reason`, { ex }); + logger.error(`Job failed with error ${error.message}`); } } } From 5a88ef753ba1efb5af3d8168366df4c8f6f26118 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jun 2020 23:29:54 +0200 Subject: [PATCH 08/22] chore(scaffolder): Reworking how the processor works. It's starting to look a lot cleaner now --- .../src/scaffolder/jobs/processor.test.ts | 34 ++++++- .../src/scaffolder/jobs/processor.ts | 93 +++++++++++-------- .../src/scaffolder/jobs/types.ts | 11 +-- .../scaffolder-backend/src/service/router.ts | 86 ++++++++--------- 4 files changed, 129 insertions(+), 95 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 1db114f597..aacbcf61f0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -13,4 +13,36 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -describe('JobProcessor', () => {}); +import { JobProcessor } from './processor'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +describe('JobProcessor', () => { + describe('create', () => { + const mockEntity: TemplateEntityV1alpha1 = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + }, + name: 'graphql-starter', + title: 'GraphQL Service', + description: + 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', + etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + + generation: 1, + }, + spec: { + type: 'cookiecutter', + path: './template', + }, + }; + const processor = new JobProcessor(); + + it('should create a unique id for the job', async () => { + const job = processor.create(); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 0c2a6ceae2..4e90b457a7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,30 +13,38 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job, ProcessorContstructorArgs } from './types'; +import { Processor, Job } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { PassThrough } from 'stream'; import uuid from 'uuid'; +import Docker from 'dockerode'; import winston from 'winston'; -import { RequiredTemplateValues } from '../templater'; +import { RequiredTemplateValues, TemplaterBase } from '../templater'; import { createNewRootLogger } from '@backstage/backend-common'; +import { PreparerBuilder } from '../prepare'; + +export type JobProcessorArguments = { + preparers: PreparerBuilder; + templater: TemplaterBase; + dockerClient: Docker; +}; + +export type JobAndDirectoryTuple = { + job: Job; + directory: string; +}; export class JobProcessor implements Processor { - private preparers: ProcessorContstructorArgs['preparers']; - private templater: ProcessorContstructorArgs['templater']; - private dockerClient: ProcessorContstructorArgs['dockerClient']; + private preparers: PreparerBuilder; + private templater: TemplaterBase; + private dockerClient: Docker; private jobs = new Map(); - constructor({ - preparers, - templater, - dockerClient, - }: ProcessorContstructorArgs) { + constructor({ preparers, templater, dockerClient }: JobProcessorArguments) { this.preparers = preparers; this.templater = templater; this.dockerClient = dockerClient; - return this; } create( @@ -74,47 +82,50 @@ export class JobProcessor implements Processor { return job; } + get(id: string): Job | undefined { return this.jobs.get(id); } - async run(job: Job) { + + private async prepare(job: Job): Promise { + job.status = 'PREPARING'; + const entity = job.metadata.entity; + const preparer = this.preparers.get(entity); + return await preparer.prepare(entity); + } + + private async run(job: Job, directory: string): Promise { + job.status = 'TEMPLATING'; + return await this.templater.run({ + directory, + values: job.metadata.values, + dockerClient: this.dockerClient, + logStream: job.logStream, + }); + } + + private async store(job: Job): Promise { + job.status = 'STORING'; + } + + private async complete(job: Job): Promise { + job.status = 'COMPLETE'; + } + + async process(job: Job) { if (job.status !== 'PENDING') { throw new Error('Job is not in pending state'); } - const { logger, logStream } = job; - try { - // Prepare a folder for the templater to run in - logger.debug('Prepare started'); - job.status = 'PREPARING'; - const entity = job.metadata.entity; - const preparer = this.preparers.get(entity); - const skeletonPath = await preparer.prepare(entity); - logger.debug('Prepare finished', { - skeletonPath, - }); - - // Run the templater on the directory with values passed in - logger.debug('Templating started'); - job.status = 'TEMPLATING'; - const templatedPath = await this.templater.run({ - directory: skeletonPath, - values: job.metadata.values, - dockerClient: this.dockerClient, - logStream, - }); - logger.debug('Template finished', { templatedPath }); - - // Store the template somewhere when finished - job.status = 'STORING'; - // TODO(blam): Implement VCS Push here - - job.status = 'COMPLETE'; + const skeletonPath = await this.prepare(job); + await this.run(job, skeletonPath); + await this.store(job); + await this.complete(job); } catch (error) { job.error = error; job.status = 'FAILED'; - logger.error(`Job failed with error ${error.message}`); + job.logger.error(`Job failed with error ${error.message}`); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 05630e2c63..e6c5436e95 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -16,9 +16,7 @@ import type { Writable } from 'stream'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; -import { PreparerBuilder } from '../prepare'; -import Docker from 'dockerode'; -import { TemplaterBase, RequiredTemplateValues } from '../templater'; +import { RequiredTemplateValues } from '../templater'; import { Logger } from 'winston'; export type Job = { @@ -40,13 +38,6 @@ export type Job = { error?: Error; }; -export type ProcessorContstructorArgs = { - preparers: PreparerBuilder; - templater: TemplaterBase; - logger: Logger; - dockerClient: Docker; -}; - export type Processor = { create( entity: TemplateEntityV1alpha1, diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index a0e31718aa..4d64bc159f 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -42,57 +42,57 @@ export async function createRouter( dockerClient, }); - router.get('/v1/job/:jobId', ({ params }, res) => { - const job = jobProcessor.get(params.jobId); + router + .get('/v1/job/:jobId', ({ params }, res) => { + const job = jobProcessor.get(params.jobId); - if (!job) { - return res.status(404).send({ error: 'job not found' }); - } + if (!job) { + return res.status(404).send({ error: 'job not found' }); + } - res.send({ - id: job.id, - metadata: job.metadata, - status: job.status, - log: job.log, - error: job.error, - }); - }); + res.send({ + id: job.id, + metadata: job.metadata, + status: job.status, + log: job.log, + error: job.error, + }); + }) + .post('/v1/jobs', async (_, res) => { + // TODO(blam): Create a unique job here and return the ID so that + // The end user can poll for updates on the current job - router.post('/v1/jobs', async (_, res) => { - // TODO(blam): Create a unique job here and return the ID so that - // The end user can poll for updates on the current job + // TODO(blam): Take this entity from the post body sent from the frontend + const mockEntity: TemplateEntityV1alpha1 = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + }, + name: 'graphql-starter', + title: 'GraphQL Service', + description: + 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', + etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', - // TODO(blam): Take this entity from the post body sent from the frontend - const mockEntity: TemplateEntityV1alpha1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + generation: 1, }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + spec: { + type: 'cookiecutter', + path: './template', + }, + }; - generation: 1, - }, - spec: { - type: 'cookiecutter', - path: './template', - }, - }; + const job = jobProcessor.create(mockEntity, { component_id: 'test' }); + res.status(201).json({ jobId: job.id }); - const job = jobProcessor.create(mockEntity, { component_id: 'test' }); - res.status(201).json({ jobId: job.id }); + jobProcessor.run(job); - jobProcessor.run(job); - - // console.warn(templatedPath); - }); + // console.warn(templatedPath); + }); const app = express(); app.set('logger', logger); From 1655b8c16e93882070a58ba280b750e275019d14 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jun 2020 23:39:36 +0200 Subject: [PATCH 09/22] chore(scaffolder): added some more tests and more refactoring --- .../src/scaffolder/jobs/processor.test.ts | 56 ++++++++++--------- .../scaffolder/templater/cookiecutter.test.ts | 6 +- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index aacbcf61f0..49b5ef7f54 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -16,33 +16,37 @@ import { JobProcessor } from './processor'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; describe('JobProcessor', () => { + const mockEntity: TemplateEntityV1alpha1 = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + }, + name: 'graphql-starter', + title: 'GraphQL Service', + description: + 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', + etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + + generation: 1, + }, + spec: { + type: 'cookiecutter', + path: './template', + }, + }; + describe('create', () => { - const mockEntity: TemplateEntityV1alpha1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', - }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + it.todo('creates a new job'); + }); - generation: 1, - }, - spec: { - type: 'cookiecutter', - path: './template', - }, - }; - const processor = new JobProcessor(); - - it('should create a unique id for the job', async () => { - const job = processor.create(); - }); + describe('process', () => { + it.todo('allows running of a job in a pending state'); + it.todo('fails when the job is not in a pending state'); + it.todo('calls the preparer with the entity'); + it.todo('calls the templater with the correct directory'); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index 7c78badff1..f7e2d68719 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -100,7 +100,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}/result`, + resultDir: `${tempdir}-result`, logStream: undefined, dockerClient: mockDocker, }); @@ -119,7 +119,7 @@ describe('CookieCutter Templater', () => { dockerClient: mockDocker, }); - expect(path).toBe(`${tempdir}/result`); + expect(path).toBe(`${tempdir}-result`); }); it('should pass through the streamer to the run docker helper', async () => { @@ -143,7 +143,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}/result`, + resultDir: `${tempdir}-result`, logStream: stream, dockerClient: mockDocker, }); From e0f026bb392a440ab660aaffa22cbe84eef7c2da Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 13:53:14 +0200 Subject: [PATCH 10/22] chore(scaffolder): fixing cookiecutter templater tests --- .../src/scaffolder/templater/cookiecutter.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index f7e2d68719..f31dd7cdd0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -100,7 +100,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}-result`, + resultDir: expect.stringContaining(`${tempdir}-result`), logStream: undefined, dockerClient: mockDocker, }); @@ -119,7 +119,7 @@ describe('CookieCutter Templater', () => { dockerClient: mockDocker, }); - expect(path).toBe(`${tempdir}-result`); + expect(path.startsWith(`${tempdir}-result`)).toBeTruthy(); }); it('should pass through the streamer to the run docker helper', async () => { @@ -143,7 +143,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}-result`, + resultDir: expect.stringContaining(`${tempdir}-result`), logStream: stream, dockerClient: mockDocker, }); From f2d01c5cb4fe529ee48f99f025bbd9c608d0c775 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 14:19:13 +0200 Subject: [PATCH 11/22] chore(scaffolder): adding some more tests for scaffolder processor --- .../src/scaffolder/jobs/processor.test.ts | 21 ++++++++++++++++++- .../src/scaffolder/jobs/processor.ts | 2 +- .../scaffolder/templater/cookiecutter.test.ts | 1 + .../scaffolder-backend/src/service/router.ts | 8 +++---- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 49b5ef7f54..8c3643b10b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -15,6 +15,10 @@ */ import { JobProcessor } from './processor'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import Docker from 'dockerode'; +import { CookieCutter } from '../templater/cookiecutter'; +import { Preparers } from '../'; + describe('JobProcessor', () => { const mockEntity: TemplateEntityV1alpha1 = { apiVersion: 'backstage.io/v1alpha1', @@ -40,7 +44,22 @@ describe('JobProcessor', () => { }; describe('create', () => { - it.todo('creates a new job'); + const templater = new CookieCutter(); + const preparers = new Preparers(); + const mockDocker = {} as jest.Mocked; + it('creates a new job', async () => { + const processor = new JobProcessor({ + dockerClient: mockDocker, + preparers, + templater, + }); + + const job = processor.create(mockEntity, { component_id: 'bob' }); + + expect(job.id).toMatch( + /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i, + ); + }); }); describe('process', () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 4e90b457a7..341ae6724e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -17,7 +17,7 @@ import { Processor, Job } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { PassThrough } from 'stream'; -import uuid from 'uuid'; +import * as uuid from 'uuid'; import Docker from 'dockerode'; import winston from 'winston'; import { RequiredTemplateValues, TemplaterBase } from '../templater'; diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index f31dd7cdd0..82e84bc1d3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -105,6 +105,7 @@ describe('CookieCutter Templater', () => { dockerClient: mockDocker, }); }); + it('should return the result path to the end templated folder', async () => { const tempdir = os.tmpdir(); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 4d64bc159f..5957b66915 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -38,7 +38,6 @@ export async function createRouter( const jobProcessor = new JobProcessor({ preparers, templater, - logger, dockerClient, }); @@ -47,7 +46,8 @@ export async function createRouter( const job = jobProcessor.get(params.jobId); if (!job) { - return res.status(404).send({ error: 'job not found' }); + res.status(404).send({ error: 'job not found' }); + return; } res.send({ @@ -89,9 +89,7 @@ export async function createRouter( const job = jobProcessor.create(mockEntity, { component_id: 'test' }); res.status(201).json({ jobId: job.id }); - jobProcessor.run(job); - - // console.warn(templatedPath); + jobProcessor.process(job); }); const app = express(); From b401cf2f1789b03f647bf9452e9d55322b9a730b Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 17:11:46 +0200 Subject: [PATCH 12/22] chore(scaffolder): Updating scaffolder tests to start runnning some stuff --- .../src/scaffolder/jobs/processor.test.ts | 82 +++++++++++++++++-- .../src/scaffolder/jobs/processor.ts | 2 +- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 8c3643b10b..b8f1c8f86e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -18,6 +18,7 @@ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; import { CookieCutter } from '../templater/cookiecutter'; import { Preparers } from '../'; +import { Job } from './types'; describe('JobProcessor', () => { const mockEntity: TemplateEntityV1alpha1 = { @@ -43,6 +44,8 @@ describe('JobProcessor', () => { }, }; + const mockValues = { component_id: 'bob' }; + describe('create', () => { const templater = new CookieCutter(); const preparers = new Preparers(); @@ -54,18 +57,87 @@ describe('JobProcessor', () => { templater, }); - const job = processor.create(mockEntity, { component_id: 'bob' }); + const job = processor.create(mockEntity, mockValues); expect(job.id).toMatch( /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i, ); + + expect(job.log).toEqual([]); + expect(job.status).toBe('PENDING'); + expect(job.metadata.entity).toBe(mockEntity); + expect(job.metadata.values).toBe(mockValues); }); }); describe('process', () => { - it.todo('allows running of a job in a pending state'); - it.todo('fails when the job is not in a pending state'); - it.todo('calls the preparer with the entity'); - it.todo('calls the templater with the correct directory'); + const preparers = new Preparers(); + const mockDocker = {} as jest.Mocked; + const mockPreparer = { prepare: jest.fn() }; + const templater = { run: jest.fn() }; + + const createJob = (): { job: Job; processor: JobProcessor } => { + preparers.register('github', mockPreparer); + + const processor = new JobProcessor({ + preparers, + dockerClient: mockDocker, + templater, + }); + + return { job: processor.create(mockEntity, mockValues), processor }; + }; + + // TODO(blam): make this better. + // Wait 10ms for processor to finish. + const waitForProcessor = () => + new Promise(resolve => setTimeout(resolve, 10)); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('fails when the job is not in a pending state', async () => { + const { job, processor } = createJob(); + job.status = 'TEMPLATING'; + + await expect(processor.process(job)).rejects.toThrow( + /Job is not in a 'PENDING' state/, + ); + }); + + it('calls the preparer with the entity', async () => { + const { job, processor } = createJob(); + + // Create a promise to hold it at this step so we can test + mockPreparer.prepare.mockImplementationOnce(() => new Promise(() => {})); + + processor.process(job); + + await waitForProcessor(); + + expect(mockPreparer.prepare).toHaveBeenCalledWith(mockEntity); + expect(job.status).toBe('PREPARING'); + }); + + it('calls the templater with the correct directory', async () => { + const { job, processor } = createJob(); + const mockDirectory = '/test/blam/bo'; + mockPreparer.prepare.mockResolvedValueOnce(mockDirectory); + + // Create a promise to hold it at this step so we can test + templater.run.mockImplementationOnce(() => new Promise(() => {})); + + processor.process(job); + + await waitForProcessor(); + + expect(templater.run).toHaveBeenCalledWith({ + directory: mockDirectory, + values: mockValues, + dockerClient: mockDocker, + logStream: job.logStream, + }); + }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 341ae6724e..c97d5ced79 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -114,7 +114,7 @@ export class JobProcessor implements Processor { async process(job: Job) { if (job.status !== 'PENDING') { - throw new Error('Job is not in pending state'); + throw new Error("Job is not in a 'PENDING' state"); } try { From ce43dce8ff7b9daf6feeac6c3d2523baf8734ff2 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 18:12:48 +0200 Subject: [PATCH 13/22] feat(scaffolder): Adjusting the types for the scaffolder --- .../src/scaffolder/jobs/types.ts | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index e6c5436e95..060d5d400e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -19,21 +19,31 @@ import { JsonValue } from '@backstage/config'; import { RequiredTemplateValues } from '../templater'; import { Logger } from 'winston'; +// Context will be a mutable object which is passed between stages +// To share data, but also thinking that we can pass in functions here too +// To maybe create sub steps or fail the entire thing, or skip stages down the line. +export type StageContext = T & { + values: RequiredTemplateValues & Record; + entity: TemplateEntityV1alpha1; + logger: Logger; +}; + +export type Stage = { + log: string[]; + status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; + name: string; + handler: (ctx: StageContext) => Promise; +}; + export type Job = { id: string; metadata: { entity: TemplateEntityV1alpha1; values: RequiredTemplateValues & Record; }; - status: - | 'PENDING' - | 'PREPARING' - | 'TEMPLATING' - | 'STORING' - | 'COMPLETE' - | 'FAILED'; + status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; + stages: Stage[]; logStream: Writable; - log: string[]; logger: Logger; error?: Error; }; From ad2354909eb1af6fc3b9274453621c3ce3b11f52 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 21:27:48 +0200 Subject: [PATCH 14/22] chore(scaffolder): work on a better api for holding stages and metadata --- .../src/scaffolder/jobs/processor.test.ts | 1 + .../src/scaffolder/jobs/processor.ts | 27 ++++++++------- .../src/scaffolder/jobs/types.ts | 34 ++++++++++++------- .../{ => stages}/prepare/file.test.ts | 0 .../scaffolder/{ => stages}/prepare/file.ts | 0 .../{ => stages}/prepare/github.test.ts | 0 .../scaffolder/{ => stages}/prepare/github.ts | 0 .../{ => stages}/prepare/helpers.test.ts | 0 .../{ => stages}/prepare/helpers.ts | 0 .../scaffolder/{ => stages}/prepare/index.ts | 0 .../{ => stages}/prepare/preparers.test.ts | 0 .../{ => stages}/prepare/preparers.ts | 0 .../scaffolder/{ => stages}/prepare/types.ts | 0 .../templater/cookiecutter.test.ts | 0 .../{ => stages}/templater/cookiecutter.ts | 0 .../{ => stages}/templater/helpers.test.ts | 0 .../{ => stages}/templater/helpers.ts | 0 .../{ => stages}/templater/index.ts | 0 18 files changed, 36 insertions(+), 26 deletions(-) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/file.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/file.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/github.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/github.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/helpers.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/helpers.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/index.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/preparers.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/preparers.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/types.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/cookiecutter.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/cookiecutter.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/helpers.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/helpers.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/index.ts (100%) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index b8f1c8f86e..367d140f7e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -79,6 +79,7 @@ describe('JobProcessor', () => { const createJob = (): { job: Job; processor: JobProcessor } => { preparers.register('github', mockPreparer); + new JobProcessor(1); const processor = new JobProcessor({ preparers, dockerClient: mockDocker, diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index c97d5ced79..a1b8795156 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,16 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job } from './types'; +import { Processor, Job, Stage, StageContext } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { PassThrough } from 'stream'; import * as uuid from 'uuid'; import Docker from 'dockerode'; import winston from 'winston'; -import { RequiredTemplateValues, TemplaterBase } from '../templater'; +import { RequiredTemplateValues, TemplaterBase } from '../stages/templater'; import { createNewRootLogger } from '@backstage/backend-common'; -import { PreparerBuilder } from '../prepare'; +import { PreparerBuilder } from '../stages/prepare'; export type JobProcessorArguments = { preparers: PreparerBuilder; @@ -36,15 +36,10 @@ export type JobAndDirectoryTuple = { }; export class JobProcessor implements Processor { - private preparers: PreparerBuilder; - private templater: TemplaterBase; - private dockerClient: Docker; private jobs = new Map(); - - constructor({ preparers, templater, dockerClient }: JobProcessorArguments) { - this.preparers = preparers; - this.templater = templater; - this.dockerClient = dockerClient; + private stages: Stage[]; + constructor({ stages }: { stages: Stage[] }) { + this.stages = stages; } create( @@ -66,11 +61,17 @@ export class JobProcessor implements Processor { const logger = createNewRootLogger(); logger.add(new winston.transports.Stream({ stream: logStream })); + const context: StageContext = { + entity, + values, + logger, + }; + const job: Job = { id, logStream, - logger, - log, + context, + stages: this.stages.map((stage) => ({})) status: 'PENDING', metadata: { entity, diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 060d5d400e..b01fae9625 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -16,13 +16,13 @@ import type { Writable } from 'stream'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; -import { RequiredTemplateValues } from '../templater'; +import { RequiredTemplateValues } from '../stages/templater'; import { Logger } from 'winston'; // Context will be a mutable object which is passed between stages // To share data, but also thinking that we can pass in functions here too // To maybe create sub steps or fail the entire thing, or skip stages down the line. -export type StageContext = T & { +export type StageContext = T & { values: RequiredTemplateValues & Record; entity: TemplateEntityV1alpha1; logger: Logger; @@ -35,12 +35,9 @@ export type Stage = { handler: (ctx: StageContext) => Promise; }; -export type Job = { +export type Job = { id: string; - metadata: { - entity: TemplateEntityV1alpha1; - values: RequiredTemplateValues & Record; - }; + context: StageContext; status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; stages: Stage[]; logStream: Writable; @@ -48,11 +45,22 @@ export type Job = { error?: Error; }; -export type Processor = { - create( - entity: TemplateEntityV1alpha1, - values: RequiredTemplateValues & Record, - ): Job; +export interface ProcessorConstructor { + new (t: string): Processor; +} + +export interface Processor { + create({ + entity, + values, + stages, + }: { + entity: TemplateEntityV1alpha1; + values: RequiredTemplateValues & Record; + stages: Stage[]; + }): Job; get(id: string): Job | undefined; -}; +} + +declare let Processor: ProcessorConstructor; diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/file.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/file.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/file.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/file.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/github.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/github.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/helpers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/helpers.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/index.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/index.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/index.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/index.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/preparers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/preparers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/preparers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/preparers.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/types.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/index.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/index.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/index.ts From 17c6030ca0400bd27bddb73811c9483a2b3b285a Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 03:41:06 +0200 Subject: [PATCH 15/22] feat(scaffolder): finally managed to get typescript to play nice with the new stage approach --- .../backend-common/src/logging/rootLogger.ts | 41 +++++++--------- .../src/scaffolder/jobs/logger.ts | 44 +++++++++++++++++ .../src/scaffolder/jobs/processor.ts | 45 ++++++------------ .../src/scaffolder/jobs/types.ts | 45 +++++++++--------- .../scaffolder-backend/src/service/router.ts | 47 ++++++++++++++++--- 5 files changed, 140 insertions(+), 82 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 47379b027c..fd2f3a1c8b 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -13,31 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import * as winston from 'winston'; -export function createNewRootLogger(): winston.Logger { - return winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: - process.env.NODE_ENV === 'production' - ? winston.format.json() - : winston.format.combine( - winston.format.colorize(), - winston.format.timestamp(), - winston.format.simple(), - ), - defaultMeta: { service: 'backstage' }, - transports: [ - new winston.transports.Console({ - silent: - process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, - }), - ], - }); -} - -let rootLogger: winston.Logger = createNewRootLogger(); +let rootLogger: winston.Logger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: + process.env.NODE_ENV === 'production' + ? winston.format.json() + : winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: { service: 'backstage' }, + transports: [ + new winston.transports.Console({ + silent: + process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, + }), + ], +}); export function getRootLogger(): winston.Logger { return rootLogger; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts new file mode 100644 index 0000000000..61d95b6b08 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts @@ -0,0 +1,44 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { PassThrough } from 'stream'; +import winston from 'winston'; +import { JsonValue } from '@backstage/config'; + +export const useLogStream = (meta: Record) => { + const log: string[] = []; + + // Create an empty stream to collect all the log lines into + // one variable for the API. + const stream = new PassThrough(); + stream.on('data', chunk => log.push(chunk.toString())); + + const logger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + ), + defaultMeta: meta, + }); + + logger.add(new winston.transports.Stream({ stream })); + + return { + log, + stream, + logger, + }; +}; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index a1b8795156..69d02fa7d1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,16 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job, Stage, StageContext } from './types'; +import { Processor, Job, Stage, StageContext, StageInput } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { PassThrough } from 'stream'; import * as uuid from 'uuid'; import Docker from 'dockerode'; -import winston from 'winston'; import { RequiredTemplateValues, TemplaterBase } from '../stages/templater'; -import { createNewRootLogger } from '@backstage/backend-common'; import { PreparerBuilder } from '../stages/prepare'; +import { useLogStream } from './logger'; export type JobProcessorArguments = { preparers: PreparerBuilder; @@ -37,29 +35,18 @@ export type JobAndDirectoryTuple = { export class JobProcessor implements Processor { private jobs = new Map(); - private stages: Stage[]; - constructor({ stages }: { stages: Stage[] }) { - this.stages = stages; - } - create( - entity: TemplateEntityV1alpha1, - values: RequiredTemplateValues & Record, - ): Job { + create({ + entity, + values, + stages, + }: { + entity: TemplateEntityV1alpha1; + values: RequiredTemplateValues & Record; + stages: StageInput[]; + }): Job { const id = uuid.v4(); - const log: string[] = []; - - // Create an empty stream to collect all the log lines into - // one variable for the API. - const logStream = new PassThrough(); - logStream.on('data', chunk => log.push(chunk.toString())); - - // TODO(blam): Maybe this is not the right way to build the logger - // Maybe we want to be more ux specific and drop the json support. - // Child loggers can not have specific transports which sucks, so we have to - // create another here. - const logger = createNewRootLogger(); - logger.add(new winston.transports.Stream({ stream: logStream })); + const { logger, stream } = useLogStream({ id }); const context: StageContext = { entity, @@ -69,14 +56,10 @@ export class JobProcessor implements Processor { const job: Job = { id, - logStream, + logStream: stream, context, - stages: this.stages.map((stage) => ({})) + stages, status: 'PENDING', - metadata: { - entity, - values, - }, }; this.jobs.set(job.id, job); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index b01fae9625..6c66a65355 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -22,45 +22,46 @@ import { Logger } from 'winston'; // Context will be a mutable object which is passed between stages // To share data, but also thinking that we can pass in functions here too // To maybe create sub steps or fail the entire thing, or skip stages down the line. -export type StageContext = T & { +export type StageContext = { values: RequiredTemplateValues & Record; entity: TemplateEntityV1alpha1; logger: Logger; -}; + logStream: Writable; +} & T; -export type Stage = { +export type ProcessorStatus = 'PENDING' | 'STARTED' | 'COMPLETED' | 'FAILED'; + +export interface Stage extends StageInput { log: string[]; - status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; - name: string; - handler: (ctx: StageContext) => Promise; -}; + status: ProcessorStatus; + startedAt?: number; + endedAt?: number; +} -export type Job = { +export interface StageInput { + name: string; + handler(ctx: StageContext): Promise; +} + +export type Job = { id: string; - context: StageContext; - status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; + context: StageContext; + status: ProcessorStatus; stages: Stage[]; logStream: Writable; - logger: Logger; error?: Error; }; -export interface ProcessorConstructor { - new (t: string): Processor; -} - -export interface Processor { - create({ +export type Processor = { + create({ entity, values, stages, }: { entity: TemplateEntityV1alpha1; values: RequiredTemplateValues & Record; - stages: Stage[]; - }): Job; + stages: StageInput[]; + }): Job; get(id: string): Job | undefined; -} - -declare let Processor: ProcessorConstructor; +}; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 5957b66915..b340190b41 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -21,6 +21,7 @@ import { PreparerBuilder, TemplaterBase, JobProcessor } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; import {} from '@backstage/backend-common'; +import { StageContext, Stage } from '../scaffolder/jobs/types'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; @@ -35,11 +36,7 @@ export async function createRouter( const { preparers, templater, logger: parentLogger, dockerClient } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); - const jobProcessor = new JobProcessor({ - preparers, - templater, - dockerClient, - }); + const jobProcessor = new JobProcessor(); router .get('/v1/job/:jobId', ({ params }, res) => { @@ -86,7 +83,45 @@ export async function createRouter( }, }; - const job = jobProcessor.create(mockEntity, { component_id: 'test' }); + const job = jobProcessor.create({ + entity: mockEntity, + values: { component_id: 'blob' }, + stages: [ + { + name: 'Prepare the skeleton', + handler: async ctx => { + const preparer = preparers.get(ctx.entity); + const skeletonDir = await preparer.prepare(ctx.entity); + return { skeletonDir }; + }, + }, + { + name: 'Run the templater', + handler: async (ctx: StageContext<{ skeletonDir: string }>) => { + const resultDir = await templater.run({ + directory: ctx.skeletonDir, + dockerClient, + logStream: ctx.logStream, + values: ctx.values, + }); + + return { resultDir }; + }, + }, + { + name: 'Create VCS Repo', + handler: async (ctx: StageContext<{ resultDir: string }>) => { + ctx.logger.info('Should now create the VCS repo'); + }, + }, + { + name: 'Push to remote', + handler: async ctx => { + ctx.logger.info('Should now push to the remote'); + }, + }, + ], + }); res.status(201).json({ jobId: job.id }); jobProcessor.process(job); From c600d64db8e2eac524c371b66b123d2a953d7e13 Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 05:21:42 +0200 Subject: [PATCH 16/22] feat(scaffolder): reworking how the loading of stages goes and made it super flexible --- .../src/scaffolder/index.ts | 6 +- .../src/scaffolder/jobs/logger.ts | 1 + .../src/scaffolder/jobs/processor.ts | 76 +++++++++++-------- .../src/scaffolder/jobs/types.ts | 2 + .../src/scaffolder/stages/prepare/types.ts | 6 +- .../stages/templater/cookiecutter.ts | 11 ++- .../scaffolder-backend/src/service/router.ts | 20 +++-- 7 files changed, 78 insertions(+), 44 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index f2f7d3dad3..6010d31e98 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export * from './templater'; -export * from './prepare'; -export * from './templater/cookiecutter'; +export * from './stages/templater'; +export * from './stages/prepare'; +export * from './stages/templater/cookiecutter'; export * from './jobs'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts index 61d95b6b08..d2eaa13c3b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts @@ -30,6 +30,7 @@ export const useLogStream = (meta: Record) => { format: winston.format.combine( winston.format.colorize(), winston.format.timestamp(), + winston.format.simple(), ), defaultMeta: meta, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 69d02fa7d1..795e07291d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -52,13 +52,19 @@ export class JobProcessor implements Processor { entity, values, logger, + logStream: stream, }; const job: Job = { id, logStream: stream, context, - stages, + stages: stages.map(stage => ({ + handler: stage.handler, + log: [], + name: stage.name, + status: 'PENDING', + })), status: 'PENDING', }; @@ -71,45 +77,49 @@ export class JobProcessor implements Processor { return this.jobs.get(id); } - private async prepare(job: Job): Promise { - job.status = 'PREPARING'; - const entity = job.metadata.entity; - const preparer = this.preparers.get(entity); - return await preparer.prepare(entity); - } - - private async run(job: Job, directory: string): Promise { - job.status = 'TEMPLATING'; - return await this.templater.run({ - directory, - values: job.metadata.values, - dockerClient: this.dockerClient, - logStream: job.logStream, - }); - } - - private async store(job: Job): Promise { - job.status = 'STORING'; - } - - private async complete(job: Job): Promise { - job.status = 'COMPLETE'; - } - - async process(job: Job) { + async run(job: Job): Promise { if (job.status !== 'PENDING') { throw new Error("Job is not in a 'PENDING' state"); } + job.status = 'STARTED'; + try { - const skeletonPath = await this.prepare(job); - await this.run(job, skeletonPath); - await this.store(job); - await this.complete(job); + for (const entry of job.stages) { + const { logger, log, stream } = useLogStream({ + id: job.id, + stage: entry.name, + }); + try { + entry.startedAt = Date.now(); + + entry.log = log; + + const handler = await entry.handler({ + ...job.context, + logger, + logStream: stream, + }); + + job.context = { + ...job.context, + ...handler, + }; + + entry.status = 'COMPLETED'; + } catch (error) { + logger.error(error); + entry.status = 'FAILED'; + throw error; + } finally { + entry.endedAt = Date.now(); + } + } + job.status = 'COMPLETED'; } catch (error) { - job.error = error; + job.error = { name: error.name, message: error.message }; job.status = 'FAILED'; - job.logger.error(`Job failed with error ${error.message}`); + job.context.logger.error(`Job failed with error ${error.message}`); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 6c66a65355..92e4e1cbf4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -64,4 +64,6 @@ export type Processor = { }): Job; get(id: string): Job | undefined; + + run(job: Job): Promise; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts index a6e42c465f..adbc38ef58 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import type { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { Logger } from 'winston'; export type PreparerBase = { /** @@ -21,7 +22,10 @@ export type PreparerBase = { * with contents from the remote location in temporary storage and return the path * @param template The template entity from the Service Catalog */ - prepare(template: TemplateEntityV1alpha1): Promise; + prepare( + template: TemplateEntityV1alpha1, + opts: { logger: Logger }, + ): Promise; }; export type PreparerBuilder = { diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts index cb11bf04af..cef8a64b60 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts @@ -51,8 +51,15 @@ export class CookieCutter implements TemplaterBase { const resultDir = await fs.promises.mkdtemp(`${options.directory}-result`); await runDockerContainer({ - imageName: 'backstage/cookiecutter', - args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], + imageName: 'spotify/backstage-cookiecutter', + args: [ + 'cookiecutter', + '--no-input', + '-o', + '/result', + '/template', + '--verbose', + ], templateDir, resultDir, logStream: options.logStream, diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index b340190b41..0b4ec2a5b8 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -49,9 +49,16 @@ export async function createRouter( res.send({ id: job.id, - metadata: job.metadata, + metadata: { + ...job.context, + logger: undefined, + logStream: undefined, + }, status: job.status, - log: job.log, + stages: job.stages.map(stage => ({ + ...stage, + handler: undefined, + })), error: job.error, }); }) @@ -66,7 +73,7 @@ export async function createRouter( metadata: { annotations: { 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + 'github:https://github.com/benjdlambert/backstage-graphqsl-template/blob/master/template.yaml', }, name: 'graphql-starter', title: 'GraphQL Service', @@ -91,7 +98,9 @@ export async function createRouter( name: 'Prepare the skeleton', handler: async ctx => { const preparer = preparers.get(ctx.entity); - const skeletonDir = await preparer.prepare(ctx.entity); + const skeletonDir = await preparer.prepare(ctx.entity, { + logger: ctx.logger, + }); return { skeletonDir }; }, }, @@ -122,9 +131,10 @@ export async function createRouter( }, ], }); + res.status(201).json({ jobId: job.id }); - jobProcessor.process(job); + jobProcessor.run(job); }); const app = express(); From 9e6ed5fb5239f48cc9b8dc90adbdd9fdf739071e Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 22:10:55 +0200 Subject: [PATCH 17/22] feat(scaffolder): added some more tests for the new job processor --- .../src/scaffolder/jobs/processor.test.ts | 201 +++++++++++------- .../src/scaffolder/jobs/processor.ts | 2 +- 2 files changed, 131 insertions(+), 72 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 367d140f7e..6b80298926 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -15,10 +15,7 @@ */ import { JobProcessor } from './processor'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import Docker from 'dockerode'; -import { CookieCutter } from '../templater/cookiecutter'; -import { Preparers } from '../'; -import { Job } from './types'; +import { StageInput } from './types'; describe('JobProcessor', () => { const mockEntity: TemplateEntityV1alpha1 = { @@ -47,98 +44,160 @@ describe('JobProcessor', () => { const mockValues = { component_id: 'bob' }; describe('create', () => { - const templater = new CookieCutter(); - const preparers = new Preparers(); - const mockDocker = {} as jest.Mocked; - it('creates a new job', async () => { - const processor = new JobProcessor({ - dockerClient: mockDocker, - preparers, - templater, - }); + it('creates should create a new job with a unique id', async () => { + const processor = new JobProcessor(); - const job = processor.create(mockEntity, mockValues); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); expect(job.id).toMatch( /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i, ); + }); + + it('should setup the correct context for the job', async () => { + const processor = new JobProcessor(); + + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); + + console.warn(job.context); + expect(job.context.entity).toBe(mockEntity); + expect(job.context.values).toBe(mockValues); + }); + + it('should set the status as pending', async () => { + const processor = new JobProcessor(); + + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); - expect(job.log).toEqual([]); expect(job.status).toBe('PENDING'); - expect(job.metadata.entity).toBe(mockEntity); - expect(job.metadata.values).toBe(mockValues); + }); + + it('should create the correct stages', async () => { + const stages: StageInput[] = [ + { + name: 'Do something cool step 1', + handler: jest.fn(), + }, + { + name: 'Do something cool step 2', + handler: jest.fn(), + }, + ]; + + const processor = new JobProcessor(); + + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); + + expect(job.stages).toHaveLength(stages.length); + + for (let i = 0; i < job.stages.length; i++) { + expect(job.stages[i].name).toBe(stages[i].name); + expect(job.stages[i].status).toBe('PENDING'); + } }); }); - describe('process', () => { - const preparers = new Preparers(); - const mockDocker = {} as jest.Mocked; - const mockPreparer = { prepare: jest.fn() }; - const templater = { run: jest.fn() }; - - const createJob = (): { job: Job; processor: JobProcessor } => { - preparers.register('github', mockPreparer); - - new JobProcessor(1); - const processor = new JobProcessor({ - preparers, - dockerClient: mockDocker, - templater, - }); - - return { job: processor.create(mockEntity, mockValues), processor }; - }; - - // TODO(blam): make this better. - // Wait 10ms for processor to finish. - const waitForProcessor = () => - new Promise(resolve => setTimeout(resolve, 10)); - - beforeEach(() => { - jest.clearAllMocks(); + describe('get', () => { + it('return undefined for when the job does not exist', () => { + const processor = new JobProcessor(); + expect(processor.get('123')).not.toBeDefined(); }); - it('fails when the job is not in a pending state', async () => { - const { job, processor } = createJob(); - job.status = 'TEMPLATING'; + it('should return the exact same instance of the job when one is created', async () => { + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); - await expect(processor.process(job)).rejects.toThrow( + expect(processor.get(job.id)).toBe(job); + }); + }); + describe('process', () => { + it('throws an error when the status of the job is not in pending state', async () => { + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); + + job.status = 'STARTED'; + + await expect(processor.run(job)).rejects.toThrow( /Job is not in a 'PENDING' state/, ); }); - it('calls the preparer with the entity', async () => { - const { job, processor } = createJob(); + it('will call each of the handlers in the stages', async () => { + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest.fn(), + }, + { + name: 'g/p', + handler: jest.fn(), + }, + ]; - // Create a promise to hold it at this step so we can test - mockPreparer.prepare.mockImplementationOnce(() => new Promise(() => {})); + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); - processor.process(job); + await processor.run(job); - await waitForProcessor(); - - expect(mockPreparer.prepare).toHaveBeenCalledWith(mockEntity); - expect(job.status).toBe('PREPARING'); + for (const stage of stages) { + expect(stage.handler).toHaveBeenCalled(); + } }); - it('calls the templater with the correct directory', async () => { - const { job, processor } = createJob(); - const mockDirectory = '/test/blam/bo'; - mockPreparer.prepare.mockResolvedValueOnce(mockDirectory); + it('should set all stages to complete and the job to complete when finishes without errors', async () => { + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest.fn(), + }, + { + name: 'g/p', + handler: jest.fn(), + }, + ]; - // Create a promise to hold it at this step so we can test - templater.run.mockImplementationOnce(() => new Promise(() => {})); - - processor.process(job); - - await waitForProcessor(); - - expect(templater.run).toHaveBeenCalledWith({ - directory: mockDirectory, + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, values: mockValues, - dockerClient: mockDocker, - logStream: job.logStream, + stages, }); + + await processor.run(job); + + for (const stage of job.stages) { + expect(stage.status).toBe('COMPLETED'); + } + + expect(job.status).toBe('COMPLETED'); }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 795e07291d..89f2e89d97 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job, Stage, StageContext, StageInput } from './types'; +import { Processor, Job, StageContext, StageInput } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import * as uuid from 'uuid'; From 837a182f68856d1bafbcc9b8509a4c7313b38784 Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 22:35:48 +0200 Subject: [PATCH 18/22] chore(scaffolder): finished off tests for the processor. It now works pretty well --- .../src/scaffolder/jobs/processor.test.ts | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 6b80298926..501e72c078 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -67,7 +67,6 @@ describe('JobProcessor', () => { stages: [], }); - console.warn(job.context); expect(job.context.entity).toBe(mockEntity); expect(job.context.values).toBe(mockValues); }); @@ -199,5 +198,81 @@ describe('JobProcessor', () => { expect(job.status).toBe('COMPLETED'); }); + + it('should merge the return value from previous steps into the context of the next step', async () => { + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest + .fn() + .mockResolvedValue({ first: 'ben', second: 'lambert' }), + }, + { + name: 'g/p', + handler: jest + .fn() + .mockResolvedValue({ second: 'linus', third: 'lambert' }), + }, + { + name: 'go', + handler: jest.fn(), + }, + ]; + + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); + + await processor.run(job); + + expect(stages[1].handler).toHaveBeenCalledWith( + expect.objectContaining({ first: 'ben', second: 'lambert' }), + ); + + expect(stages[2].handler).toHaveBeenCalledWith( + expect.objectContaining({ + first: 'ben', + second: 'linus', + third: 'lambert', + }), + ); + }); + + it('should fail the job and the step if one of them fails', async () => { + const fail = new Error('something went wrong here'); + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest.fn(), + }, + { + name: 'g/p', + handler: jest.fn().mockRejectedValue(fail), + }, + { + name: 'go', + handler: jest.fn(), + }, + ]; + + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); + + await processor.run(job); + + expect(job.status).toBe('FAILED'); + expect(job.stages[0].status).toBe('COMPLETED'); + expect(job.stages[1].status).toBe('FAILED'); + expect(job.stages[2].status).toBe('PENDING'); + expect(job.error?.message).toBe('something went wrong here'); + expect(job.stages[1].log.join()).toContain('something went wrong here'); + }); }); }); From c96075ce8c94158f9616e8b05e5ac3a68193987a Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 23:18:15 +0200 Subject: [PATCH 19/22] feat(scaffolder): added an endpoint for getting each stage logs as plaintext in case that's what is needed. --- .../src/scaffolder/index.ts | 2 +- .../src/scaffolder/jobs/index.ts | 1 + .../src/scaffolder/jobs/processor.ts | 48 ++++++++++++------- .../src/scaffolder/jobs/types.ts | 1 - .../scaffolder/stages/prepare/github.test.ts | 4 +- .../src/scaffolder/stages/prepare/github.ts | 8 +--- .../scaffolder-backend/src/service/router.ts | 16 +++++-- 7 files changed, 49 insertions(+), 31 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index 6010d31e98..a1ee172184 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -14,6 +14,6 @@ * limitations under the License. */ export * from './stages/templater'; -export * from './stages/prepare'; export * from './stages/templater/cookiecutter'; +export * from './stages/prepare'; export * from './jobs'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts index 303987c5b1..683d9c2750 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export * from './processor'; +export * from './types'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 89f2e89d97..1c76ae252d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -57,7 +57,6 @@ export class JobProcessor implements Processor { const job: Job = { id, - logStream: stream, context, stages: stages.map(stage => ({ handler: stage.handler, @@ -85,41 +84,56 @@ export class JobProcessor implements Processor { job.status = 'STARTED'; try { - for (const entry of job.stages) { + for (const stage of job.stages) { + // Create a logger for each stage so we can create seperate + // Streams for each step. const { logger, log, stream } = useLogStream({ id: job.id, - stage: entry.name, + stage: stage.name, }); + // Attach the logger to the stage, and setup some timestamps. + stage.log = log; + stage.startedAt = Date.now(); + try { - entry.startedAt = Date.now(); - - entry.log = log; - - const handler = await entry.handler({ + // Run the handler with the context created for the Job and some + // Additional logging helpers. + const handlerResponse = await stage.handler({ ...job.context, logger, logStream: stream, }); - job.context = { - ...job.context, - ...handler, - }; + // If the handler returns something, then let's merge this onto the ontext + // For the next stage to use as it might be relevant. + if (handlerResponse) { + job.context = { + ...job.context, + ...handlerResponse, + }; + } - entry.status = 'COMPLETED'; + // Complete the current stage + stage.status = 'COMPLETED'; } catch (error) { - logger.error(error); - entry.status = 'FAILED'; + // Log to the current stage the error that occured and fail the stage. + logger.error(`Stage failed with error: ${error.message}`); + stage.status = 'FAILED'; + + // Throw the error so the job can be failed too. throw error; } finally { - entry.endedAt = Date.now(); + // Always set the stage end timestamp. + stage.endedAt = Date.now(); } } + + // If all went to plan, complete the job. job.status = 'COMPLETED'; } catch (error) { + // If something went wrong, fail the job, and set the error property on the job. job.error = { name: error.name, message: error.message }; job.status = 'FAILED'; - job.context.logger.error(`Job failed with error ${error.message}`); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 92e4e1cbf4..fd554e2033 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -48,7 +48,6 @@ export type Job = { context: StageContext; status: ProcessorStatus; stages: Stage[]; - logStream: Writable; error?: Error; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts index 44d53248d0..86377be6d0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts @@ -60,7 +60,7 @@ describe('GitHubPreparer', () => { 1, 'https://github.com/benjdlambert/backstage-graphql-template', expect.any(String), - { checkoutOpts: { paths: ['template'] } }, + {}, ); }); it('calls the clone command with the correct arguments for a repository when no path is provided', async () => { @@ -71,7 +71,7 @@ describe('GitHubPreparer', () => { 1, 'https://github.com/benjdlambert/backstage-graphql-template', expect.any(String), - { checkoutOpts: {} }, + {}, ); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts index b0ced7db2b..f53151e55d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts @@ -21,7 +21,7 @@ import { parseLocationAnnotation } from './helpers'; import { InputError } from '@backstage/backend-common'; import { PreparerBase } from './types'; import GitUriParser from 'git-url-parse'; -import { Clone, CheckoutOptions } from 'nodegit'; +import { Clone } from 'nodegit'; export class GithubPreparer implements PreparerBase { async prepare(template: TemplateEntityV1alpha1): Promise { @@ -45,13 +45,7 @@ export class GithubPreparer implements PreparerBase { template.spec.path ?? '.', ); - const checkoutOptions = new CheckoutOptions(); - if (template.spec.path) { - checkoutOptions.paths = [templateDirectory]; - } - await Clone.clone(repositoryCheckoutUrl, tempDir, { - checkoutOpts: checkoutOptions, // TODO(blam): Maybe need some auth here? }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0b4ec2a5b8..0f43c3e0de 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -21,7 +21,7 @@ import { PreparerBuilder, TemplaterBase, JobProcessor } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; import {} from '@backstage/backend-common'; -import { StageContext, Stage } from '../scaffolder/jobs/types'; +import { StageContext } from '../scaffolder/jobs/types'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; @@ -39,6 +39,16 @@ export async function createRouter( const jobProcessor = new JobProcessor(); router + .get('/v1/job/:jobId/stage/:index/log', ({ params }, res) => { + const job = jobProcessor.get(params.jobId); + + if (!job) { + res.status(404).send({ error: 'job not found' }); + return; + } + + res.send(job.stages[Number(params.index)].log.join('')); + }) .get('/v1/job/:jobId', ({ params }, res) => { const job = jobProcessor.get(params.jobId); @@ -73,7 +83,7 @@ export async function createRouter( metadata: { annotations: { 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphqsl-template/blob/master/template.yaml', + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', }, name: 'graphql-starter', title: 'GraphQL Service', @@ -132,7 +142,7 @@ export async function createRouter( ], }); - res.status(201).json({ jobId: job.id }); + res.status(201).json({ id: job.id }); jobProcessor.run(job); }); From 066ced06ac1101ce47f60a7f6583b3e950533dba Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 23:53:41 +0200 Subject: [PATCH 20/22] chore(scaffolder): fixing some tests with it creating bad tmp dirs --- .../stages/templater/cookiecutter.test.ts | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index 82e84bc1d3..a24f1ed64f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -33,12 +33,15 @@ describe('CookieCutter Templater', () => { beforeEach(async () => { jest.clearAllMocks(); - - await fs.remove(`${os.tmpdir()}/cookiecutter.json`); }); + const mkTemp = async () => { + const tempDir = os.tmpdir(); + return await fs.promises.mkdtemp(tempDir); + }; + it('should write a cookiecutter.json file with the values from the entitiy', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -53,10 +56,11 @@ describe('CookieCutter Templater', () => { }); it('should merge any value that is in the cookiecutter.json path already', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const existingJson = { _copy_without_render: ['./github/workflows/*'], }; + await fs.writeJSON(`${tempdir}/cookiecutter.json`, existingJson); const values = { @@ -72,7 +76,7 @@ describe('CookieCutter Templater', () => { }); it('should throw an error if the cookiecutter json is malformed and not missing', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); await fs.writeFile(`${tempdir}/cookiecutter.json`, "{'"); @@ -87,7 +91,7 @@ describe('CookieCutter Templater', () => { }); it('should run the correct docker container with the correct bindings for the volumes', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -97,8 +101,15 @@ describe('CookieCutter Templater', () => { await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); expect(runDockerContainer).toHaveBeenCalledWith({ - imageName: 'backstage/cookiecutter', - args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], + imageName: 'spotify/backstage-cookiecutter', + args: [ + 'cookiecutter', + '--no-input', + '-o', + '/result', + '/template', + '--verbose', + ], templateDir: tempdir, resultDir: expect.stringContaining(`${tempdir}-result`), logStream: undefined, @@ -107,7 +118,7 @@ describe('CookieCutter Templater', () => { }); it('should return the result path to the end templated folder', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -126,7 +137,7 @@ describe('CookieCutter Templater', () => { it('should pass through the streamer to the run docker helper', async () => { const stream = new PassThrough(); - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -141,8 +152,15 @@ describe('CookieCutter Templater', () => { }); expect(runDockerContainer).toHaveBeenCalledWith({ - imageName: 'backstage/cookiecutter', - args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], + imageName: 'spotify/backstage-cookiecutter', + args: [ + 'cookiecutter', + '--no-input', + '-o', + '/result', + '/template', + '--verbose', + ], templateDir: tempdir, resultDir: expect.stringContaining(`${tempdir}-result`), logStream: stream, From f07b794987db8a6ad785804145b90b75fdd43484 Mon Sep 17 00:00:00 2001 From: blam Date: Sun, 28 Jun 2020 00:05:45 +0200 Subject: [PATCH 21/22] chore(scaffolder): trying to fix some tests on CI, they pass locall --- .../src/scaffolder/stages/templater/cookiecutter.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index a24f1ed64f..8c0cd5d6d7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -18,6 +18,7 @@ jest.mock('./helpers', () => ({ runDockerContainer: jest.fn() })); import { CookieCutter } from './cookiecutter'; import fs from 'fs-extra'; import os from 'os'; +import path from 'path'; import { RunDockerContainerOptions } from './helpers'; import { PassThrough } from 'stream'; import Docker from 'dockerode'; @@ -37,7 +38,7 @@ describe('CookieCutter Templater', () => { const mkTemp = async () => { const tempDir = os.tmpdir(); - return await fs.promises.mkdtemp(tempDir); + return await fs.promises.mkdtemp(path.join(tempDir, 'temp')); }; it('should write a cookiecutter.json file with the values from the entitiy', async () => { From 9d8709d5b78c4d8b1a992cedd8fdaf826216cf8a Mon Sep 17 00:00:00 2001 From: blam Date: Sun, 28 Jun 2020 00:09:44 +0200 Subject: [PATCH 22/22] chore(scaffolder): fixing linting issues --- .../src/scaffolder/stages/templater/cookiecutter.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index 8c0cd5d6d7..994307e8b7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -126,13 +126,13 @@ describe('CookieCutter Templater', () => { description: 'description', }; - const path = await cookie.run({ + const returnPath = await cookie.run({ directory: tempdir, values, dockerClient: mockDocker, }); - expect(path.startsWith(`${tempdir}-result`)).toBeTruthy(); + expect(returnPath.startsWith(`${tempdir}-result`)).toBeTruthy(); }); it('should pass through the streamer to the run docker helper', async () => {