chore(scaffolder): code review comments, passing down the docker client

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"

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 });
 }

plugins/scaffolder-backend/src/index.ts

@@ -16,3 +16,4 @@
 
 export * from './scaffolder';
 export * from './service/router';
+

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<RunDockerContainerOptions>;
   } = 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,
     });
   });
 });

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;

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<any, any>(() => [{ 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<Docker>;
+
   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(

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,

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<string>;
-}
+  run(opts: TemplaterRunOptions): Promise<string>;
+};
 
 export interface TemplaterConfig {
   templater?: TemplaterBase;

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<express.Router> {
   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);