Merge pull request #24593 from backstage/techdocs-move-away-from-container-runners

Add Docker container runner to techdocs-node

.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.

.changeset/late-falcons-sort.md

@@ -0,0 +1,7 @@
+---
+'@backstage/create-app': patch
+'@backstage/plugin-techdocs-backend': patch
+'@techdocs/cli': patch
+---
+
+Removed `dockerode` dependency.

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

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"

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",

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...');

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"
   },

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

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<GeneratorBuilder>;

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",

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: {},
+      }),
+    );
+  });
+});

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<string, string>;
+    workingDir?: string;
+    envVars?: Record<string, string>;
+    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<void>((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})`,
+      );
+    }
+  }
+}

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<ContainerRunner> = {
-    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);

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<GeneratorBuilder> {

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'.`,

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;
 };
 
 /**

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