From c65dc26c105f4437e7b3527f9a798692fa7e49b8 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 6 Jan 2021 10:59:16 +0100 Subject: [PATCH 1/5] TechDocs: Generator can be given a pre-created output directory TechDocs Generator uses a temporary directory to store the generated files. But with techdocs-cli, the generator needs to output in a fixed directory. Also, make parsedLocationAnnotation optional since it is not a critical requirement for the generator. --- .../src/stages/generate/techdocs.ts | 34 +++++++++++++------ .../src/stages/generate/types.ts | 4 ++- .../src/stages/prepare/index.ts | 2 +- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/techdocs-common/src/stages/generate/techdocs.ts b/packages/techdocs-common/src/stages/generate/techdocs.ts index faefd8ac16..bdecb9cbf3 100644 --- a/packages/techdocs-common/src/stages/generate/techdocs.ts +++ b/packages/techdocs-common/src/stages/generate/techdocs.ts @@ -66,24 +66,36 @@ export class TechdocsGenerator implements GeneratorBase { public async run({ directory, dockerClient, + expectedResultDir, parsedLocationAnnotation, }: GeneratorRunOptions): Promise { - const tmpdirPath = os.tmpdir(); - // Fixes a problem with macOS returning a path that is a symlink - const tmpdirResolvedPath = fs.realpathSync(tmpdirPath); - const resultDir = fs.mkdtempSync( - path.join(tmpdirResolvedPath, 'techdocs-tmp-'), - ); + // If provided with a directory to output the generated files in, use that directory. + // Else create a new temporary directory and return path. + let resultDir; + if (expectedResultDir) { + resultDir = expectedResultDir; + await fs.ensureDir(resultDir); + } else { + const tmpdirPath = os.tmpdir(); + // Fixes a problem with macOS returning a path that is a symlink + const tmpdirResolvedPath = fs.realpathSync(tmpdirPath); + resultDir = fs.mkdtempSync( + path.join(tmpdirResolvedPath, 'techdocs-tmp-'), + ); + } + const [log, logStream] = createStream(); // TODO: In future mkdocs.yml can be mkdocs.yaml. So, use a config variable here to find out // the correct file name. // Do some updates to mkdocs.yml before generating docs e.g. adding repo_url - await patchMkdocsYmlPreBuild( - path.join(directory, 'mkdocs.yml'), - this.logger, - parsedLocationAnnotation, - ); + if (parsedLocationAnnotation) { + await patchMkdocsYmlPreBuild( + path.join(directory, 'mkdocs.yml'), + this.logger, + parsedLocationAnnotation, + ); + } try { switch (this.options.runGeneratorIn) { diff --git a/packages/techdocs-common/src/stages/generate/types.ts b/packages/techdocs-common/src/stages/generate/types.ts index 65e411c3aa..8a3e68392b 100644 --- a/packages/techdocs-common/src/stages/generate/types.ts +++ b/packages/techdocs-common/src/stages/generate/types.ts @@ -31,13 +31,15 @@ export type GeneratorRunResult = { * * @param {string} directory The directory of the uncompiled documentation, with the values from the frontend * @param {Docker} dockerClient A docker client to run any generator on top of your directory + * @param {string} expectedResultDir Optional directory to store generated docs in. Default: A newly created temporary directory. * @param {ParsedLocationAnnotation} parsedLocationAnnotation backstage.io/techdocs-ref annotation of an entity * @param {Writable} [logStream] A dedicated log stream */ export type GeneratorRunOptions = { directory: string; dockerClient: Docker; - parsedLocationAnnotation: ParsedLocationAnnotation; + expectedResultDir?: string; + parsedLocationAnnotation?: ParsedLocationAnnotation; logStream?: Writable; }; diff --git a/packages/techdocs-common/src/stages/prepare/index.ts b/packages/techdocs-common/src/stages/prepare/index.ts index 6c73725a3a..dcb178bafa 100644 --- a/packages/techdocs-common/src/stages/prepare/index.ts +++ b/packages/techdocs-common/src/stages/prepare/index.ts @@ -17,4 +17,4 @@ export { DirectoryPreparer } from './dir'; export { CommonGitPreparer } from './commonGit'; export { UrlPreparer } from './url'; export { Preparers } from './preparers'; -export type { PreparerBuilder, PreparerBase } from './types'; +export type { PreparerBuilder, PreparerBase, RemoteProtocol } from './types'; From 68ad5af519ff3bbfb5b154309f39ae79e737958c Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Fri, 8 Jan 2021 14:30:18 +0100 Subject: [PATCH 2/5] Create eleven-badgers-wink.md --- .changeset/eleven-badgers-wink.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/eleven-badgers-wink.md diff --git a/.changeset/eleven-badgers-wink.md b/.changeset/eleven-badgers-wink.md new file mode 100644 index 0000000000..1d0c1443fb --- /dev/null +++ b/.changeset/eleven-badgers-wink.md @@ -0,0 +1,6 @@ +--- +'@backstage/techdocs-common': patch +--- + +Improve techdocs-common Generator API for it to be used by techdocs-cli. No breaking changes. +Generator can now accept an output directory. From 2165901c000c9632079418338a18f582c966881a Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 11 Jan 2021 22:33:10 +0100 Subject: [PATCH 3/5] TechDocs: Generator now requires input and output directory and does not create them The creation of temporary directories and their clean up should be handled on its own. techdocs-cli already has a fixed input output dir requirement. It makes that generator accepts these values as requirements. --- .../src/stages/generate/helpers.ts | 6 +-- .../src/stages/generate/techdocs.ts | 49 +++++-------------- .../src/stages/generate/types.ts | 20 +++----- plugins/techdocs-backend/package.json | 1 + .../src/DocsBuilder/builder.ts | 20 ++++++-- 5 files changed, 40 insertions(+), 56 deletions(-) diff --git a/packages/techdocs-common/src/stages/generate/helpers.ts b/packages/techdocs-common/src/stages/generate/helpers.ts index 16f5b26632..e396157b92 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.ts @@ -39,7 +39,7 @@ type RunDockerContainerOptions = { args: string[]; logStream?: Writable; docsDir: string; - resultDir: string; + outputDir: string; dockerClient: Docker; createOptions?: Docker.ContainerCreateOptions; }; @@ -56,7 +56,7 @@ export async function runDockerContainer({ args, logStream = new PassThrough(), docsDir, - resultDir, + outputDir, dockerClient, createOptions, }: RunDockerContainerOptions) { @@ -89,7 +89,7 @@ export async function runDockerContainer({ }, WorkingDir: '/content', HostConfig: { - Binds: [`${docsDir}:/content`, `${resultDir}:/result`], + Binds: [`${docsDir}:/content`, `${outputDir}:/result`], }, ...createOptions, }, diff --git a/packages/techdocs-common/src/stages/generate/techdocs.ts b/packages/techdocs-common/src/stages/generate/techdocs.ts index bdecb9cbf3..507b9da6b8 100644 --- a/packages/techdocs-common/src/stages/generate/techdocs.ts +++ b/packages/techdocs-common/src/stages/generate/techdocs.ts @@ -14,18 +14,12 @@ * limitations under the License. */ -import fs from 'fs-extra'; import path from 'path'; -import os from 'os'; import { Logger } from 'winston'; import { PassThrough } from 'stream'; import { Config } from '@backstage/config'; -import { - GeneratorBase, - GeneratorRunOptions, - GeneratorRunResult, -} from './types'; +import { GeneratorBase, GeneratorRunOptions } from './types'; import { runDockerContainer, runCommand, @@ -64,26 +58,11 @@ export class TechdocsGenerator implements GeneratorBase { } public async run({ - directory, + inputDir, + outputDir, dockerClient, - expectedResultDir, parsedLocationAnnotation, - }: GeneratorRunOptions): Promise { - // If provided with a directory to output the generated files in, use that directory. - // Else create a new temporary directory and return path. - let resultDir; - if (expectedResultDir) { - resultDir = expectedResultDir; - await fs.ensureDir(resultDir); - } else { - const tmpdirPath = os.tmpdir(); - // Fixes a problem with macOS returning a path that is a symlink - const tmpdirResolvedPath = fs.realpathSync(tmpdirPath); - resultDir = fs.mkdtempSync( - path.join(tmpdirResolvedPath, 'techdocs-tmp-'), - ); - } - + }: GeneratorRunOptions): Promise { const [log, logStream] = createStream(); // TODO: In future mkdocs.yml can be mkdocs.yaml. So, use a config variable here to find out @@ -91,7 +70,7 @@ export class TechdocsGenerator implements GeneratorBase { // Do some updates to mkdocs.yml before generating docs e.g. adding repo_url if (parsedLocationAnnotation) { await patchMkdocsYmlPreBuild( - path.join(directory, 'mkdocs.yml'), + path.join(inputDir, 'mkdocs.yml'), this.logger, parsedLocationAnnotation, ); @@ -102,14 +81,14 @@ export class TechdocsGenerator implements GeneratorBase { case 'local': await runCommand({ command: 'mkdocs', - args: ['build', '-d', resultDir, '-v'], + args: ['build', '-d', outputDir, '-v'], options: { - cwd: directory, + cwd: inputDir, }, logStream, }); this.logger.info( - `Successfully generated docs from ${directory} into ${resultDir} using local mkdocs`, + `Successfully generated docs from ${inputDir} into ${outputDir} using local mkdocs`, ); break; case 'docker': @@ -117,12 +96,12 @@ export class TechdocsGenerator implements GeneratorBase { imageName: 'spotify/techdocs', args: ['build', '-d', '/result'], logStream, - docsDir: directory, - resultDir, + docsDir: inputDir, + outputDir, dockerClient, }); this.logger.info( - `Successfully generated docs from ${directory} into ${resultDir} using techdocs-container`, + `Successfully generated docs from ${inputDir} into ${outputDir} using techdocs-container`, ); break; default: @@ -132,14 +111,12 @@ export class TechdocsGenerator implements GeneratorBase { } } catch (error) { this.logger.debug( - `Failed to generate docs from ${directory} into ${resultDir}`, + `Failed to generate docs from ${inputDir} into ${outputDir}`, ); this.logger.debug(`Build failed with error: ${log}`); throw new Error( - `Failed to generate docs from ${directory} into ${resultDir} with error ${error.message}`, + `Failed to generate docs from ${inputDir} into ${outputDir} with error ${error.message}`, ); } - - return { resultDir }; } } diff --git a/packages/techdocs-common/src/stages/generate/types.ts b/packages/techdocs-common/src/stages/generate/types.ts index 8a3e68392b..7724107dac 100644 --- a/packages/techdocs-common/src/stages/generate/types.ts +++ b/packages/techdocs-common/src/stages/generate/types.ts @@ -18,34 +18,26 @@ import Docker from 'dockerode'; import { Entity } from '@backstage/catalog-model'; import { ParsedLocationAnnotation } from '../../helpers'; -/** - * The returned directory from the generator which is ready - * to pass to the next stage of the TechDocs which is publishing - */ -export type GeneratorRunResult = { - resultDir: string; -}; - /** * The values that the generator will receive. * - * @param {string} directory The directory of the uncompiled documentation, with the values from the frontend + * @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 {string} expectedResultDir Optional directory to store generated docs in. Default: A newly created temporary directory. * @param {ParsedLocationAnnotation} parsedLocationAnnotation backstage.io/techdocs-ref annotation of an entity * @param {Writable} [logStream] A dedicated log stream */ export type GeneratorRunOptions = { - directory: string; + inputDir: string; + outputDir: string; dockerClient: Docker; - expectedResultDir?: string; parsedLocationAnnotation?: ParsedLocationAnnotation; logStream?: Writable; }; export type GeneratorBase = { - // runs the generator with the values and returns the directory to be published - run(opts: GeneratorRunOptions): Promise; + // Runs the generator with the values + run(opts: GeneratorRunOptions): Promise; }; /** diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 12f1646517..6b0aeec177 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -40,6 +40,7 @@ "dockerode": "^3.2.1", "express": "^4.17.1", "express-promise-router": "^3.0.3", + "fs-extra": "^9.0.1", "knex": "^0.21.6", "winston": "^3.2.1" }, diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 863f587a96..3d40f0a870 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -13,6 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import fs from 'fs-extra'; +import os from 'os'; +import path from 'path'; import Docker from 'dockerode'; import { Logger } from 'winston'; import { Entity } from '@backstage/catalog-model'; @@ -78,8 +81,17 @@ export class DocsBuilder { const parsedLocationAnnotation = getLocationForEntity(this.entity); this.logger.info(`Running generator on entity ${getEntityId(this.entity)}`); - const { resultDir } = await this.generator.run({ - directory: preparedDir, + // Create a temporary directory to store the generated files in. + const tmpdirPath = os.tmpdir(); + // Fixes a problem with macOS returning a path that is a symlink + const tmpdirResolvedPath = fs.realpathSync(tmpdirPath); + const outputDir = await fs.mkdtemp( + path.join(tmpdirResolvedPath, 'techdocs-tmp-'), + ); + + await this.generator.run({ + inputDir: preparedDir, + outputDir, dockerClient: this.dockerClient, parsedLocationAnnotation, }); @@ -87,9 +99,11 @@ export class DocsBuilder { this.logger.info(`Running publisher on entity ${getEntityId(this.entity)}`); await this.publisher.publish({ entity: this.entity, - directory: resultDir, + directory: outputDir, }); + // TODO: Remove the generated directory once published. + if (!this.entity.metadata.uid) { throw new Error( 'Trying to build documentation for entity not in service catalog', From f7ecdbb3977d96a7849578304847e953be18223b Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 11 Jan 2021 22:39:05 +0100 Subject: [PATCH 4/5] changeset: Info about techdocs generator api change --- .changeset/eleven-badgers-wink.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.changeset/eleven-badgers-wink.md b/.changeset/eleven-badgers-wink.md index 1d0c1443fb..a08c11d257 100644 --- a/.changeset/eleven-badgers-wink.md +++ b/.changeset/eleven-badgers-wink.md @@ -1,6 +1,9 @@ --- '@backstage/techdocs-common': patch +'@backstage/plugin-techdocs-backend': patch --- -Improve techdocs-common Generator API for it to be used by techdocs-cli. No breaking changes. -Generator can now accept an output directory. +Improve techdocs-common Generator API for it to be used by techdocs-cli. TechDocs generator.run function now takes +an input AND an output directory. Most probably you use techdocs-common via plugin-techdocs-backend, and so there +is no breaking change for you. +But if you use techdocs-common separately, you need to create an output directory and pass into the generator. From 078987e2512348ef4f0e168156757f7d7b50deaf Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 11 Jan 2021 23:08:30 +0100 Subject: [PATCH 5/5] techdocs-common: fix tests for the new generator API --- .../src/stages/generate/helpers.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/techdocs-common/src/stages/generate/helpers.test.ts b/packages/techdocs-common/src/stages/generate/helpers.test.ts index 48a48986f2..4d62648a7d 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.test.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.test.ts @@ -80,14 +80,14 @@ describe('helpers', () => { const imageName = 'spotify/techdocs'; const args = ['build', '-d', '/result']; const docsDir = os.tmpdir(); - const resultDir = os.tmpdir(); + const outputDir = os.tmpdir(); it('should pull the techdocs docker container', async () => { await runDockerContainer({ imageName, args, docsDir, - resultDir, + outputDir, dockerClient: mockDocker, }); @@ -103,7 +103,7 @@ describe('helpers', () => { imageName, args, docsDir, - resultDir, + outputDir, dockerClient: mockDocker, }); @@ -118,7 +118,7 @@ describe('helpers', () => { }, WorkingDir: '/content', HostConfig: { - Binds: [`${docsDir}:/content`, `${resultDir}:/result`], + Binds: [`${docsDir}:/content`, `${outputDir}:/result`], }, }, ); @@ -129,7 +129,7 @@ describe('helpers', () => { imageName, args, docsDir, - resultDir, + outputDir, dockerClient: mockDocker, }); @@ -151,7 +151,7 @@ describe('helpers', () => { imageName, args, docsDir, - resultDir, + outputDir, dockerClient: mockDocker, }), ).rejects.toThrow(new RegExp(`.+: ${dockerError}`));