From 7ead8675dd130af527830d6e71049c8bc58bc2f9 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 10:47:43 +0200 Subject: [PATCH] chore(scaffolder): code review comments, passing down the docker client --- packages/backend/package.json | 2 + packages/backend/src/plugins/scaffolder.ts | 4 +- plugins/scaffolder-backend/src/index.ts | 1 + .../scaffolder/templater/cookiecutter.test.ts | 43 ++++++++++++++++--- .../src/scaffolder/templater/cookiecutter.ts | 5 +++ .../src/scaffolder/templater/helpers.test.ts | 40 +++++++++-------- .../src/scaffolder/templater/helpers.ts | 6 ++- .../src/scaffolder/templater/index.ts | 8 ++-- .../scaffolder-backend/src/service/router.ts | 5 ++- 9 files changed, 82 insertions(+), 32 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index a4d5f059f9..2ae211fd8e 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -26,6 +26,7 @@ "@backstage/plugin-identity-backend": "^0.1.1-alpha.10", "@backstage/plugin-scaffolder-backend": "^0.1.1-alpha.10", "@backstage/plugin-sentry-backend": "^0.1.1-alpha.10", + "dockerode": "^3.2.0", "express": "^4.17.1", "knex": "^0.21.1", "sqlite3": "^4.2.0", @@ -33,6 +34,7 @@ }, "devDependencies": { "@backstage/cli": "^0.1.1-alpha.10", + "@types/dockerode": "^2.5.32", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", "@types/helmet": "^0.0.47" diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index 95d28eacb1..1b0ee46969 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -22,15 +22,17 @@ import { Preparers, } from '@backstage/plugin-scaffolder-backend'; import type { PluginEnvironment } from '../types'; +import Docker from 'dockerode'; export default async function createPlugin({ logger }: PluginEnvironment) { const templater = new CookieCutter(); const filePreparer = new FilePreparer(); const githubPreparer = new GithubPreparer(); const preparers = new Preparers(); + const dockerClient = new Docker(); preparers.register('file', filePreparer); preparers.register('github', githubPreparer); - return await createRouter({ preparers, templater, logger }); + return await createRouter({ preparers, templater, logger, dockerClient }); } diff --git a/plugins/scaffolder-backend/src/index.ts b/plugins/scaffolder-backend/src/index.ts index c461bfede6..0a0a4cb95f 100644 --- a/plugins/scaffolder-backend/src/index.ts +++ b/plugins/scaffolder-backend/src/index.ts @@ -16,3 +16,4 @@ export * from './scaffolder'; export * from './service/router'; + diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index bba3705a41..7c78badff1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -20,18 +20,21 @@ import fs from 'fs-extra'; import os from 'os'; import { RunDockerContainerOptions } from './helpers'; import { PassThrough } from 'stream'; +import Docker from 'dockerode'; describe('CookieCutter Templater', () => { const cookie = new CookieCutter(); - + const mockDocker = {} as Docker; const { runDockerContainer, }: { runDockerContainer: jest.Mock; } = require('./helpers'); - beforeEach(() => { + beforeEach(async () => { jest.clearAllMocks(); + + await fs.remove(`${os.tmpdir()}/cookiecutter.json`); }); it('should write a cookiecutter.json file with the values from the entitiy', async () => { @@ -42,7 +45,7 @@ describe('CookieCutter Templater', () => { description: 'description', }; - await cookie.run({ directory: tempdir, values }); + await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); const cookieCutterJson = await fs.readJSON(`${tempdir}/cookiecutter.json`); @@ -61,13 +64,28 @@ describe('CookieCutter Templater', () => { description: 'im something cool', }; - await cookie.run({ directory: tempdir, values }); + await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); const cookieCutterJson = await fs.readJSON(`${tempdir}/cookiecutter.json`); expect(cookieCutterJson).toEqual({ ...existingJson, ...values }); }); + it('should throw an error if the cookiecutter json is malformed and not missing', async () => { + const tempdir = os.tmpdir(); + + await fs.writeFile(`${tempdir}/cookiecutter.json`, "{'"); + + const values = { + component_id: 'hello', + description: 'im something cool', + }; + + await expect( + cookie.run({ directory: tempdir, values, dockerClient: mockDocker }), + ).rejects.toThrow(/Unexpected token ' in JSON at position 1/); + }); + it('should run the correct docker container with the correct bindings for the volumes', async () => { const tempdir = os.tmpdir(); @@ -76,7 +94,7 @@ describe('CookieCutter Templater', () => { description: 'description', }; - await cookie.run({ directory: tempdir, values }); + await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); expect(runDockerContainer).toHaveBeenCalledWith({ imageName: 'backstage/cookiecutter', @@ -84,6 +102,7 @@ describe('CookieCutter Templater', () => { templateDir: tempdir, resultDir: `${tempdir}/result`, logStream: undefined, + dockerClient: mockDocker, }); }); it('should return the result path to the end templated folder', async () => { @@ -94,7 +113,11 @@ describe('CookieCutter Templater', () => { description: 'description', }; - const path = await cookie.run({ directory: tempdir, values }); + const path = await cookie.run({ + directory: tempdir, + values, + dockerClient: mockDocker, + }); expect(path).toBe(`${tempdir}/result`); }); @@ -109,7 +132,12 @@ describe('CookieCutter Templater', () => { description: 'description', }; - await cookie.run({ directory: tempdir, values, logStream: stream }); + await cookie.run({ + directory: tempdir, + values, + logStream: stream, + dockerClient: mockDocker, + }); expect(runDockerContainer).toHaveBeenCalledWith({ imageName: 'backstage/cookiecutter', @@ -117,6 +145,7 @@ describe('CookieCutter Templater', () => { templateDir: tempdir, resultDir: `${tempdir}/result`, logStream: stream, + dockerClient: mockDocker, }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts index 70a0cc28af..a95eee7fb4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts @@ -26,6 +26,10 @@ export class CookieCutter implements TemplaterBase { try { return await fs.readJSON(`${directory}/cookiecutter.json`); } catch (ex) { + if (ex.code !== 'ENOENT') { + throw ex; + } + return {}; } } @@ -55,6 +59,7 @@ export class CookieCutter implements TemplaterBase { templateDir, resultDir, logStream: options.logStream, + dockerClient: options.dockerClient, }); return resultDir; diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts index a5ac00165e..d27eb4f93f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts @@ -16,26 +16,16 @@ import Stream, { PassThrough } from 'stream'; import os from 'os'; import fs from 'fs'; - -const mockDocker = { - run: jest.fn(() => [{ Error: null, StatusCode: 0 }]), -}; - -jest.mock( - 'dockerode', - () => - class { - constructor() { - return mockDocker; - } - }, -); - +import Docker from 'dockerode'; import { runDockerContainer } from './helpers'; describe('helpers', () => { + const mockDocker = new Docker() as jest.Mocked; + beforeEach(() => { - jest.clearAllMocks(); + jest + .spyOn(mockDocker, 'run') + .mockResolvedValue([{ Error: null, StatusCode: 0 }]); }); describe('runDockerContainer', () => { @@ -50,6 +40,7 @@ describe('helpers', () => { args, templateDir, resultDir, + dockerClient: mockDocker, }); expect(mockDocker.run).toHaveBeenCalledWith( @@ -80,7 +71,13 @@ describe('helpers', () => { ]); await expect( - runDockerContainer({ imageName, args, templateDir, resultDir }), + runDockerContainer({ + imageName, + args, + templateDir, + resultDir, + dockerClient: mockDocker, + }), ).rejects.toThrow(/Something went wrong with docker/); }); @@ -93,7 +90,13 @@ describe('helpers', () => { ]); await expect( - runDockerContainer({ imageName, args, templateDir, resultDir }), + runDockerContainer({ + imageName, + args, + templateDir, + resultDir, + dockerClient: mockDocker, + }), ).rejects.toThrow( /Docker container returned a non-zero exit code \(123\)/, ); @@ -107,6 +110,7 @@ describe('helpers', () => { templateDir, resultDir, logStream, + dockerClient: mockDocker, }); expect(mockDocker.run).toHaveBeenCalledWith( diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts index f60dcfc664..2856a016e4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts @@ -13,8 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import Docker from 'dockerode'; import { Writable, PassThrough } from 'stream'; +import Docker from 'dockerode'; import fs from 'fs'; export type RunDockerContainerOptions = { @@ -23,9 +23,9 @@ export type RunDockerContainerOptions = { logStream?: Writable; resultDir: string; templateDir: string; + dockerClient: Docker; }; -const dockerClient = new Docker(); /** * * @param options the options object @@ -34,6 +34,7 @@ const dockerClient = new Docker(); * @param options.logStream the log streamer to capture log messages * @param options.resultDir the /result path inside the container * @param options.templateDir the /template path inside the container + * @param options.dockerClient the dockerClient to use */ export const runDockerContainer = async ({ imageName, @@ -41,6 +42,7 @@ export const runDockerContainer = async ({ logStream = new PassThrough(), resultDir, templateDir, + dockerClient, }: RunDockerContainerOptions) => { const [{ Error: error, StatusCode: statusCode }] = await dockerClient.run( imageName, diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts b/plugins/scaffolder-backend/src/scaffolder/templater/index.ts index 0c255354fe..8885e65308 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/index.ts @@ -15,6 +15,7 @@ */ import type { Writable } from 'stream'; +import Docker from 'dockerode'; export interface RequiredTemplateValues { component_id: string; @@ -24,12 +25,13 @@ export interface TemplaterRunOptions { directory: string; values: RequiredTemplateValues & object; logStream?: Writable; + dockerClient: Docker; } -export abstract class TemplaterBase { +export type TemplaterBase = { // runs the templating with the values and returns the directory to push the VCS - abstract async run(opts: TemplaterRunOptions): Promise; -} + run(opts: TemplaterRunOptions): Promise; +}; export interface TemplaterConfig { templater?: TemplaterBase; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 16b119da02..ab39864e76 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -19,18 +19,20 @@ import Router from 'express-promise-router'; import express from 'express'; import { PreparerBuilder, TemplaterBase } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import Docker from 'dockerode'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; logger: Logger; + dockerClient: Docker; } export async function createRouter( options: RouterOptions, ): Promise { const router = Router(); - const { preparers, templater, logger: parentLogger } = options; + const { preparers, templater, logger: parentLogger, dockerClient } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); router.post('/v1/jobs', async (_, res) => { @@ -72,6 +74,7 @@ export async function createRouter( const templatedPath = await templater.run({ directory: skeletonPath, values: { component_id: 'test' }, + dockerClient, }); console.warn(templatedPath);