diff --git a/.changeset/famous-roses-smell.md b/.changeset/famous-roses-smell.md new file mode 100644 index 0000000000..a091a6f32e --- /dev/null +++ b/.changeset/famous-roses-smell.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-node': patch +--- + +`TechdocsGenerator` won't require a `containerRunner` option anymore for generating TechDocs in docker. diff --git a/.changeset/late-falcons-sort.md b/.changeset/late-falcons-sort.md new file mode 100644 index 0000000000..3f6468e652 --- /dev/null +++ b/.changeset/late-falcons-sort.md @@ -0,0 +1,7 @@ +--- +'@backstage/create-app': patch +'@backstage/plugin-techdocs-backend': patch +'@techdocs/cli': patch +--- + +Removed `dockerode` dependency. diff --git a/packages/backend-legacy/src/plugins/techdocs.ts b/packages/backend-legacy/src/plugins/techdocs.ts index 6e369f8f0f..21048d4bb9 100644 --- a/packages/backend-legacy/src/plugins/techdocs.ts +++ b/packages/backend-legacy/src/plugins/techdocs.ts @@ -14,14 +14,12 @@ * limitations under the License. */ -import { DockerContainerRunner } from '@backstage/backend-common'; import { createRouter, Generators, Preparers, Publisher, } from '@backstage/plugin-techdocs-backend'; -import Docker from 'dockerode'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -34,14 +32,9 @@ export default async function createPlugin( reader: env.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(env.config, { logger: env.logger, - containerRunner, }); // Publisher is used for diff --git a/packages/create-app/templates/default-app/packages/backend/package.json.hbs b/packages/create-app/templates/default-app/packages/backend/package.json.hbs index 049ea1c30c..8d95ee60a5 100644 --- a/packages/create-app/templates/default-app/packages/backend/package.json.hbs +++ b/packages/create-app/templates/default-app/packages/backend/package.json.hbs @@ -40,14 +40,12 @@ "@backstage/plugin-techdocs-backend": "^{{version '@backstage/plugin-techdocs-backend'}}", "app": "link:../app", "better-sqlite3": "^9.0.0", - "dockerode": "^3.3.1", "node-gyp": "^10.0.0", "pg": "^8.11.3", "winston": "^3.2.1" }, "devDependencies": { "@backstage/cli": "^{{version '@backstage/cli'}}", - "@types/dockerode": "^3.3.0", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", "@types/luxon": "^2.0.4" diff --git a/packages/techdocs-cli/package.json b/packages/techdocs-cli/package.json index 74c90b4854..7a20eee17b 100644 --- a/packages/techdocs-cli/package.json +++ b/packages/techdocs-cli/package.json @@ -61,9 +61,7 @@ "@backstage/cli-common": "workspace:^", "@backstage/config": "workspace:^", "@backstage/plugin-techdocs-node": "workspace:^", - "@types/dockerode": "^3.3.0", "commander": "^12.0.0", - "dockerode": "^4.0.0", "fs-extra": "^11.0.0", "global-agent": "^3.0.0", "http-proxy": "^1.18.1", diff --git a/packages/techdocs-cli/src/commands/generate/generate.ts b/packages/techdocs-cli/src/commands/generate/generate.ts index bf6910ff4a..4553c18810 100644 --- a/packages/techdocs-cli/src/commands/generate/generate.ts +++ b/packages/techdocs-cli/src/commands/generate/generate.ts @@ -17,16 +17,11 @@ import { resolve } from 'path'; import { OptionValues } from 'commander'; import fs from 'fs-extra'; -import Docker from 'dockerode'; import { TechdocsGenerator, ParsedLocationAnnotation, getMkdocsYml, } from '@backstage/plugin-techdocs-node'; -import { - ContainerRunner, - DockerContainerRunner, -} from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { convertTechDocsRefToLocationAnnotation, @@ -75,14 +70,6 @@ export default async function generate(opts: OptionValues) { }, }); - // Docker client (conditionally) used by the generators, based on techdocs.generators config. - let containerRunner: ContainerRunner | undefined; - - if (opts.docker) { - const dockerClient = new Docker(); - containerRunner = new DockerContainerRunner({ dockerClient }); - } - let parsedLocationAnnotation = {} as ParsedLocationAnnotation; if (opts.techdocsRef) { try { @@ -97,7 +84,6 @@ export default async function generate(opts: OptionValues) { // Generate docs using @backstage/plugin-techdocs-node const techdocsGenerator = await TechdocsGenerator.fromConfig(config, { logger, - containerRunner, }); logger.info('Generating documentation...'); diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 1abf653784..3346214371 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -69,7 +69,6 @@ "@backstage/plugin-search-backend-module-techdocs": "workspace:^", "@backstage/plugin-techdocs-node": "workspace:^", "@types/express": "^4.17.6", - "dockerode": "^4.0.0", "express": "^4.17.1", "express-promise-router": "^4.1.0", "fs-extra": "^11.2.0", @@ -83,7 +82,6 @@ "@backstage/backend-defaults": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", - "@types/dockerode": "^3.3.0", "msw": "^1.0.0", "supertest": "^6.1.3" }, diff --git a/plugins/techdocs-backend/src/plugin.ts b/plugins/techdocs-backend/src/plugin.ts index 094335a0f0..e3a0c60a15 100644 --- a/plugins/techdocs-backend/src/plugin.ts +++ b/plugins/techdocs-backend/src/plugin.ts @@ -16,7 +16,6 @@ import { cacheToPluginCacheManager, - DockerContainerRunner, loggerToWinstonLogger, } from '@backstage/backend-common'; import { @@ -36,7 +35,6 @@ import { techdocsGeneratorExtensionPoint, techdocsPreparerExtensionPoint, } from '@backstage/plugin-techdocs-node'; -import Docker from 'dockerode'; import { createRouter } from '@backstage/plugin-techdocs-backend'; import * as winston from 'winston'; @@ -118,14 +116,9 @@ export const techdocsPlugin = createBackendPlugin({ preparers.register(protocol, preparer); } - // 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: winstonLogger, - containerRunner, customGenerator: customTechdocsGenerator, }); diff --git a/plugins/techdocs-node/api-report.md b/plugins/techdocs-node/api-report.md index 2d3ca90983..3966b7d2d8 100644 --- a/plugins/techdocs-node/api-report.md +++ b/plugins/techdocs-node/api-report.md @@ -48,8 +48,8 @@ export type GeneratorBuilder = { // @public export type GeneratorOptions = { - containerRunner?: ContainerRunner; logger: Logger; + containerRunner?: ContainerRunner; }; // @public @@ -72,7 +72,7 @@ export class Generators implements GeneratorBuilder { config: Config, options: { logger: Logger; - containerRunner: ContainerRunner; + containerRunner?: ContainerRunner; customGenerator?: TechdocsGenerator; }, ): Promise; diff --git a/plugins/techdocs-node/package.json b/plugins/techdocs-node/package.json index 7ef38c56ff..8b7f87b230 100644 --- a/plugins/techdocs-node/package.json +++ b/plugins/techdocs-node/package.json @@ -64,6 +64,7 @@ "@smithy/node-http-handler": "^2.1.7", "@trendyol-js/openstack-swift-sdk": "^0.0.7", "@types/express": "^4.17.6", + "dockerode": "^4.0.0", "express": "^4.17.1", "fs-extra": "^11.2.0", "git-url-parse": "^14.0.0", diff --git a/plugins/techdocs-node/src/stages/generate/DockerContainerRunner.test.ts b/plugins/techdocs-node/src/stages/generate/DockerContainerRunner.test.ts new file mode 100644 index 0000000000..4a9e364af3 --- /dev/null +++ b/plugins/techdocs-node/src/stages/generate/DockerContainerRunner.test.ts @@ -0,0 +1,218 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import fs from 'fs-extra'; +import Stream, { PassThrough } from 'stream'; +import { DockerContainerRunner, UserOptions } from './DockerContainerRunner'; +import { createMockDirectory } from '@backstage/backend-test-utils'; + +const mockPull = jest.fn(); +const mockRun = jest.fn(); +const mockPing = jest.fn(); + +jest.mock( + 'dockerode', + () => + function MockDocker() { + return { + pull: mockPull, + run: mockRun, + ping: mockPing, + }; + }, +); + +describe('DockerContainerRunner', () => { + let containerTaskApi: DockerContainerRunner; + + const inputDir = createMockDirectory(); + const outputDir = createMockDirectory(); + + beforeEach(() => { + inputDir.clear(); + outputDir.clear(); + + mockPull.mockImplementation( + ( + _repoTag: string, + _options: {}, + callback: (error?: any, result?: any) => void, + ) => { + const mockStream = new PassThrough(); + callback(undefined, mockStream); + mockStream.end(); + }, + ); + mockRun.mockResolvedValue([{ Error: null, StatusCode: 0 }]); + mockPing.mockResolvedValue(Buffer.from('OK', 'utf-8')); + + containerTaskApi = new DockerContainerRunner(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + const imageName = 'dockerorg/image'; + const args = ['bash', '-c', 'echo test']; + const mountDirs = { + [inputDir.path]: '/input', + [outputDir.path]: '/output', + }; + const workingDir = inputDir.path; + const envVars = { HOME: '/tmp', LOG_LEVEL: 'debug' }; + const envVarsArray = ['HOME=/tmp', 'LOG_LEVEL=debug']; + + it('should pull the docker container', async () => { + await containerTaskApi.runContainer({ + imageName, + args, + }); + + expect(mockPull).toHaveBeenCalledWith(imageName, {}, expect.any(Function)); + + expect(mockRun).toHaveBeenCalled(); + }); + + it('should not pull the docker container when pullImage is false', async () => { + await containerTaskApi.runContainer({ + imageName, + args, + pullImage: false, + }); + + expect(mockPull).not.toHaveBeenCalled(); + expect(mockRun).toHaveBeenCalled(); + }); + + it('should call the dockerClient run command with the correct arguments passed through', async () => { + await containerTaskApi.runContainer({ + imageName, + args, + mountDirs, + envVars, + workingDir, + }); + + expect(mockRun).toHaveBeenCalledWith( + imageName, + args, + expect.any(Stream), + expect.objectContaining({ + Env: envVarsArray, + WorkingDir: workingDir, + HostConfig: { + AutoRemove: true, + Binds: expect.arrayContaining([ + `${await fs.realpath(inputDir.path)}:/input`, + `${await fs.realpath(outputDir.path)}:/output`, + ]), + }, + Volumes: { + '/input': {}, + '/output': {}, + }, + }), + ); + }); + + it('should ping docker to test availability', async () => { + await containerTaskApi.runContainer({ + imageName, + args, + }); + + expect(mockPing).toHaveBeenCalled(); + }); + + it('should pass through the user and group id from the host machine and set the home dir', async () => { + await containerTaskApi.runContainer({ + imageName, + args, + }); + + const userOptions: UserOptions = {}; + if (process.getuid && process.getgid) { + userOptions.User = `${process.getuid()}:${process.getgid()}`; + } + + expect(mockRun).toHaveBeenCalledWith( + imageName, + args, + expect.any(Stream), + expect.objectContaining({ + ...userOptions, + }), + ); + }); + + it('throws a correct error if the command fails in docker', async () => { + mockRun.mockResolvedValueOnce([ + { + Error: new Error('Something went wrong with docker'), + StatusCode: 0, + }, + ]); + + await expect( + containerTaskApi.runContainer({ + imageName, + args, + }), + ).rejects.toThrow(/Something went wrong with docker/); + }); + + describe('where docker is unavailable', () => { + const dockerError = 'a docker error'; + + beforeEach(() => { + mockPing.mockImplementationOnce(() => { + throw new Error(dockerError); + }); + }); + + it('should throw with a descriptive error message including the docker error message', async () => { + await expect( + containerTaskApi.runContainer({ + imageName, + args, + }), + ).rejects.toThrow(new RegExp(`.+: ${dockerError}`)); + }); + }); + + it('should pass through the log stream to the docker client', async () => { + const logStream = new PassThrough(); + await containerTaskApi.runContainer({ + imageName, + args, + logStream, + }); + + expect(mockRun).toHaveBeenCalledWith( + imageName, + args, + logStream, + expect.objectContaining({ + HostConfig: { + AutoRemove: true, + Binds: [], + }, + Volumes: {}, + }), + ); + }); +}); diff --git a/plugins/techdocs-node/src/stages/generate/DockerContainerRunner.ts b/plugins/techdocs-node/src/stages/generate/DockerContainerRunner.ts new file mode 100644 index 0000000000..b7cb7f1c86 --- /dev/null +++ b/plugins/techdocs-node/src/stages/generate/DockerContainerRunner.ts @@ -0,0 +1,145 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import Docker from 'dockerode'; +import fs from 'fs-extra'; +import { ForwardedError } from '@backstage/errors'; +import { PassThrough } from 'stream'; +import { pipeline as pipelineStream } from 'stream'; +import { promisify } from 'util'; +import { Writable } from 'stream'; + +const pipeline = promisify(pipelineStream); + +export type UserOptions = { + User?: string; +}; + +/** + * @internal + */ +export class DockerContainerRunner { + private readonly dockerClient: Docker; + + constructor() { + this.dockerClient = new Docker(); + } + + async runContainer(options: { + imageName: string; + command?: string | string[]; + args: string[]; + logStream?: Writable; + mountDirs?: Record; + workingDir?: string; + envVars?: Record; + pullImage?: boolean; + defaultUser?: boolean; + }) { + const { + imageName, + command, + args, + logStream = new PassThrough(), + mountDirs = {}, + workingDir, + envVars = {}, + pullImage = true, + defaultUser = false, + } = options; + + // Show a better error message when Docker is unavailable. + try { + await this.dockerClient.ping(); + } catch (e) { + throw new ForwardedError( + 'This operation requires Docker. Docker does not appear to be available. Docker.ping() failed with', + e, + ); + } + + if (pullImage) { + await new Promise((resolve, reject) => { + this.dockerClient.pull(imageName, {}, (err, stream) => { + if (err) { + reject(err); + return; + } + + pipeline(stream, logStream, { end: false }) + .then(resolve) + .catch(reject); + }); + }); + } + + const userOptions: UserOptions = {}; + if (!defaultUser && 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: { + AutoRemove: true, + 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/plugins/techdocs-node/src/stages/generate/generators.test.ts b/plugins/techdocs-node/src/stages/generate/generators.test.ts index 3e0571af54..f80f070796 100644 --- a/plugins/techdocs-node/src/stages/generate/generators.test.ts +++ b/plugins/techdocs-node/src/stages/generate/generators.test.ts @@ -14,10 +14,7 @@ * limitations under the License. */ -import { - ContainerRunner, - loggerToWinstonLogger, -} from '@backstage/backend-common'; +import { loggerToWinstonLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { Generators } from './generators'; import { TechdocsGenerator } from './techdocs'; @@ -34,10 +31,6 @@ 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(); @@ -50,7 +43,6 @@ describe('generators', () => { const generators = new Generators(); const techdocs = TechdocsGenerator.fromConfig(new ConfigReader({}), { logger, - containerRunner, }); generators.register('techdocs', techdocs); diff --git a/plugins/techdocs-node/src/stages/generate/generators.ts b/plugins/techdocs-node/src/stages/generate/generators.ts index 603c714370..0c20ac1588 100644 --- a/plugins/techdocs-node/src/stages/generate/generators.ts +++ b/plugins/techdocs-node/src/stages/generate/generators.ts @@ -42,7 +42,7 @@ export class Generators implements GeneratorBuilder { config: Config, options: { logger: Logger; - containerRunner: ContainerRunner; + containerRunner?: ContainerRunner; customGenerator?: TechdocsGenerator; }, ): Promise { diff --git a/plugins/techdocs-node/src/stages/generate/techdocs.ts b/plugins/techdocs-node/src/stages/generate/techdocs.ts index c5130a3f8f..e5e2b7191e 100644 --- a/plugins/techdocs-node/src/stages/generate/techdocs.ts +++ b/plugins/techdocs-node/src/stages/generate/techdocs.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { ContainerRunner } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import path from 'path'; import { Logger } from 'winston'; @@ -43,6 +42,8 @@ import { GeneratorRunOptions, } from './types'; import { ForwardedError } from '@backstage/errors'; +import { DockerContainerRunner } from './DockerContainerRunner'; +import { ContainerRunner } from '@backstage/backend-common'; /** * Generates documentation files @@ -155,13 +156,10 @@ export class TechdocsGenerator implements GeneratorBase { `Successfully generated docs from ${inputDir} into ${outputDir} using local mkdocs`, ); break; - case 'docker': - if (this.containerRunner === undefined) { - throw new Error( - "Invalid state: containerRunner cannot be undefined when runIn is 'docker'", - ); - } - await this.containerRunner.runContainer({ + case 'docker': { + const containerRunner = + this.containerRunner || new DockerContainerRunner(); + await containerRunner.runContainer({ imageName: this.options.dockerImage ?? TechdocsGenerator.defaultDockerImage, args: ['build', '-d', '/output'], @@ -178,6 +176,7 @@ export class TechdocsGenerator implements GeneratorBase { `Successfully generated docs from ${inputDir} into ${outputDir} using techdocs-container`, ); break; + } default: throw new Error( `Invalid config value "${this.options.runIn}" provided in 'techdocs.generators.techdocs'.`, diff --git a/plugins/techdocs-node/src/stages/generate/types.ts b/plugins/techdocs-node/src/stages/generate/types.ts index 149c3c2195..ca816d560f 100644 --- a/plugins/techdocs-node/src/stages/generate/types.ts +++ b/plugins/techdocs-node/src/stages/generate/types.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { ContainerRunner } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Writable } from 'stream'; import { Logger } from 'winston'; import { ParsedLocationAnnotation } from '../../helpers'; +import { ContainerRunner } from '@backstage/backend-common'; // Determines where the generator will be run export type GeneratorRunInType = 'docker' | 'local'; @@ -28,8 +28,12 @@ export type GeneratorRunInType = 'docker' | 'local'; * @public */ export type GeneratorOptions = { - containerRunner?: ContainerRunner; logger: Logger; + /** + * @deprecated containerRunner is now instantiated in + * the generator and this option will be removed in the future + */ + containerRunner?: ContainerRunner; }; /** diff --git a/yarn.lock b/yarn.lock index bc91bd8c71..d69450fae8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7733,9 +7733,7 @@ __metadata: "@backstage/plugin-permission-common": "workspace:^" "@backstage/plugin-search-backend-module-techdocs": "workspace:^" "@backstage/plugin-techdocs-node": "workspace:^" - "@types/dockerode": ^3.3.0 "@types/express": ^4.17.6 - dockerode: ^4.0.0 express: ^4.17.1 express-promise-router: ^4.1.0 fs-extra: ^11.2.0 @@ -7807,6 +7805,7 @@ __metadata: "@types/recursive-readdir": ^2.2.0 "@types/supertest": ^2.0.8 aws-sdk-client-mock: ^4.0.0 + dockerode: ^4.0.0 express: ^4.17.1 fs-extra: ^11.2.0 git-url-parse: ^14.0.0 @@ -16613,14 +16612,12 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/plugin-techdocs-node": "workspace:^" "@types/commander": ^2.12.2 - "@types/dockerode": ^3.3.0 "@types/fs-extra": ^11.0.0 "@types/http-proxy": ^1.17.4 "@types/node": ^18.17.8 "@types/serve-handler": ^6.1.0 "@types/webpack-env": ^1.15.3 commander: ^12.0.0 - dockerode: ^4.0.0 find-process: ^1.4.5 fs-extra: ^11.0.0 global-agent: ^3.0.0