Merge pull request #4744 from backstage/orkohunter/move-runDockerContainer

packages/backend-common/package.json

@@ -35,12 +35,14 @@
     "@backstage/integration": "^0.5.0",
     "@octokit/rest": "^18.0.12",
     "@types/cors": "^2.8.6",
+    "@types/dockerode": "^3.2.1",
     "@types/express": "^4.17.6",
     "archiver": "^5.0.2",
     "compression": "^1.7.4",
     "concat-stream": "^2.0.0",
     "cors": "^2.8.5",
     "cross-fetch": "^3.0.6",
+    "dockerode": "^3.2.1",
     "express": "^4.17.1",
     "express-promise-router": "^3.0.3",
     "fs-extra": "^9.0.1",

packages/backend-common/src/index.ts

@@ -18,10 +18,11 @@ export * from './config';
 export * from './database';
 export * from './discovery';
 export * from './errors';
+export * from './hot';
 export * from './logging';
 export * from './middleware';
-export * from './reading';
-export * from './service';
 export * from './paths';
-export * from './hot';
+export * from './reading';
 export * from './scm';
+export * from './service';
+export * from './util';

packages/backend-common/src/util/docker.test.ts

@@ -0,0 +1,205 @@
+/*
+ * 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 mockFs from 'mock-fs';
+import os from 'os';
+import path from 'path';
+import Stream, { PassThrough } from 'stream';
+import { runDockerContainer, UserOptions } from './docker';
+
+const mockDocker = new Docker() as jest.Mocked<Docker>;
+const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir';
+
+describe('runDockerContainer', () => {
+  beforeEach(() => {
+    mockFs({
+      [rootDir]: {
+        input: mockFs.directory(),
+        output: mockFs.directory(),
+      },
+    });
+
+    jest.spyOn(mockDocker, 'pull').mockImplementation((async (
+      _image: string,
+      _something: any,
+      handler: (err: Error | undefined, stream: PassThrough) => void,
+    ) => {
+      const mockStream = new PassThrough();
+      handler(undefined, mockStream);
+      mockStream.end();
+    }) as any);
+
+    jest
+      .spyOn(mockDocker, 'run')
+      .mockResolvedValue([{ Error: null, StatusCode: 0 }]);
+
+    jest
+      .spyOn(mockDocker, 'ping')
+      .mockResolvedValue(Buffer.from('OK', 'utf-8'));
+  });
+
+  afterEach(() => {
+    mockFs.restore();
+  });
+
+  const imageName = 'dockerOrg/image';
+  const args = ['bash', '-c', 'echo test'];
+  const mountDirs = {
+    [path.join(rootDir, 'input')]: '/input',
+    [path.join(rootDir, 'output')]: '/output',
+  };
+  const workingDir = path.join(rootDir, 'input');
+  const envVars = { HOME: '/tmp', LOG_LEVEL: 'debug' };
+  const envVarsArray = ['HOME=/tmp', 'LOG_LEVEL=debug'];
+
+  it('should pull the docker container', async () => {
+    await runDockerContainer({
+      imageName,
+      args,
+      dockerClient: mockDocker,
+    });
+
+    expect(mockDocker.pull).toHaveBeenCalledWith(
+      imageName,
+      {},
+      expect.any(Function),
+    );
+
+    expect(mockDocker.run).toHaveBeenCalled();
+  });
+
+  it('should call the dockerClient run command with the correct arguments passed through', async () => {
+    await runDockerContainer({
+      imageName,
+      args,
+      mountDirs,
+      envVars,
+      workingDir,
+      dockerClient: mockDocker,
+    });
+
+    expect(mockDocker.run).toHaveBeenCalledWith(
+      imageName,
+      args,
+      expect.any(Stream),
+      expect.objectContaining({
+        Env: envVarsArray,
+        WorkingDir: workingDir,
+        HostConfig: {
+          Binds: expect.arrayContaining([
+            `${path.join(rootDir, 'input')}:/input`,
+            `${path.join(rootDir, 'output')}:/output`,
+          ]),
+        },
+        Volumes: {
+          '/input': {},
+          '/output': {},
+        },
+      }),
+    );
+  });
+
+  it('should ping docker to test availability', async () => {
+    await runDockerContainer({
+      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({
+      imageName,
+      args,
+      dockerClient: mockDocker,
+    });
+
+    const userOptions: UserOptions = {};
+    if (process.getuid && process.getgid) {
+      userOptions.User = `${process.getuid()}:${process.getgid()}`;
+    }
+
+    expect(mockDocker.run).toHaveBeenCalledWith(
+      imageName,
+      args,
+      expect.any(Stream),
+      expect.objectContaining({
+        ...userOptions,
+      }),
+    );
+  });
+
+  it('throws a correct error if the command fails in docker', async () => {
+    mockDocker.run.mockResolvedValueOnce([
+      {
+        Error: new Error('Something went wrong with docker'),
+        StatusCode: 0,
+      },
+    ]);
+
+    await expect(
+      runDockerContainer({
+        imageName,
+        args,
+        dockerClient: mockDocker,
+      }),
+    ).rejects.toThrow(/Something went wrong with docker/);
+  });
+
+  describe('where docker is unavailable', () => {
+    const dockerError = 'a docker error';
+
+    beforeEach(() => {
+      jest.spyOn(mockDocker, 'ping').mockImplementationOnce(() => {
+        throw new Error(dockerError);
+      });
+    });
+
+    it('should throw with a descriptive error message including the docker error message', async () => {
+      await expect(
+        runDockerContainer({
+          imageName,
+          args,
+          dockerClient: mockDocker,
+        }),
+      ).rejects.toThrow(new RegExp(`.+: ${dockerError}`));
+    });
+  });
+
+  it('should pass through the log stream to the docker client', async () => {
+    const logStream = new PassThrough();
+    await runDockerContainer({
+      imageName,
+      args,
+      logStream,
+      dockerClient: mockDocker,
+    });
+
+    expect(mockDocker.run).toHaveBeenCalledWith(
+      imageName,
+      args,
+      logStream,
+      expect.objectContaining({
+        HostConfig: {
+          Binds: [],
+        },
+        Volumes: {},
+      }),
+    );
+  });
+});

packages/backend-common/src/util/docker.ts

@@ -0,0 +1,137 @@
+/*
+ * 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<string, string>;
+  workingDir?: string;
+  envVars?: Record<string, string>;
+  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<void>((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 };
+};

packages/backend-common/src/util/index.ts

@@ -0,0 +1,17 @@
+/*
+ * 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.
+ */
+
+export { runDockerContainer } from './docker';

