From e0bfd3d44888e89cae10bd423f0742f48be8dfc6 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 21 Apr 2021 17:14:11 +0200 Subject: [PATCH] Refactor the runDockerContainer function to a ContainerRunner interface. Signed-off-by: Dominik Henneke --- .changeset/bright-bags-hope.md | 56 +++++++ .changeset/eight-dingos-exist.md | 58 ++++++++ .changeset/few-games-collect.md | 61 ++++++++ .changeset/spicy-countries-hope.md | 27 ++++ .changeset/wild-stingrays-yawn.md | 115 +++++++++++++++ .../software-templates/installation.md | 15 +- .../writing-custom-actions.md | 7 +- docs/features/techdocs/getting-started.md | 11 +- .../src/util/ContainerRunner.ts | 31 ++++ ....test.ts => DockerContainerRunner.test.ts} | 31 ++-- .../src/util/DockerContainerRunner.ts | 117 +++++++++++++++ packages/backend-common/src/util/docker.ts | 137 ------------------ packages/backend-common/src/util/index.ts | 3 +- packages/backend/src/plugins/scaffolder.ts | 15 +- packages/backend/src/plugins/techdocs.ts | 10 +- .../backend/src/plugins/scaffolder.ts | 19 ++- .../packages/backend/src/plugins/techdocs.ts | 17 ++- packages/techdocs-common/package.json | 2 - .../src/stages/generate/generators.test.ts | 12 +- .../src/stages/generate/generators.ts | 12 +- .../src/stages/generate/techdocs.ts | 18 ++- .../src/stages/generate/types.ts | 3 - plugins/scaffolder-backend/package.json | 2 - .../actions/builtin/createBuiltinActions.ts | 13 +- .../builtin/fetch/cookiecutter.test.ts | 17 +-- .../actions/builtin/fetch/cookiecutter.ts | 13 +- .../src/scaffolder/jobs/processor.ts | 10 +- .../src/scaffolder/stages/legacy.ts | 11 +- .../stages/templater/cookiecutter.test.ts | 40 ++--- .../stages/templater/cookiecutter.ts | 22 ++- .../scaffolder/stages/templater/cra/index.ts | 18 ++- .../stages/templater/templaters.test.ts | 7 +- .../src/scaffolder/stages/templater/types.ts | 5 +- .../src/service/router.test.ts | 15 +- .../scaffolder-backend/src/service/router.ts | 6 - plugins/techdocs-backend/package.json | 2 +- .../src/DocsBuilder/builder.ts | 6 - .../techdocs-backend/src/service/router.ts | 4 - .../src/service/standaloneServer.ts | 31 ++-- 39 files changed, 652 insertions(+), 347 deletions(-) create mode 100644 .changeset/bright-bags-hope.md create mode 100644 .changeset/eight-dingos-exist.md create mode 100644 .changeset/few-games-collect.md create mode 100644 .changeset/spicy-countries-hope.md create mode 100644 .changeset/wild-stingrays-yawn.md create mode 100644 packages/backend-common/src/util/ContainerRunner.ts rename packages/backend-common/src/util/{docker.test.ts => DockerContainerRunner.test.ts} (89%) create mode 100644 packages/backend-common/src/util/DockerContainerRunner.ts delete mode 100644 packages/backend-common/src/util/docker.ts diff --git a/.changeset/bright-bags-hope.md b/.changeset/bright-bags-hope.md new file mode 100644 index 0000000000..88ef046fb7 --- /dev/null +++ b/.changeset/bright-bags-hope.md @@ -0,0 +1,56 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Migrate the plugin to use the `ContainerRunner` interface instead of `runDockerContainer(…)`. +It also provides the `ContainerRunner` to the individual templaters instead of to the `createRouter` function. + +To apply this change to an existing backend application, add the following to `src/plugins/scaffolder.ts`: + +```diff +- import { SingleHostDiscovery } from '@backstage/backend-common'; ++ import { ++ DockerContainerRunner, ++ SingleHostDiscovery, ++ } from '@backstage/backend-common'; + + + export default async function createPlugin({ + logger, + config, + database, + reader, + }: PluginEnvironment): Promise { ++ const dockerClient = new Docker(); ++ const containerRunner = new DockerContainerRunner({ dockerClient }); + ++ const cookiecutterTemplater = new CookieCutter({ containerRunner }); +- const cookiecutterTemplater = new CookieCutter(); ++ const craTemplater = new CreateReactAppTemplater({ containerRunner }); +- const craTemplater = new CreateReactAppTemplater(); + const templaters = new Templaters(); + + templaters.register('cookiecutter', cookiecutterTemplater); + templaters.register('cra', craTemplater); + + const preparers = await Preparers.fromConfig(config, { logger }); + const publishers = await Publishers.fromConfig(config, { logger }); + +- const dockerClient = new Docker(); + + const discovery = SingleHostDiscovery.fromConfig(config); + const catalogClient = new CatalogClient({ discoveryApi: discovery }); + + return await createRouter({ + preparers, + templaters, + publishers, + logger, + config, +- dockerClient, + database, + catalogClient, + reader, + }); + } +``` diff --git a/.changeset/eight-dingos-exist.md b/.changeset/eight-dingos-exist.md new file mode 100644 index 0000000000..da05523a15 --- /dev/null +++ b/.changeset/eight-dingos-exist.md @@ -0,0 +1,58 @@ +--- +'@backstage/backend-common': minor +--- + +Refactor the `runDockerContainer(…)` function to an interface-based api. +This gives the option to replace the docker runtime in the future. + +Packages and plugins that previously used the `dockerode` as argument should be migrated to use the new `ContainerRunner` interface instead. + +```diff + import { +- runDockerContainer, ++ ContainerRunner, + PluginEndpointDiscovery, + } from '@backstage/backend-common'; +- import Docker from 'dockerode'; + + type RouterOptions = { + // ... +- dockerClient: Docker, ++ containerRunner: ContainerRunner; + }; + + export async function createRouter({ + // ... +- dockerClient, ++ containerRunner, + }: RouterOptions): Promise { + // ... + ++ await containerRunner.runContainer({ +- await runDockerContainer({ + image: 'docker', + // ... +- dockerClient, + }); + + // ... + } +``` + +To keep the `dockerode` based runtime, use the `DockerContainerRunner` implementation: + +```diff ++ import { ++ ContainerRunner, ++ DockerContainerRunner ++ } from '@backstage/backend-common'; +- import { runDockerContainer } from '@backstage/backend-common'; + ++ const containerRunner: ContainerRUnner = new DockerContainerRunner({dockerClient}); ++ await containerRunner.runContainer({ +- await runDockerContainer({ + image: 'docker', + // ... +- dockerClient, + }); +``` diff --git a/.changeset/few-games-collect.md b/.changeset/few-games-collect.md new file mode 100644 index 0000000000..f6a27f66c7 --- /dev/null +++ b/.changeset/few-games-collect.md @@ -0,0 +1,61 @@ +--- +'@backstage/plugin-techdocs-backend': minor +--- + +Migrate the plugin to use the `ContainerRunner` interface instead of `runDockerContainer(…)`. +It also provides the `ContainerRunner` to the generators instead of to the `createRouter` function. + +To apply this change to an existing backend application, add the following to `src/plugins/techdocs.ts`: + +```diff ++ import { DockerContainerRunner } from '@backstage/backend-common'; + + // ... + + export default async function createPlugin({ + logger, + config, + discovery, + reader, + }: PluginEnvironment): Promise { + // Preparers are responsible for fetching source files for documentation. + const preparers = await Preparers.fromConfig(config, { + logger, + reader, + }); + ++ // Docker client (conditionally) used by the generators, based on techdocs.generators config. ++ const dockerClient = new Docker(); ++ const containerRunner = new DockerContainerRunner({ dockerClient }); + + // Generators are used for generating documentation sites. + const generators = await Generators.fromConfig(config, { + logger, ++ containerRunner, + }); + + // Publisher is used for + // 1. Publishing generated files to storage + // 2. Fetching files from storage and passing them to TechDocs frontend. + const publisher = await Publisher.fromConfig(config, { + logger, + discovery, + }); + + // checks if the publisher is working and logs the result + await publisher.getReadiness(); + +- // Docker client (conditionally) used by the generators, based on techdocs.generators config. +- const dockerClient = new Docker(); + + return await createRouter({ + preparers, + generators, + publisher, +- dockerClient, + logger, + config, + discovery, + }); + } +``` diff --git a/.changeset/spicy-countries-hope.md b/.changeset/spicy-countries-hope.md new file mode 100644 index 0000000000..574a117f6e --- /dev/null +++ b/.changeset/spicy-countries-hope.md @@ -0,0 +1,27 @@ +--- +'@backstage/techdocs-common': minor +--- + +Migrate the package to use the `ContainerRunner` interface instead of `runDockerContainer(…)`. +It also no longer provides the `ContainerRunner` as an input to the `GeneratorBase#run(…)` function, but expects it as a constructor parameter instead. + +If you use the `TechdocsGenerator` you need to update the usage: + +```diff ++ const containerRunner = new DockerContainerRunner({ dockerClient }); + +- const generator = new TechdocsGenerator(logger, config); ++ const techdocsGenerator = new TechdocsGenerator({ ++ logger, ++ containerRunner, ++ config, ++ }); + + await this.generator.run({ + inputDir: preparedDir, + outputDir, +- dockerClient: this.dockerClient, + parsedLocationAnnotation, + etag: newEtag, + }); +``` diff --git a/.changeset/wild-stingrays-yawn.md b/.changeset/wild-stingrays-yawn.md new file mode 100644 index 0000000000..6d8538c025 --- /dev/null +++ b/.changeset/wild-stingrays-yawn.md @@ -0,0 +1,115 @@ +--- +'@backstage/create-app': patch +--- + +The `scaffolder-backend` and `techdocs-backend` plugins have been updated. +In order to update, you need to apply the following changes to your existing backend application: + +`@backstage/plugin-techdocs-backend`: + +```diff +// packages/backend/src/plugin/techdocs.ts + ++ import { DockerContainerRunner } from '@backstage/backend-common'; + + // ... + + export default async function createPlugin({ + logger, + config, + discovery, + reader, + }: PluginEnvironment): Promise { + // Preparers are responsible for fetching source files for documentation. + const preparers = await Preparers.fromConfig(config, { + logger, + reader, + }); + ++ // Docker client (conditionally) used by the generators, based on techdocs.generators config. ++ const dockerClient = new Docker(); ++ const containerRunner = new DockerContainerRunner({ dockerClient }); + + // Generators are used for generating documentation sites. + const generators = await Generators.fromConfig(config, { + logger, ++ containerRunner, + }); + + // Publisher is used for + // 1. Publishing generated files to storage + // 2. Fetching files from storage and passing them to TechDocs frontend. + const publisher = await Publisher.fromConfig(config, { + logger, + discovery, + }); + + // checks if the publisher is working and logs the result + await publisher.getReadiness(); + +- // Docker client (conditionally) used by the generators, based on techdocs.generators config. +- const dockerClient = new Docker(); + + return await createRouter({ + preparers, + generators, + publisher, +- dockerClient, + logger, + config, + discovery, + }); + } +``` + +`@backstage/plugin-scaffolder-backend`: + +```diff +// packages/backend/src/plugin/scaffolder.ts + +- import { SingleHostDiscovery } from '@backstage/backend-common'; ++ import { ++ DockerContainerRunner, ++ SingleHostDiscovery, ++ } from '@backstage/backend-common'; + + + export default async function createPlugin({ + logger, + config, + database, + reader, + }: PluginEnvironment): Promise { ++ const dockerClient = new Docker(); ++ const containerRunner = new DockerContainerRunner({ dockerClient }); + ++ const cookiecutterTemplater = new CookieCutter({ containerRunner }); +- const cookiecutterTemplater = new CookieCutter(); ++ const craTemplater = new CreateReactAppTemplater({ containerRunner }); +- const craTemplater = new CreateReactAppTemplater(); + const templaters = new Templaters(); + + templaters.register('cookiecutter', cookiecutterTemplater); + templaters.register('cra', craTemplater); + + const preparers = await Preparers.fromConfig(config, { logger }); + const publishers = await Publishers.fromConfig(config, { logger }); + +- const dockerClient = new Docker(); + + const discovery = SingleHostDiscovery.fromConfig(config); + const catalogClient = new CatalogClient({ discoveryApi: discovery }); + + return await createRouter({ + preparers, + templaters, + publishers, + logger, + config, +- dockerClient, + database, + catalogClient, + reader, + }); + } +``` diff --git a/docs/features/software-templates/installation.md b/docs/features/software-templates/installation.md index 5acaf0e499..ad7e6d1062 100644 --- a/docs/features/software-templates/installation.md +++ b/docs/features/software-templates/installation.md @@ -70,6 +70,10 @@ creating a file called `packages/backend/src/plugins/scaffolder.ts` with the following contents to get you up and running quickly. ```ts +import { + DockerContainerRunner, + SingleHostDiscovery, +} from '@backstage/backend-common'; import { CookieCutter, createRouter, @@ -78,7 +82,6 @@ import { CreateReactAppTemplater, Templaters, } from '@backstage/plugin-scaffolder-backend'; -import { SingleHostDiscovery } from '@backstage/backend-common'; import type { PluginEnvironment } from '../types'; import Docker from 'dockerode'; import { CatalogClient } from '@backstage/catalog-client'; @@ -89,8 +92,11 @@ export default async function createPlugin({ database, reader, }: PluginEnvironment) { - const cookiecutterTemplater = new CookieCutter(); - const craTemplater = new CreateReactAppTemplater(); + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + + const cookiecutterTemplater = new CookieCutter({ containerRunner }); + const craTemplater = new CreateReactAppTemplater({ containerRunner }); const templaters = new Templaters(); templaters.register('cookiecutter', cookiecutterTemplater); @@ -99,8 +105,6 @@ export default async function createPlugin({ const preparers = await Preparers.fromConfig(config, { logger }); const publishers = await Publishers.fromConfig(config, { logger }); - const dockerClient = new Docker(); - const discovery = SingleHostDiscovery.fromConfig(config); const catalogClient = new CatalogClient({ discoveryApi: discovery }); @@ -110,7 +114,6 @@ export default async function createPlugin({ publishers, logger, config, - dockerClient, database, catalogClient, reader, diff --git a/docs/features/software-templates/writing-custom-actions.md b/docs/features/software-templates/writing-custom-actions.md index ff145ee5af..90dd603c8f 100644 --- a/docs/features/software-templates/writing-custom-actions.md +++ b/docs/features/software-templates/writing-custom-actions.md @@ -106,7 +106,6 @@ return await createRouter({ publishers, logger, config, - dockerClient, database, catalogClient, reader, @@ -124,7 +123,6 @@ return await createRouter({ publishers, logger, config, - dockerClient, database, catalogClient, reader, @@ -136,11 +134,9 @@ return await createRouter({ want to have those as well as your new one, you'll need to do the following: ```ts - -import { createBuiltinActions } from '@backstage/plugin-scaffolder-backend`; +import { createBuiltinActions } from '@backstage/plugin-scaffolder-backend'; const builtInActions = createBuiltinActions({ - dockerClient, integrations, catalogClient, templaters, @@ -155,7 +151,6 @@ return await createRouter({ publishers, logger, config, - dockerClient, database, catalogClient, reader, diff --git a/docs/features/techdocs/getting-started.md b/docs/features/techdocs/getting-started.md index 520db2245c..51f07d5cde 100644 --- a/docs/features/techdocs/getting-started.md +++ b/docs/features/techdocs/getting-started.md @@ -63,6 +63,7 @@ Create a file called `techdocs.ts` inside `packages/backend/src/plugins/` and add the following ```typescript +import { DockerContainerRunner } from '@backstage/backend-common'; import { createRouter, Generators, @@ -84,9 +85,14 @@ export default async function createPlugin({ reader, }); + // Docker client (conditionally) used by the generators, based on techdocs.generators config. + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + // Generators are used for generating documentation sites. const generators = await Generators.fromConfig(config, { logger, + containerRunner, }); // Publisher is used for @@ -97,14 +103,13 @@ export default async function createPlugin({ discovery, }); - // Docker client (conditionally) used by the generators, based on techdocs.generators config. - const dockerClient = new Docker(); + // checks if the publisher is working and logs the result + await publisher.getReadiness(); return await createRouter({ preparers, generators, publisher, - dockerClient, logger, config, discovery, diff --git a/packages/backend-common/src/util/ContainerRunner.ts b/packages/backend-common/src/util/ContainerRunner.ts new file mode 100644 index 0000000000..7740e384e4 --- /dev/null +++ b/packages/backend-common/src/util/ContainerRunner.ts @@ -0,0 +1,31 @@ +/* + * 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 { Writable } from 'stream'; + +export type RunContainerOptions = { + imageName: string; + command?: string | string[]; + args: string[]; + logStream?: Writable; + mountDirs?: Record; + workingDir?: string; + envVars?: Record; +}; + +export interface ContainerRunner { + runContainer(opts: RunContainerOptions): Promise; +} diff --git a/packages/backend-common/src/util/docker.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts similarity index 89% rename from packages/backend-common/src/util/docker.test.ts rename to packages/backend-common/src/util/DockerContainerRunner.test.ts index 2ec14494da..942264af84 100644 --- a/packages/backend-common/src/util/docker.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -13,17 +13,21 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import Docker from 'dockerode'; import mockFs from 'mock-fs'; import os from 'os'; import path from 'path'; import Stream, { PassThrough } from 'stream'; -import { runDockerContainer, UserOptions } from './docker'; +import { ContainerRunner } from './ContainerRunner'; +import { DockerContainerRunner, UserOptions } from './DockerContainerRunner'; const mockDocker = new Docker() as jest.Mocked; const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; -describe('runDockerContainer', () => { +describe('DockerContainerRunner', () => { + let containerTaskApi: ContainerRunner; + beforeEach(() => { mockFs({ [rootDir]: { @@ -49,6 +53,8 @@ describe('runDockerContainer', () => { jest .spyOn(mockDocker, 'ping') .mockResolvedValue(Buffer.from('OK', 'utf-8')); + + containerTaskApi = new DockerContainerRunner({ dockerClient: mockDocker }); }); afterEach(() => { @@ -66,10 +72,9 @@ describe('runDockerContainer', () => { const envVarsArray = ['HOME=/tmp', 'LOG_LEVEL=debug']; it('should pull the docker container', async () => { - await runDockerContainer({ + await containerTaskApi.runContainer({ imageName, args, - dockerClient: mockDocker, }); expect(mockDocker.pull).toHaveBeenCalledWith( @@ -82,13 +87,12 @@ describe('runDockerContainer', () => { }); it('should call the dockerClient run command with the correct arguments passed through', async () => { - await runDockerContainer({ + await containerTaskApi.runContainer({ imageName, args, mountDirs, envVars, workingDir, - dockerClient: mockDocker, }); expect(mockDocker.run).toHaveBeenCalledWith( @@ -113,20 +117,18 @@ describe('runDockerContainer', () => { }); it('should ping docker to test availability', async () => { - await runDockerContainer({ + await containerTaskApi.runContainer({ imageName, args, - dockerClient: mockDocker, }); expect(mockDocker.ping).toHaveBeenCalled(); }); it('should pass through the user and group id from the host machine and set the home dir', async () => { - await runDockerContainer({ + await containerTaskApi.runContainer({ imageName, args, - dockerClient: mockDocker, }); const userOptions: UserOptions = {}; @@ -153,10 +155,9 @@ describe('runDockerContainer', () => { ]); await expect( - runDockerContainer({ + containerTaskApi.runContainer({ imageName, args, - dockerClient: mockDocker, }), ).rejects.toThrow(/Something went wrong with docker/); }); @@ -172,10 +173,9 @@ describe('runDockerContainer', () => { it('should throw with a descriptive error message including the docker error message', async () => { await expect( - runDockerContainer({ + containerTaskApi.runContainer({ imageName, args, - dockerClient: mockDocker, }), ).rejects.toThrow(new RegExp(`.+: ${dockerError}`)); }); @@ -183,11 +183,10 @@ describe('runDockerContainer', () => { it('should pass through the log stream to the docker client', async () => { const logStream = new PassThrough(); - await runDockerContainer({ + await containerTaskApi.runContainer({ imageName, args, logStream, - dockerClient: mockDocker, }); expect(mockDocker.run).toHaveBeenCalledWith( diff --git a/packages/backend-common/src/util/DockerContainerRunner.ts b/packages/backend-common/src/util/DockerContainerRunner.ts new file mode 100644 index 0000000000..a96ca1351b --- /dev/null +++ b/packages/backend-common/src/util/DockerContainerRunner.ts @@ -0,0 +1,117 @@ +/* + * 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 Docker from 'dockerode'; +import fs from 'fs-extra'; +import { PassThrough } from 'stream'; +import { ContainerRunner, RunContainerOptions } from './ContainerRunner'; + +export type UserOptions = { + User?: string; +}; + +export class DockerContainerRunner implements ContainerRunner { + private readonly dockerClient: Docker; + + constructor({ dockerClient }: { dockerClient: Docker }) { + this.dockerClient = dockerClient; + } + + async runContainer({ + imageName, + command, + args, + logStream = new PassThrough(), + mountDirs = {}, + workingDir, + envVars = {}, + }: RunContainerOptions) { + // Show a better error message when Docker is unavailable. + try { + await this.dockerClient.ping(); + } catch (e) { + throw new Error( + `This operation requires Docker. Docker does not appear to be available. Docker.ping() failed with: ${e.message}`, + ); + } + + await new Promise((resolve, reject) => { + this.dockerClient.pull(imageName, {}, (err, stream) => { + if (err) return reject(err); + stream.pipe(logStream, { end: false }); + stream.on('end', () => resolve()); + stream.on('error', (error: Error) => reject(error)); + return undefined; + }); + }); + + const userOptions: UserOptions = {}; + if (process.getuid && process.getgid) { + // Files that are created inside the Docker container will be owned by + // root on the host system on non Mac systems, because of reasons. Mainly the fact that + // volume sharing is done using NFS on Mac and actual mounts in Linux world. + // So we set the user in the container as the same user and group id as the host. + // On Windows we don't have process.getuid nor process.getgid + userOptions.User = `${process.getuid()}:${process.getgid()}`; + } + + // Initialize volumes to mount based on mountDirs map + const Volumes: { [T: string]: object } = {}; + for (const containerDir of Object.values(mountDirs)) { + Volumes[containerDir] = {}; + } + + // Create bind volumes + const Binds: string[] = []; + for (const [hostDir, containerDir] of Object.entries(mountDirs)) { + // Need to use realpath here as Docker mounting does not like + // symlinks for binding volumes + const realHostDir = await fs.realpath(hostDir); + Binds.push(`${realHostDir}:${containerDir}`); + } + + // Create docker environment variables array + const Env = []; + for (const [key, value] of Object.entries(envVars)) { + Env.push(`${key}=${value}`); + } + + const [ + { Error: error, StatusCode: statusCode }, + ] = await this.dockerClient.run(imageName, args, logStream, { + Volumes, + HostConfig: { + Binds, + }, + ...(workingDir ? { WorkingDir: workingDir } : {}), + Entrypoint: command, + Env, + ...userOptions, + } as Docker.ContainerCreateOptions); + + if (error) { + throw new Error( + `Docker failed to run with the following error message: ${error}`, + ); + } + + if (statusCode !== 0) { + throw new Error( + `Docker container returned a non-zero exit code (${statusCode})`, + ); + } + } +} diff --git a/packages/backend-common/src/util/docker.ts b/packages/backend-common/src/util/docker.ts deleted file mode 100644 index 356d645465..0000000000 --- a/packages/backend-common/src/util/docker.ts +++ /dev/null @@ -1,137 +0,0 @@ -/* - * 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 Docker from 'dockerode'; -import fs from 'fs-extra'; -import { PassThrough, Writable } from 'stream'; - -export type UserOptions = { - User?: string; -}; - -export type RunDockerContainerOptions = { - imageName: string; - args: string[]; - logStream?: Writable; - dockerClient: Docker; - mountDirs?: Record; - workingDir?: string; - envVars?: Record; - createOptions?: Docker.ContainerCreateOptions; -}; - -/** - * - * @param options the options object - * @param options.imageName the image to run - * @param options.args the arguments to pass the container - * @param options.logStream the log streamer to capture log messages - * @param options.dockerClient the dockerClient to use - * @param options.mountDirs A map of host directories to mount on the container. - * Object Key: Path on host machine, Value: Path on Docker container - * @param options.workingDir Working dir in the container - * @param options.envVars Environment variables to set in the container. e.g. {'HOME': '/tmp'} - */ -export const runDockerContainer = async ({ - imageName, - args, - logStream = new PassThrough(), - dockerClient, - mountDirs = {}, - workingDir, - envVars = {}, - createOptions = {}, -}: RunDockerContainerOptions) => { - // Show a better error message when Docker is unavailable. - try { - await dockerClient.ping(); - } catch (e) { - throw new Error( - `This operation requires Docker. Docker does not appear to be available. Docker.ping() failed with: ${e.message}`, - ); - } - - await new Promise((resolve, reject) => { - dockerClient.pull(imageName, {}, (err, stream) => { - if (err) return reject(err); - stream.pipe(logStream, { end: false }); - stream.on('end', () => resolve()); - stream.on('error', (error: Error) => reject(error)); - return undefined; - }); - }); - - const userOptions: UserOptions = {}; - if (process.getuid && process.getgid) { - // Files that are created inside the Docker container will be owned by - // root on the host system on non Mac systems, because of reasons. Mainly the fact that - // volume sharing is done using NFS on Mac and actual mounts in Linux world. - // So we set the user in the container as the same user and group id as the host. - // On Windows we don't have process.getuid nor process.getgid - userOptions.User = `${process.getuid()}:${process.getgid()}`; - } - - // Initialize volumes to mount based on mountDirs map - const Volumes: { [T: string]: object } = {}; - for (const containerDir of Object.values(mountDirs)) { - Volumes[containerDir] = {}; - } - - // Create bind volumes - const Binds: string[] = []; - for (const [hostDir, containerDir] of Object.entries(mountDirs)) { - // Need to use realpath here as Docker mounting does not like - // symlinks for binding volumes - const realHostDir = await fs.realpath(hostDir); - Binds.push(`${realHostDir}:${containerDir}`); - } - - // Create docker environment variables array - const Env = []; - for (const [key, value] of Object.entries(envVars)) { - Env.push(`${key}=${value}`); - } - - const [{ Error: error, StatusCode: statusCode }] = await dockerClient.run( - imageName, - args, - logStream, - { - Volumes, - HostConfig: { - Binds, - }, - ...(workingDir ? { WorkingDir: workingDir } : {}), - Env, - ...userOptions, - ...createOptions, - }, - ); - - if (error) { - throw new Error( - `Docker failed to run with the following error message: ${error}`, - ); - } - - if (statusCode !== 0) { - throw new Error( - `Docker container returned a non-zero exit code (${statusCode})`, - ); - } - - return { error, statusCode }; -}; diff --git a/packages/backend-common/src/util/index.ts b/packages/backend-common/src/util/index.ts index 04715824b4..85c5436a2e 100644 --- a/packages/backend-common/src/util/index.ts +++ b/packages/backend-common/src/util/index.ts @@ -14,4 +14,5 @@ * limitations under the License. */ -export { runDockerContainer } from './docker'; +export type { ContainerRunner, RunContainerOptions } from './ContainerRunner'; +export { DockerContainerRunner } from './DockerContainerRunner'; diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index fca3f79270..9c7c3c12f3 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { SingleHostDiscovery } from '@backstage/backend-common'; +import { + DockerContainerRunner, + SingleHostDiscovery, +} from '@backstage/backend-common'; import { CatalogClient } from '@backstage/catalog-client'; import { CookieCutter, @@ -34,8 +37,11 @@ export default async function createPlugin({ database, reader, }: PluginEnvironment): Promise { - const cookiecutterTemplater = new CookieCutter(); - const craTemplater = new CreateReactAppTemplater(); + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + + const cookiecutterTemplater = new CookieCutter({ containerRunner }); + const craTemplater = new CreateReactAppTemplater({ containerRunner }); const templaters = new Templaters(); templaters.register('cookiecutter', cookiecutterTemplater); @@ -44,8 +50,6 @@ export default async function createPlugin({ const preparers = await Preparers.fromConfig(config, { logger }); const publishers = await Publishers.fromConfig(config, { logger }); - const dockerClient = new Docker(); - const discovery = SingleHostDiscovery.fromConfig(config); const catalogClient = new CatalogClient({ discoveryApi: discovery }); @@ -55,7 +59,6 @@ export default async function createPlugin({ publishers, logger, config, - dockerClient, database, catalogClient, reader, diff --git a/packages/backend/src/plugins/techdocs.ts b/packages/backend/src/plugins/techdocs.ts index d0387cd15d..dbdc455e09 100644 --- a/packages/backend/src/plugins/techdocs.ts +++ b/packages/backend/src/plugins/techdocs.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { DockerContainerRunner } from '@backstage/backend-common'; import { createRouter, Generators, @@ -35,9 +36,14 @@ export default async function createPlugin({ reader, }); + // Docker client (conditionally) used by the generators, based on techdocs.generators config. + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + // Generators are used for generating documentation sites. const generators = await Generators.fromConfig(config, { logger, + containerRunner, }); // Publisher is used for @@ -51,14 +57,10 @@ export default async function createPlugin({ // checks if the publisher is working and logs the result await publisher.getReadiness(); - // Docker client (conditionally) used by the generators, based on techdocs.generators config. - const dockerClient = new Docker(); - return await createRouter({ preparers, generators, publisher, - dockerClient, logger, config, discovery, diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index a3416c5080..333ffa11df 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -1,4 +1,7 @@ -import { SingleHostDiscovery } from '@backstage/backend-common'; +import { + DockerContainerRunner, + SingleHostDiscovery, +} from '@backstage/backend-common'; import { CatalogClient } from '@backstage/catalog-client'; import { CookieCutter, @@ -6,7 +9,7 @@ import { createRouter, Preparers, Publishers, - Templaters + Templaters, } from '@backstage/plugin-scaffolder-backend'; import Docker from 'dockerode'; import { Router } from 'express'; @@ -18,8 +21,11 @@ export default async function createPlugin({ database, reader, }: PluginEnvironment): Promise { - const cookiecutterTemplater = new CookieCutter(); - const craTemplater = new CreateReactAppTemplater(); + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + + const cookiecutterTemplater = new CookieCutter({ containerRunner }); + const craTemplater = new CreateReactAppTemplater({ containerRunner }); const templaters = new Templaters(); templaters.register('cookiecutter', cookiecutterTemplater); @@ -28,8 +34,6 @@ export default async function createPlugin({ const preparers = await Preparers.fromConfig(config, { logger }); const publishers = await Publishers.fromConfig(config, { logger }); - const dockerClient = new Docker(); - const discovery = SingleHostDiscovery.fromConfig(config); const catalogClient = new CatalogClient({ discoveryApi: discovery }); @@ -39,9 +43,8 @@ export default async function createPlugin({ publishers, logger, config, - dockerClient, database, catalogClient, - reader + reader, }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts index 231a7e7fd7..906d86d4a2 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts @@ -1,9 +1,9 @@ +import { DockerContainerRunner } from '@backstage/backend-common'; import { createRouter, - - Generators, Preparers, - - Publisher + Generators, + Preparers, + Publisher, } from '@backstage/plugin-techdocs-backend'; import Docker from 'dockerode'; import { Router } from 'express'; @@ -21,9 +21,14 @@ export default async function createPlugin({ reader, }); + // Docker client (conditionally) used by the generators, based on techdocs.generators config. + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + // Generators are used for generating documentation sites. const generators = await Generators.fromConfig(config, { logger, + containerRunner, }); // Publisher is used for @@ -37,14 +42,10 @@ export default async function createPlugin({ // checks if the publisher is working and logs the result await publisher.getReadiness(); - // Docker client (conditionally) used by the generators, based on techdocs.generators config. - const dockerClient = new Docker(); - return await createRouter({ preparers, generators, publisher, - dockerClient, logger, config, discovery, diff --git a/packages/techdocs-common/package.json b/packages/techdocs-common/package.json index e749b33919..1ac46163af 100644 --- a/packages/techdocs-common/package.json +++ b/packages/techdocs-common/package.json @@ -44,11 +44,9 @@ "@backstage/errors": "^0.1.1", "@backstage/integration": "^0.5.1", "@google-cloud/storage": "^5.6.0", - "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", "aws-sdk": "^2.840.0", "cross-fetch": "^3.0.6", - "dockerode": "^3.2.1", "express": "^4.17.1", "fs-extra": "^9.0.1", "git-url-parse": "^11.4.4", diff --git a/packages/techdocs-common/src/stages/generate/generators.test.ts b/packages/techdocs-common/src/stages/generate/generators.test.ts index 032aafbf49..610a3e8ad5 100644 --- a/packages/techdocs-common/src/stages/generate/generators.test.ts +++ b/packages/techdocs-common/src/stages/generate/generators.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { getVoidLogger } from '@backstage/backend-common'; +import { ContainerRunner, getVoidLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { Generators } from './generators'; import { TechdocsGenerator } from './techdocs'; @@ -30,6 +30,10 @@ const mockEntity = { }; describe('generators', () => { + const containerRunner: jest.Mocked = { + runContainer: jest.fn(), + }; + it('should return error if no generator is registered', async () => { const generators = new Generators(); @@ -40,7 +44,11 @@ describe('generators', () => { it('should return correct registered generator', async () => { const generators = new Generators(); - const techdocs = new TechdocsGenerator(logger, new ConfigReader({})); + const techdocs = new TechdocsGenerator({ + logger, + containerRunner, + config: new ConfigReader({}), + }); generators.register('techdocs', techdocs); diff --git a/packages/techdocs-common/src/stages/generate/generators.ts b/packages/techdocs-common/src/stages/generate/generators.ts index 57c4017b15..4a3af72931 100644 --- a/packages/techdocs-common/src/stages/generate/generators.ts +++ b/packages/techdocs-common/src/stages/generate/generators.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { ContainerRunner } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { Logger } from 'winston'; @@ -30,11 +31,18 @@ export class Generators implements GeneratorBuilder { static async fromConfig( config: Config, - { logger }: { logger: Logger }, + { + logger, + containerRunner, + }: { logger: Logger; containerRunner: ContainerRunner }, ): Promise { const generators = new Generators(); - const techdocsGenerator = new TechdocsGenerator(logger, config); + const techdocsGenerator = new TechdocsGenerator({ + logger, + containerRunner, + config, + }); generators.register('techdocs', techdocsGenerator); return generators; diff --git a/packages/techdocs-common/src/stages/generate/techdocs.ts b/packages/techdocs-common/src/stages/generate/techdocs.ts index 0dfa952a17..1a6b0a581c 100644 --- a/packages/techdocs-common/src/stages/generate/techdocs.ts +++ b/packages/techdocs-common/src/stages/generate/techdocs.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { runDockerContainer } from '@backstage/backend-common'; +import { ContainerRunner } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import path from 'path'; import { PassThrough } from 'stream'; @@ -48,20 +48,29 @@ const createStream = (): [string[], PassThrough] => { export class TechdocsGenerator implements GeneratorBase { private readonly logger: Logger; + private readonly containerRunner: ContainerRunner; private readonly options: TechdocsGeneratorOptions; - constructor(logger: Logger, config: Config) { + constructor({ + logger, + containerRunner, + config, + }: { + logger: Logger; + containerRunner: ContainerRunner; + config: Config; + }) { this.logger = logger; this.options = { runGeneratorIn: config.getOptionalString('techdocs.generators.techdocs') ?? 'docker', }; + this.containerRunner = containerRunner; } public async run({ inputDir, outputDir, - dockerClient, parsedLocationAnnotation, etag, }: GeneratorRunOptions): Promise { @@ -100,7 +109,7 @@ export class TechdocsGenerator implements GeneratorBase { ); break; case 'docker': - await runDockerContainer({ + await this.containerRunner.runContainer({ imageName: 'spotify/techdocs', args: ['build', '-d', '/output'], logStream, @@ -109,7 +118,6 @@ export class TechdocsGenerator implements GeneratorBase { // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, - dockerClient, }); this.logger.info( `Successfully generated docs from ${inputDir} into ${outputDir} using techdocs-container`, diff --git a/packages/techdocs-common/src/stages/generate/types.ts b/packages/techdocs-common/src/stages/generate/types.ts index fda41ed442..249b809592 100644 --- a/packages/techdocs-common/src/stages/generate/types.ts +++ b/packages/techdocs-common/src/stages/generate/types.ts @@ -14,7 +14,6 @@ * limitations under the License. */ import { Entity } from '@backstage/catalog-model'; -import Docker from 'dockerode'; import { Writable } from 'stream'; import { ParsedLocationAnnotation } from '../../helpers'; @@ -23,7 +22,6 @@ import { ParsedLocationAnnotation } from '../../helpers'; * * @param {string} inputDir The directory of the uncompiled documentation, with the values from the frontend * @param {string} outputDir Directory to store generated docs in. Usually - a newly created temporary directory. - * @param {Docker} dockerClient A docker client to run any generator on top of your directory * @param {ParsedLocationAnnotation} parsedLocationAnnotation backstage.io/techdocs-ref annotation of an entity * @param {string} etag A unique identifier for the prepared tree e.g. commit SHA. If provided it will be stored in techdocs_metadata.json. * @param {Writable} [logStream] A dedicated log stream @@ -31,7 +29,6 @@ import { ParsedLocationAnnotation } from '../../helpers'; export type GeneratorRunOptions = { inputDir: string; outputDir: string; - dockerClient: Docker; parsedLocationAnnotation?: ParsedLocationAnnotation; etag?: string; logStream?: Writable; diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index bbeeecceb0..61dc4e2cbf 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -38,7 +38,6 @@ "@gitbeaker/core": "^28.0.2", "@gitbeaker/node": "^28.0.2", "@octokit/rest": "^18.5.3", - "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", "@types/git-url-parse": "^9.0.0", "azure-devops-node-api": "^10.1.1", @@ -46,7 +45,6 @@ "compression": "^1.7.4", "cors": "^2.8.5", "cross-fetch": "^3.0.6", - "dockerode": "^3.2.1", "express": "^4.17.1", "express-promise-router": "^4.1.0", "fs-extra": "^9.0.0", diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index 2d4b8fb4c5..0209fc50dd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -17,6 +17,7 @@ import { UrlReader } from '@backstage/backend-common'; import { CatalogApi } from '@backstage/catalog-client'; import { ScmIntegrations } from '@backstage/integration'; +import { TemplaterBuilder } from '../../stages'; import { createCatalogRegisterAction } from './catalog'; import { createFetchCookiecutterAction, createFetchPlainAction } from './fetch'; import { @@ -26,23 +27,14 @@ import { createPublishGithubPullRequestAction, createPublishGitlabAction, } from './publish'; -import Docker from 'dockerode'; -import { TemplaterBuilder } from '../../stages'; export const createBuiltinActions = (options: { reader: UrlReader; integrations: ScmIntegrations; - dockerClient: Docker; catalogClient: CatalogApi; templaters: TemplaterBuilder; }) => { - const { - reader, - integrations, - dockerClient, - templaters, - catalogClient, - } = options; + const { reader, integrations, templaters, catalogClient } = options; return [ createFetchPlainAction({ @@ -52,7 +44,6 @@ export const createBuiltinActions = (options: { createFetchCookiecutterAction({ reader, integrations, - dockerClient, templaters, }), createPublishGithubAction({ diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.test.ts index 628107155d..a0f94816bd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.test.ts @@ -15,16 +15,16 @@ */ jest.mock('./helpers'); +import { getVoidLogger, UrlReader } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; +import mock from 'mock-fs'; import os from 'os'; import { resolve as resolvePath } from 'path'; -import { createFetchCookiecutterAction } from './cookiecutter'; -import { ScmIntegrations } from '@backstage/integration'; -import { ConfigReader } from '@backstage/config'; -import { Templaters } from '../../../stages/templater'; import { PassThrough } from 'stream'; -import { getVoidLogger, UrlReader } from '@backstage/backend-common'; +import { Templaters } from '../../../stages/templater'; +import { createFetchCookiecutterAction } from './cookiecutter'; import { fetchContents } from './helpers'; -import mock from 'mock-fs'; describe('fetch:cookiecutter', () => { const integrations = ScmIntegrations.fromConfig( @@ -40,7 +40,6 @@ describe('fetch:cookiecutter', () => { const templaters = new Templaters(); const cookiecutterTemplater = { run: jest.fn() }; - const mockDockerClient = {}; const mockTmpDir = os.tmpdir(); const mockContext = { input: { @@ -67,7 +66,6 @@ describe('fetch:cookiecutter', () => { const action = createFetchCookiecutterAction({ integrations, templaters, - dockerClient: mockDockerClient as any, reader: mockReader, }); @@ -102,7 +100,6 @@ describe('fetch:cookiecutter', () => { expect(cookiecutterTemplater.run).toHaveBeenCalledWith({ workspacePath: mockTmpDir, - dockerClient: mockDockerClient, logStream: mockContext.logStream, values: mockContext.input.values, }); @@ -123,7 +120,6 @@ describe('fetch:cookiecutter', () => { expect(cookiecutterTemplater.run).toHaveBeenCalledWith({ workspacePath: mockTmpDir, - dockerClient: mockDockerClient, logStream: mockContext.logStream, values: { ...mockContext.input.values, @@ -166,7 +162,6 @@ describe('fetch:cookiecutter', () => { const newAction = createFetchCookiecutterAction({ integrations, templaters: templatersWithoutCookiecutter, - dockerClient: mockDockerClient as any, reader: mockReader, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts index fca791c7bd..83b373c3de 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts @@ -14,24 +14,22 @@ * limitations under the License. */ -import fs from 'fs-extra'; -import { resolve as resolvePath } from 'path'; -import Docker from 'dockerode'; import { UrlReader } from '@backstage/backend-common'; +import { JsonObject } from '@backstage/config'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; -import { JsonObject } from '@backstage/config'; +import fs from 'fs-extra'; +import { resolve as resolvePath } from 'path'; import { TemplaterBuilder, TemplaterValues } from '../../../stages/templater'; -import { fetchContents } from './helpers'; import { createTemplateAction } from '../../createTemplateAction'; +import { fetchContents } from './helpers'; export function createFetchCookiecutterAction(options: { - dockerClient: Docker; reader: UrlReader; integrations: ScmIntegrations; templaters: TemplaterBuilder; }) { - const { dockerClient, reader, templaters, integrations } = options; + const { reader, templaters, integrations } = options; return createTemplateAction<{ url: string; @@ -134,7 +132,6 @@ export function createFetchCookiecutterAction(options: { // Will execute the template in ./template and put the result in ./result await cookiecutter.run({ workspacePath: workDir, - dockerClient, logStream: ctx.logStream, values, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 425fc07946..005df09fec 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -19,20 +19,12 @@ import fs from 'fs-extra'; import { Processor, Job, StageContext, StageInput } from './types'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import * as uuid from 'uuid'; -import Docker from 'dockerode'; import path from 'path'; -import { TemplaterValues, TemplaterBase } from '../stages/templater'; -import { PreparerBuilder } from '../stages/prepare'; +import { TemplaterValues } from '../stages/templater'; import { makeLogStream } from './logger'; import { Logger } from 'winston'; import { Config } from '@backstage/config'; -export type JobProcessorArguments = { - preparers: PreparerBuilder; - templater: TemplaterBase; - dockerClient: Docker; -}; - export type JobAndDirectoryTuple = { job: Job; directory: string; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts index c7c52fa320..bb7ab40d57 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -14,21 +14,19 @@ * limitations under the License. */ -import { FilePreparer, PreparerBuilder } from './prepare'; -import Docker from 'dockerode'; -import { TemplaterBuilder, TemplaterValues } from './templater'; -import { PublisherBuilder } from './publish'; import { createTemplateAction } from '../actions'; +import { FilePreparer, PreparerBuilder } from './prepare'; +import { PublisherBuilder } from './publish'; +import { TemplaterBuilder, TemplaterValues } from './templater'; type Options = { - dockerClient: Docker; preparers: PreparerBuilder; templaters: TemplaterBuilder; publishers: PublisherBuilder; }; export function createLegacyActions(options: Options) { - const { dockerClient, preparers, templaters, publishers } = options; + const { preparers, templaters, publishers } = options; return [ createTemplateAction({ @@ -55,7 +53,6 @@ export function createLegacyActions(options: Options) { const templater = templaters.get(ctx.input.templater as string); await templater.run({ workspacePath: ctx.workspacePath, - dockerClient, logStream: ctx.logStream, values: ctx.input.values as TemplaterValues, }); 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 7806f089ff..28e2800e55 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -14,16 +14,14 @@ * limitations under the License. */ -const runDockerContainer = jest.fn(); const runCommand = jest.fn(); const commandExists = jest.fn(); jest.mock('./helpers', () => ({ runCommand })); -jest.mock('@backstage/backend-common', () => ({ runDockerContainer })); jest.mock('command-exists-promise', () => commandExists); jest.mock('fs-extra'); -import Docker from 'dockerode'; +import { ContainerRunner } from '@backstage/backend-common'; import fs from 'fs-extra'; import parseGitUrl from 'git-url-parse'; import path from 'path'; @@ -31,7 +29,9 @@ import { PassThrough } from 'stream'; import { CookieCutter } from './cookiecutter'; describe('CookieCutter Templater', () => { - const mockDocker = {} as Docker; + const containerRunner: jest.Mocked = { + runContainer: jest.fn(), + }; beforeEach(() => { jest.clearAllMocks(); @@ -50,11 +50,10 @@ describe('CookieCutter Templater', () => { jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing'] as any); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await templater.run({ workspacePath: 'tempdir', values, - dockerClient: mockDocker, }); expect(fs.ensureDir).toBeCalledWith(path.join('tempdir', 'intermediate')); @@ -83,11 +82,10 @@ describe('CookieCutter Templater', () => { }, }; - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await templater.run({ workspacePath: 'tempdir', values, - dockerClient: mockDocker, }); expect(fs.writeJSON).toBeCalledWith( @@ -115,12 +113,11 @@ describe('CookieCutter Templater', () => { }, }; - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await expect( templater.run({ workspacePath: 'tempdir', values, - dockerClient: mockDocker, }), ).rejects.toThrow('BAM'); }); @@ -140,14 +137,13 @@ describe('CookieCutter Templater', () => { .spyOn(fs, 'realpath') .mockImplementation(x => Promise.resolve(x.toString())); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await templater.run({ workspacePath: 'tempdir', values, - dockerClient: mockDocker, }); - expect(runDockerContainer).toHaveBeenCalledWith({ + expect(containerRunner.runContainer).toHaveBeenCalledWith({ imageName: 'spotify/backstage-cookiecutter', args: [ 'cookiecutter', @@ -164,7 +160,6 @@ describe('CookieCutter Templater', () => { }, workingDir: '/input', logStream: undefined, - dockerClient: mockDocker, }); }); @@ -177,14 +172,13 @@ describe('CookieCutter Templater', () => { jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing'] as any); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await templater.run({ workspacePath: 'tempdir', values, - dockerClient: mockDocker, }); - expect(runDockerContainer).toHaveBeenCalledWith( + expect(containerRunner.runContainer).toHaveBeenCalledWith( expect.objectContaining({ imageName: 'foo/cookiecutter-image-with-extensions', }), @@ -205,15 +199,14 @@ describe('CookieCutter Templater', () => { jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing'] as any); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await templater.run({ workspacePath: 'tempdir', values, logStream: stream, - dockerClient: mockDocker, }); - expect(runDockerContainer).toHaveBeenCalledWith({ + expect(containerRunner.runContainer).toHaveBeenCalledWith({ imageName: 'spotify/backstage-cookiecutter', args: [ 'cookiecutter', @@ -230,7 +223,6 @@ describe('CookieCutter Templater', () => { }, workingDir: '/input', logStream: stream, - dockerClient: mockDocker, }); }); @@ -250,12 +242,11 @@ describe('CookieCutter Templater', () => { jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing'] as any); commandExists.mockImplementationOnce(() => () => true); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await templater.run({ workspacePath: 'tempdir', values, logStream: stream, - dockerClient: mockDocker, }); expect(runCommand).toHaveBeenCalledWith({ @@ -280,7 +271,7 @@ describe('CookieCutter Templater', () => { .spyOn(fs, 'readdir') .mockImplementationOnce(() => Promise.resolve([])); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); await expect( templater.run({ workspacePath: 'tempdir', @@ -292,7 +283,6 @@ describe('CookieCutter Templater', () => { }, }, logStream: stream, - dockerClient: mockDocker, }), ).rejects.toThrow(/No data generated by cookiecutter/); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts index e60c5efb4d..8819a90d63 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { runDockerContainer } from '@backstage/backend-common'; +import { ContainerRunner } from '@backstage/backend-common'; import { JsonValue } from '@backstage/config'; import fs from 'fs-extra'; import path from 'path'; @@ -24,6 +24,12 @@ import { TemplaterBase, TemplaterRunOptions } from './types'; const commandExists = require('command-exists-promise'); export class CookieCutter implements TemplaterBase { + private readonly containerRunner: ContainerRunner; + + constructor({ containerRunner }: { containerRunner: ContainerRunner }) { + this.containerRunner = containerRunner; + } + private async fetchTemplateCookieCutter( directory: string, ): Promise> { @@ -40,7 +46,6 @@ export class CookieCutter implements TemplaterBase { public async run({ workspacePath, - dockerClient, values, logStream, }: TemplaterRunOptions): Promise { @@ -74,23 +79,16 @@ export class CookieCutter implements TemplaterBase { logStream, }); } else { - await runDockerContainer({ + await this.containerRunner.runContainer({ imageName: imageName || 'spotify/backstage-cookiecutter', - args: [ - 'cookiecutter', - '--no-input', - '-o', - '/output', - '/input', - '--verbose', - ], + command: 'cookiecutter', + args: ['--no-input', '-o', '/output', '/input', '--verbose'], mountDirs, workingDir: '/input', // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, logStream, - dockerClient, }); } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts index 3d03b002c0..36716b6c99 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { runDockerContainer } from '@backstage/backend-common'; +import { ContainerRunner } from '@backstage/backend-common'; import fs from 'fs-extra'; import path from 'path'; import * as yaml from 'yaml'; @@ -25,11 +25,16 @@ import { TemplaterBase, TemplaterRunOptions } from '../types'; const GITHUB_ACTIONS_ANNOTATION = 'github.com/project-slug'; export class CreateReactAppTemplater implements TemplaterBase { + private readonly containerRunner: ContainerRunner; + + constructor({ containerRunner }: { containerRunner: ContainerRunner }) { + this.containerRunner = containerRunner; + } + public async run({ workspacePath, values, logStream, - dockerClient, }: TemplaterRunOptions): Promise { const { component_id: componentName, @@ -46,23 +51,20 @@ export class CreateReactAppTemplater implements TemplaterBase { [intermediateDir]: '/result', }; - await runDockerContainer({ + await this.containerRunner.runContainer({ imageName: 'node:lts-alpine', + command: ['npx'], args: [ 'create-react-app', componentName as string, withTypescript ? ' --template typescript' : '', ], mountDirs, + workingDir: '/result', logStream: logStream, - dockerClient: dockerClient, // Set the home directory inside the container as something that applications can // write to, otherwise they will just fail trying to write to / envVars: { HOME: '/tmp' }, - createOptions: { - Entrypoint: ['npx'], - WorkingDir: '/result', - }, }); // if cookiecutter was successful, intermediateDir will contain diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts index d390bfbd9d..713ec514c0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts @@ -14,10 +14,15 @@ * limitations under the License. */ +import { ContainerRunner } from '@backstage/backend-common'; import { CookieCutter } from './cookiecutter'; import { Templaters } from './templaters'; describe('Templaters', () => { + const containerRunner: jest.Mocked = { + runContainer: jest.fn(), + }; + it('should throw an error when the templater is not registered', () => { const templaters = new Templaters(); @@ -29,7 +34,7 @@ describe('Templaters', () => { }); it('should return the correct templater when the templater matches', () => { const templaters = new Templaters(); - const templater = new CookieCutter(); + const templater = new CookieCutter({ containerRunner }); templaters.register('cookiecutter', templater); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts index fa14788d22..c5fe454e33 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts @@ -13,9 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import type { Writable } from 'stream'; -import Docker from 'dockerode'; + import gitUrlParse from 'git-url-parse'; +import type { Writable } from 'stream'; /** * Currently the required template values. The owner @@ -48,7 +48,6 @@ export type TemplaterRunOptions = { workspacePath: string; values: TemplaterValues; logStream?: Writable; - dockerClient: Docker; }; export type TemplaterBase = { diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index bb52edaaea..e8aa76a63a 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -29,20 +29,17 @@ jest.doMock('fs-extra', () => ({ })); import { - SingleConnectionDatabaseManager, - PluginDatabaseManager, getVoidLogger, + PluginDatabaseManager, + SingleConnectionDatabaseManager, UrlReaders, } from '@backstage/backend-common'; +import { CatalogApi } from '@backstage/catalog-client'; import { ConfigReader } from '@backstage/config'; import express from 'express'; import request from 'supertest'; +import { Preparers, Publishers, Templaters } from '../scaffolder'; import { createRouter } from './router'; -import { Templaters, Preparers, Publishers } from '../scaffolder'; -import Docker from 'dockerode'; -import { CatalogApi } from '@backstage/catalog-client'; - -jest.mock('dockerode'); const createCatalogClient = (templates: any[] = []) => ({ @@ -115,7 +112,6 @@ describe('createRouter - working directory', () => { templaters: new Templaters(), publishers: new Publishers(), config: new ConfigReader(workDirConfig('/path')), - dockerClient: new Docker(), database: createDatabase(), catalogClient: createCatalogClient([template]), reader: mockUrlReader, @@ -130,7 +126,6 @@ describe('createRouter - working directory', () => { templaters: new Templaters(), publishers: new Publishers(), config: new ConfigReader(workDirConfig('/path')), - dockerClient: new Docker(), database: createDatabase(), catalogClient: createCatalogClient([template]), reader: mockUrlReader, @@ -160,7 +155,6 @@ describe('createRouter - working directory', () => { templaters: new Templaters(), publishers: new Publishers(), config: new ConfigReader({}), - dockerClient: new Docker(), database: createDatabase(), catalogClient: createCatalogClient([template]), reader: mockUrlReader, @@ -234,7 +228,6 @@ describe('createRouter', () => { templaters: new Templaters(), publishers: new Publishers(), config: new ConfigReader({}), - dockerClient: new Docker(), database: createDatabase(), catalogClient: createCatalogClient([template]), reader: mockUrlReader, diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 35a57da6a2..5e0790526e 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -15,7 +15,6 @@ */ import { Config } from '@backstage/config'; -import Docker from 'dockerode'; import express from 'express'; import { resolve as resolvePath, dirname } from 'path'; import Router from 'express-promise-router'; @@ -62,7 +61,6 @@ export interface RouterOptions { logger: Logger; config: Config; reader: UrlReader; - dockerClient: Docker; database: PluginDatabaseManager; catalogClient: CatalogApi; actions?: TemplateAction[]; @@ -96,7 +94,6 @@ export async function createRouter( logger: parentLogger, config, reader, - dockerClient, database, catalogClient, actions, @@ -124,13 +121,11 @@ export async function createRouter( ? actions : [ ...createLegacyActions({ - dockerClient, preparers, publishers, templaters, }), ...createBuiltinActions({ - dockerClient, integrations, catalogClient, templaters, @@ -243,7 +238,6 @@ export async function createRouter( const templater = templaters.get(ctx.entity.spec.templater); await templater.run({ workspacePath: ctx.workspacePath, - dockerClient, logStream: ctx.logStream, values: ctx.values, }); diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 53e0865430..03d8e9e2ab 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -35,7 +35,6 @@ "@backstage/config": "^0.1.4", "@backstage/errors": "^0.1.1", "@backstage/techdocs-common": "^0.5.0", - "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", "cross-fetch": "^3.0.6", "dockerode": "^3.2.1", @@ -47,6 +46,7 @@ }, "devDependencies": { "@backstage/cli": "^0.6.8", + "@types/dockerode": "^3.2.1", "supertest": "^6.1.3" }, "files": [ diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 9dd53f159e..d1aa1e08be 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -28,7 +28,6 @@ import { PublisherBase, UrlPreparer, } from '@backstage/techdocs-common'; -import Docker from 'dockerode'; import fs from 'fs-extra'; import os from 'os'; import path from 'path'; @@ -41,7 +40,6 @@ type DocsBuilderArguments = { publisher: PublisherBase; entity: Entity; logger: Logger; - dockerClient: Docker; }; export class DocsBuilder { @@ -50,7 +48,6 @@ export class DocsBuilder { private publisher: PublisherBase; private entity: Entity; private logger: Logger; - private dockerClient: Docker; constructor({ preparers, @@ -58,14 +55,12 @@ export class DocsBuilder { publisher, entity, logger, - dockerClient, }: DocsBuilderArguments) { this.preparer = preparers.get(entity); this.generator = generators.get(entity); this.publisher = publisher; this.entity = entity; this.logger = logger; - this.dockerClient = dockerClient; } public async build(): Promise { @@ -157,7 +152,6 @@ export class DocsBuilder { await this.generator.run({ inputDir: preparedDir, outputDir, - dockerClient: this.dockerClient, parsedLocationAnnotation, etag: newEtag, }); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 121ddddef5..9f383ea31d 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -24,7 +24,6 @@ import { PublisherBase, } from '@backstage/techdocs-common'; import fetch from 'cross-fetch'; -import Docker from 'dockerode'; import express from 'express'; import Router from 'express-promise-router'; import { Knex } from 'knex'; @@ -40,7 +39,6 @@ type RouterOptions = { discovery: PluginEndpointDiscovery; database?: Knex; // TODO: Make database required when we're implementing database stuff. config: Config; - dockerClient: Docker; }; export async function createRouter({ @@ -48,7 +46,6 @@ export async function createRouter({ generators, publisher, config, - dockerClient, logger, discovery, }: RouterOptions): Promise { @@ -165,7 +162,6 @@ export async function createRouter({ preparers, generators, publisher, - dockerClient, logger, entity, }); diff --git a/plugins/techdocs-backend/src/service/standaloneServer.ts b/plugins/techdocs-backend/src/service/standaloneServer.ts index 0fd23a4f7d..af03987870 100644 --- a/plugins/techdocs-backend/src/service/standaloneServer.ts +++ b/plugins/techdocs-backend/src/service/standaloneServer.ts @@ -16,21 +16,22 @@ import { createServiceBuilder, + DockerContainerRunner, SingleHostDiscovery, UrlReader, } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; +import { + DirectoryPreparer, + Generators, + Preparers, + Publisher, + TechdocsGenerator, +} from '@backstage/techdocs-common'; +import Docker from 'dockerode'; import { Server } from 'http'; import { Logger } from 'winston'; import { createRouter } from './router'; -import Docker from 'dockerode'; -import { - Preparers, - DirectoryPreparer, - Generators, - TechdocsGenerator, - Publisher, -} from '@backstage/techdocs-common'; -import { ConfigReader } from '@backstage/config'; export interface ServerOptions { port: number; @@ -65,21 +66,25 @@ export async function startStandaloneServer( ); preparers.register('dir', directoryPreparer); + const dockerClient = new Docker(); + const containerRunner = new DockerContainerRunner({ dockerClient }); + const generators = new Generators(); - const techdocsGenerator = new TechdocsGenerator(logger, config); + const techdocsGenerator = new TechdocsGenerator({ + logger, + containerRunner, + config, + }); generators.register('techdocs', techdocsGenerator); const publisher = await Publisher.fromConfig(config, { logger, discovery }); - const dockerClient = new Docker(); - logger.debug('Starting application server...'); const router = await createRouter({ preparers, generators, logger, publisher, - dockerClient, config, discovery, });