packages/techdocs-common/src/stages/generate/helpers.test.ts

@@ -14,12 +14,10 @@
  * limitations under the License.
  */
 import { getVoidLogger } from '@backstage/backend-common';
-import Docker from 'dockerode';
 import fs from 'fs-extra';
 import mockFs from 'mock-fs';
 import os from 'os';
 import path, { resolve as resolvePath } from 'path';
-import Stream, { PassThrough } from 'stream';
 import { ParsedLocationAnnotation } from '../../helpers';
 import { RemoteProtocol } from '../prepare/types';
 import {
@@ -28,9 +26,7 @@ import {
   getRepoUrlFromLocationAnnotation,
   isValidRepoUrlForMkdocs,
   patchMkdocsYmlPreBuild,
-  runDockerContainer,
   storeEtagMetadata,
-  UserOptions,
 } from './helpers';
 
 const mockEntity = {
@@ -41,8 +37,6 @@ const mockEntity = {
   },
 };
 
-const mockDocker = new Docker() as jest.Mocked<Docker>;
-
 const mkdocsYml = fs.readFileSync(
   resolvePath(__filename, '../__fixtures__/mkdocs.yml'),
 );
@@ -60,133 +54,6 @@ describe('helpers', () => {
     });
   });
 
-  describe('runDockerContainer', () => {
-    beforeEach(() => {
-      jest.spyOn(mockDocker, 'pull').mockImplementation((async (
-        _image: string,
-        _something: any,
-        handler: (err: Error | undefined, stream: PassThrough) => void,
-      ) => {
-        const mockStream = new PassThrough();
-        handler(undefined, mockStream);
-        mockStream.end();
-      }) as any);
-
-      jest
-        .spyOn(mockDocker, 'run')
-        .mockResolvedValue([{ Error: null, StatusCode: 0 }]);
-
-      jest
-        .spyOn(mockDocker, 'ping')
-        .mockResolvedValue(Buffer.from('OK', 'utf-8'));
-    });
-
-    const imageName = 'spotify/techdocs';
-    const args = ['build', '-d', '/result'];
-    const docsDir = os.tmpdir();
-    const outputDir = os.tmpdir();
-
-    it('should pull the techdocs docker container', async () => {
-      await runDockerContainer({
-        imageName,
-        args,
-        docsDir,
-        outputDir,
-        dockerClient: mockDocker,
-      });
-
-      expect(mockDocker.pull).toHaveBeenCalledWith(
-        imageName,
-        {},
-        expect.any(Function),
-      );
-    });
-
-    it('should run the techdocs docker container', async () => {
-      await runDockerContainer({
-        imageName,
-        args,
-        docsDir,
-        outputDir,
-        dockerClient: mockDocker,
-      });
-
-      expect(mockDocker.run).toHaveBeenCalledWith(
-        imageName,
-        args,
-        expect.any(Stream),
-        expect.objectContaining({
-          Volumes: {
-            '/content': {},
-            '/result': {},
-          },
-          WorkingDir: '/content',
-          HostConfig: {
-            Binds: [`${docsDir}:/content`, `${outputDir}:/result`],
-          },
-        }),
-      );
-    });
-
-    it('should ping docker to test availability', async () => {
-      await runDockerContainer({
-        imageName,
-        args,
-        docsDir,
-        outputDir,
-        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({
-        imageName,
-        args,
-        docsDir,
-        outputDir,
-        dockerClient: mockDocker,
-      });
-
-      const userOptions: UserOptions = {};
-      if (process.getuid && process.getgid) {
-        userOptions.User = `${process.getuid()}:${process.getgid()}`;
-      }
-
-      expect(mockDocker.run).toHaveBeenCalledWith(
-        imageName,
-        args,
-        expect.any(Stream),
-        expect.objectContaining({
-          ...userOptions,
-        }),
-      );
-    });
-
-    describe('where docker is unavailable', () => {
-      const dockerError = 'a docker error';
-
-      beforeEach(() => {
-        jest.spyOn(mockDocker, 'ping').mockImplementationOnce(() => {
-          throw new Error(dockerError);
-        });
-      });
-
-      it('should throw with a descriptive error message including the docker error message', async () => {
-        await expect(
-          runDockerContainer({
-            imageName,
-            args,
-            docsDir,
-            outputDir,
-            dockerClient: mockDocker,
-          }),
-        ).rejects.toThrow(new RegExp(`.+: ${dockerError}`));
-      });
-    });
-  });
-
   describe('isValidRepoUrlForMkdocs', () => {
     it('should return true for valid repo_url values for mkdocs', () => {
       const validRepoUrls = [

packages/techdocs-common/src/stages/generate/helpers.ts

@@ -16,7 +16,6 @@
 
 import { Entity } from '@backstage/catalog-model';
 import { spawn } from 'child_process';
-import Docker from 'dockerode';
 import fs from 'fs-extra';
 import yaml from 'js-yaml';
 import { PassThrough, Writable } from 'stream';
@@ -34,16 +33,6 @@ export function getGeneratorKey(entity: Entity): SupportedGeneratorKey {
   return 'techdocs';
 }
 
-type RunDockerContainerOptions = {
-  imageName: string;
-  args: string[];
-  logStream?: Writable;
-  docsDir: string;
-  outputDir: string;
-  dockerClient: Docker;
-  createOptions?: Docker.ContainerCreateOptions;
-};
-
 export type RunCommandOptions = {
   command: string;
   args: string[];
@@ -51,83 +40,6 @@ export type RunCommandOptions = {
   logStream?: Writable;
 };
 
-export type UserOptions = {
-  User?: string;
-};
-
-// To be replaced by a runDockerContainer from backend-common
-// shared between Scaffolder and TechDocs and any other plugin.
-export async function runDockerContainer({
-  imageName,
-  args,
-  logStream = new PassThrough(),
-  docsDir,
-  outputDir,
-  dockerClient,
-  createOptions,
-}: RunDockerContainerOptions) {
-  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<void>((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 = {};
-  // @ts-ignore
-  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()}`;
-  }
-
-  const [{ Error: error, StatusCode: statusCode }] = await dockerClient.run(
-    imageName,
-    args,
-    logStream,
-    {
-      Volumes: {
-        '/content': {},
-        '/result': {},
-      },
-      WorkingDir: '/content',
-      HostConfig: {
-        Binds: [`${docsDir}:/content`, `${outputDir}:/result`],
-      },
-      ...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 };
-}
-
 /**
  *
  * @param options the options object

packages/techdocs-common/src/stages/generate/techdocs.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { runDockerContainer } from '@backstage/backend-common';
 import { Config } from '@backstage/config';
 import path from 'path';
 import { PassThrough } from 'stream';
@@ -22,7 +23,6 @@ import {
   addBuildTimestampMetadata,
   patchMkdocsYmlPreBuild,
   runCommand,
-  runDockerContainer,
   storeEtagMetadata,
 } from './helpers';
 import { GeneratorBase, GeneratorRunOptions } from './types';
@@ -78,6 +78,12 @@ export class TechdocsGenerator implements GeneratorBase {
       );
     }
 
+    // Directories to bind on container
+    const mountDirs = {
+      [inputDir]: '/input',
+      [outputDir]: '/output',
+    };
+
     try {
       switch (this.options.runGeneratorIn) {
         case 'local':
@@ -96,10 +102,13 @@ export class TechdocsGenerator implements GeneratorBase {
         case 'docker':
           await runDockerContainer({
             imageName: 'spotify/techdocs',
-            args: ['build', '-d', '/result'],
+            args: ['build', '-d', '/output'],
             logStream,
-            docsDir: inputDir,
-            outputDir,
+            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' },
             dockerClient,
           });
           this.logger.info(
@@ -115,7 +124,7 @@ export class TechdocsGenerator implements GeneratorBase {
       this.logger.debug(
         `Failed to generate docs from ${inputDir} into ${outputDir}`,
       );
-      this.logger.debug(`Build failed with error: ${log}`);
+      this.logger.error(`Build failed with error: ${log}`);
       throw new Error(
         `Failed to generate docs from ${inputDir} into ${outputDir} with error ${error.message}`,
       );