From 266e93b47831b2c058612d4ee643b1a0df4e8030 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 24 Nov 2020 23:29:18 +0100 Subject: [PATCH 1/3] TechDocs: Refactor metadata retrieval 1. Don't use mkdocs in name of APIs or variables. It is an implementation detail and liable to change in future. 2. techDocsApi.getMetadata('mkdocs') and techDocsAPI.getMetadata('entity') should be two separate functions just because their responses differ in structure. They should also be type checked. 3. Use either 'techdocs' or 'TechDocs' consistently. 'techDocs' seems like an unnecessary third way to write TechDocs, which can be avoided. 4. Remove unused /plugins/techdocs-backend/src/service/metadata.ts file --- .../techdocs-backend/src/service/metadata.ts | 41 ------------------- .../techdocs-backend/src/service/router.ts | 6 +-- plugins/techdocs/src/api.ts | 36 ++++++++++++++-- .../reader/components/TechDocsPage.test.tsx | 11 ++--- .../src/reader/components/TechDocsPage.tsx | 14 +++---- .../components/TechDocsPageHeader.test.tsx | 4 +- .../reader/components/TechDocsPageHeader.tsx | 11 +++-- 7 files changed, 57 insertions(+), 66 deletions(-) delete mode 100644 plugins/techdocs-backend/src/service/metadata.ts diff --git a/plugins/techdocs-backend/src/service/metadata.ts b/plugins/techdocs-backend/src/service/metadata.ts deleted file mode 100644 index 760180f8d2..0000000000 --- a/plugins/techdocs-backend/src/service/metadata.ts +++ /dev/null @@ -1,41 +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 fetch from 'cross-fetch'; - -export class TechDocsMetadata { - private async getMetadataFile(docsUrl: String) { - const metadataURL = `${docsUrl}/techdocs_metadata.json`; - - try { - const req = await fetch(metadataURL); - - return await req.json(); - } catch (error) { - throw new Error(error); - } - } - - public async getMkDocsMetaData(docsUrl: any) { - const mkDocsMetadata = await this.getMetadataFile(docsUrl); - - if (!mkDocsMetadata) return null; - - return { - ...mkDocsMetadata, - }; - } -} diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 1d932a1fb9..625252dd7e 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -61,7 +61,7 @@ export async function createRouter({ }: RouterOptions): Promise { const router = Router(); - router.get('/metadata/mkdocs/*', async (req, res) => { + router.get('/metadata/techdocs/*', async (req, res) => { let storageUrl = config.getString('techdocs.storageUrl'); if (publisher instanceof LocalPublish) { storageUrl = new URL( @@ -74,8 +74,8 @@ export async function createRouter({ const metadataURL = `${storageUrl}/${path}/techdocs_metadata.json`; try { - const mkDocsMetadata = await (await fetch(metadataURL)).json(); - res.send(mkDocsMetadata); + const techdocsMetadata = await (await fetch(metadataURL)).json(); + res.send(techdocsMetadata); } catch (err) { logger.info(`Unable to get metadata for ${path} with error ${err}`); throw new Error(`Unable to get metadata for ${path} with error ${err}`); diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index 7281ebdf13..98e1e6c310 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -15,7 +15,6 @@ */ import { createApiRef } from '@backstage/core'; - import { ParsedEntityId } from './types'; export const techdocsStorageApiRef = createApiRef({ @@ -38,7 +37,8 @@ export interface TechDocsStorage { } export interface TechDocs { - getMetadata(metadataType: string, entityId: ParsedEntityId): Promise; + getTechDocsMetadata(entityId: ParsedEntityId): Promise; + getEntityMetadata(entityId: ParsedEntityId): Promise; } /** @@ -53,10 +53,38 @@ export class TechDocsApi implements TechDocs { this.apiOrigin = apiOrigin; } - async getMetadata(metadataType: string, entityId: ParsedEntityId) { + /** + * Retrieve TechDocs metadata. + * + * When docs are built, we generate a techdocs_metadata.json and store it along with the generated + * static files. It includes necessary data about the docs site. This method requests techdocs-backend + * which retries the TechDocs metadata. + * + * @param {ParsedEntityId} entityId Object containing entity data like name, namespace, etc. + */ + async getTechDocsMetadata(entityId: ParsedEntityId) { const { kind, namespace, name } = entityId; - const requestUrl = `${this.apiOrigin}/metadata/${metadataType}/${namespace}/${kind}/${name}`; + const requestUrl = `${this.apiOrigin}/metadata/techdocs/${namespace}/${kind}/${name}`; + + const request = await fetch(`${requestUrl}`); + const res = await request.json(); + + return res; + } + + /** + * Retrieve metadata about an entity. + * + * This method requests techdocs-backend which uses the catalog APIs to respond with filtered + * information required here. + * + * @param {ParsedEntityId} entityId Object containing entity data like name, namespace, etc. + */ + async getEntityMetadata(entityId: ParsedEntityId) { + const { kind, namespace, name } = entityId; + + const requestUrl = `${this.apiOrigin}/metadata/entity/${namespace}/${kind}/${name}`; const request = await fetch(`${requestUrl}`); const res = await request.json(); diff --git a/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx b/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx index 464115bab7..c05c0488ff 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx @@ -50,17 +50,18 @@ describe('', () => { entityId: 'Component::backstage', }); - const techDocsApi: Partial = { - getMetadata: () => Promise.resolve([]), + const techdocsApi: Partial = { + getEntityMetadata: () => Promise.resolve([]), + getTechDocsMetadata: () => Promise.resolve([]), }; - const techDocsStorageApi: Partial = { + const techdocsStorageApi: Partial = { getEntityDocs: (): Promise => Promise.resolve('String'), getBaseUrl: (): string => '', }; const apiRegistry = ApiRegistry.from([ - [techdocsApiRef, techDocsApi], - [techdocsStorageApiRef, techDocsStorageApi], + [techdocsApiRef, techdocsApi], + [techdocsStorageApiRef, techdocsStorageApi], ]); await act(async () => { diff --git a/plugins/techdocs/src/reader/components/TechDocsPage.tsx b/plugins/techdocs/src/reader/components/TechDocsPage.tsx index 00f0b3c9d3..f2635c0dfb 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPage.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPage.tsx @@ -26,19 +26,19 @@ export const TechDocsPage = () => { const [documentReady, setDocumentReady] = useState(false); const { namespace, kind, name } = useParams(); - const techDocsApi = useApi(techdocsApiRef); + const techdocsApi = useApi(techdocsApiRef); - const mkdocsMetadataRequest = useAsync(() => { + const techdocsMetadataRequest = useAsync(() => { if (documentReady) { - return techDocsApi.getMetadata('mkdocs', { kind, namespace, name }); + return techdocsApi.getTechDocsMetadata({ kind, namespace, name }); } return Promise.resolve({ loading: true }); - }, [kind, namespace, name, techDocsApi, documentReady]); + }, [kind, namespace, name, techdocsApi, documentReady]); const entityMetadataRequest = useAsync(() => { - return techDocsApi.getMetadata('entity', { kind, namespace, name }); - }, [kind, namespace, name, techDocsApi]); + return techdocsApi.getEntityMetadata({ kind, namespace, name }); + }, [kind, namespace, name, techdocsApi]); const onReady = () => { setDocumentReady(true); @@ -48,7 +48,7 @@ export const TechDocsPage = () => { ', () => { }, }, }, - mkdocs: { + techdocs: { loading: false, value: { site_name: 'test-site-name', @@ -73,7 +73,7 @@ describe('', () => { entity: { loading: false, }, - mkdocs: { + techdocs: { loading: false, }, }} diff --git a/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx b/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx index f4c447bae2..475b87da6f 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx @@ -25,7 +25,7 @@ type TechDocsPageHeaderProps = { entityId: ParsedEntityId; metadataRequest: { entity: AsyncState; - mkdocs: AsyncState; + techdocs: AsyncState; }; }; @@ -33,15 +33,18 @@ export const TechDocsPageHeader = ({ entityId, metadataRequest, }: TechDocsPageHeaderProps) => { - const { mkdocs: mkdocsMetadata, entity: entityMetadata } = metadataRequest; + const { + techdocs: techdocsMetadata, + entity: entityMetadata, + } = metadataRequest; - const { value: mkDocsMetadataValues } = mkdocsMetadata; + const { value: techdocsMetadataValues } = techdocsMetadata; const { value: entityMetadataValues } = entityMetadata; const { kind, name } = entityId; const { site_name: siteName, site_description: siteDescription } = - mkDocsMetadataValues || {}; + techdocsMetadataValues || {}; const { locationMetadata, From 4b53294a6cb75b145f90cd53e97d13498f05c125 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 26 Nov 2020 00:29:46 +0100 Subject: [PATCH 2/3] TechDocs: Update mkdocs.yml with repo_url when possible before building docs --- .changeset/eleven-lobsters-train.md | 7 + plugins/techdocs-backend/package.json | 2 + .../techdocs-backend/src/service/helpers.ts | 3 + .../stages/generate/__fixtures__/mkdocs.yml | 2 + .../__fixtures__/mkdocs_with_repo_url.yml | 4 + .../techdocs/stages/generate/helpers.test.ts | 197 +++++++++++++++++- .../src/techdocs/stages/generate/helpers.ts | 137 +++++++++++- .../src/techdocs/stages/generate/techdocs.ts | 16 +- .../src/techdocs/stages/generate/types.ts | 15 +- 9 files changed, 373 insertions(+), 10 deletions(-) create mode 100644 .changeset/eleven-lobsters-train.md create mode 100644 plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs.yml create mode 100644 plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs_with_repo_url.yml diff --git a/.changeset/eleven-lobsters-train.md b/.changeset/eleven-lobsters-train.md new file mode 100644 index 0000000000..95b9145ee6 --- /dev/null +++ b/.changeset/eleven-lobsters-train.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-techdocs': minor +'@backstage/plugin-techdocs-backend': minor +--- + +- Use techdocs annotation to add repo_url if missing in mkdocs.yml. Having repo_url creates a Edit button on techdocs pages. +- techdocs-backend: API endpoint `/metadata/mkdocs/*` renamed to `/metadata/techdocs/*` diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 1d9f3ea7c1..a09adb7380 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -32,7 +32,9 @@ "express-promise-router": "^3.0.3", "fs-extra": "^9.0.1", "git-url-parse": "^11.4.0", + "js-yaml": "^3.14.0", "knex": "^0.21.6", + "mock-fs": "^4.13.0", "nodegit": "^0.27.0", "winston": "^3.2.1" }, diff --git a/plugins/techdocs-backend/src/service/helpers.ts b/plugins/techdocs-backend/src/service/helpers.ts index 3017a68461..4a736f7388 100644 --- a/plugins/techdocs-backend/src/service/helpers.ts +++ b/plugins/techdocs-backend/src/service/helpers.ts @@ -69,10 +69,13 @@ export class DocsBuilder { this.logger.info(`Running preparer on entity ${getEntityId(this.entity)}`); const preparedDir = await this.preparer.prepare(this.entity); + const parsedLocationAnnotation = getLocationForEntity(this.entity); + this.logger.info(`Running generator on entity ${getEntityId(this.entity)}`); const { resultDir } = await this.generator.run({ directory: preparedDir, dockerClient: this.dockerClient, + parsedLocationAnnotation, }); this.logger.info(`Running publisher on entity ${getEntityId(this.entity)}`); diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs.yml b/plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs.yml new file mode 100644 index 0000000000..1d1186840f --- /dev/null +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs.yml @@ -0,0 +1,2 @@ +site_name: Test site name +site_description: Test site description diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs_with_repo_url.yml b/plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs_with_repo_url.yml new file mode 100644 index 0000000000..37b004bfe4 --- /dev/null +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/__fixtures__/mkdocs_with_repo_url.yml @@ -0,0 +1,4 @@ +site_name: Test site name +site_description: Test site description + +repo_url: https://github.com/backstage/backstage diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts index 283560f0ce..4938739fdf 100644 --- a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts @@ -13,10 +13,22 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import Stream, { PassThrough } from 'stream'; +import fs from 'fs'; import os from 'os'; +import { resolve as resolvePath } from 'path'; +import Stream, { PassThrough } from 'stream'; import Docker from 'dockerode'; -import { runDockerContainer, getGeneratorKey } from './helpers'; +import mockFs from 'mock-fs'; +import * as winston from 'winston'; +import { + runDockerContainer, + getGeneratorKey, + isValidRepoUrlForMkdocs, + getRepoUrlFromLocationAnnotation, + patchMkdocsYmlPreBuild, +} from './helpers'; +import { RemoteProtocol } from '../prepare/types'; +import { ParsedLocationAnnotation } from '../../../helpers'; const mockEntity = { apiVersion: 'version', @@ -28,6 +40,14 @@ const mockEntity = { const mockDocker = new Docker() as jest.Mocked; +const mkdocsYml = fs.readFileSync( + resolvePath(__filename, '../__fixtures__/mkdocs.yml'), +); +const mkdocsYmlWithRepoUrl = fs.readFileSync( + resolvePath(__filename, '../__fixtures__/mkdocs_with_repo_url.yml'), +); +const mockLogger = winston.createLogger(); + describe('helpers', () => { describe('getGeneratorKey', () => { it('should return techdocs as the only generator key', () => { @@ -138,4 +158,177 @@ describe('helpers', () => { }); }); }); + + describe('isValidRepoUrlForMkdocs', () => { + it('should return true for valid repo_url values for mkdocs', () => { + const validRepoUrls = [ + 'https://github.com/org/repo', + 'https://github.com/backstage/backstage/', + 'https://github.com/org123/repo1-2-3/', + 'http://github.com/insecureOrg/insecureRepo', + 'https://gitlab.com/org/repo', + 'https://gitlab.com/backstage/backstage/', + 'https://gitlab.com/org123/repo1-2-3/', + 'http://gitlab.com/insecureOrg/insecureRepo', + ]; + + const validRemoteProtocols = ['github', 'gitlab']; + + validRepoUrls.forEach(url => { + validRemoteProtocols.forEach(targetType => { + expect( + isValidRepoUrlForMkdocs(url, targetType as RemoteProtocol), + ).toBe(true); + }); + }); + }); + + it('should return false for invalid repo_urls values for mkdocs', () => { + const invalidRepoUrls = [ + 'git@github.com:org/repo', + 'https://github.com/backstage/backstage/tree/master/plugins/techdocs-backend', + ]; + + invalidRepoUrls.forEach(url => { + expect(isValidRepoUrlForMkdocs(url, 'github')).toBe(false); + }); + }); + + it('should return false for unsupported remote protocols', () => { + const validRepoUrl = 'https://github.com/backstage/backstage'; + + const unsupportedRemoteProtocols = ['dir', 'file', 'url']; + + unsupportedRemoteProtocols.forEach(targetType => { + expect( + isValidRepoUrlForMkdocs(validRepoUrl, targetType as RemoteProtocol), + ).toBe(false); + }); + }); + }); + + describe('getRepoUrlFromLocationAnnotation', () => { + it('should return undefined for unsupported location type', () => { + const parsedLocationAnnotation1: ParsedLocationAnnotation = { + type: 'dir', + target: '/home/user/workspace/docs-repository', + }; + + const parsedLocationAnnotation2: ParsedLocationAnnotation = { + type: 'file', + target: '/home/user/workspace/docs-repository/catalog-info.yaml', + }; + + const parsedLocationAnnotation3: ParsedLocationAnnotation = { + type: 'url', + target: 'https://my-website.com/storage/this/docs/repository', + }; + + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation1)).toBe( + undefined, + ); + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation2)).toBe( + undefined, + ); + + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation3)).toBe( + undefined, + ); + }); + + it('should return correct target url for supported hosts', () => { + const parsedLocationAnnotation1: ParsedLocationAnnotation = { + type: 'github', + target: 'https://github.com/backstage/backstage.git', + }; + + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation1)).toBe( + 'https://github.com/backstage/backstage', + ); + + const parsedLocationAnnotation2: ParsedLocationAnnotation = { + type: 'github', + target: 'https://github.com/org/repo', + }; + + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation2)).toBe( + 'https://github.com/org/repo', + ); + + const parsedLocationAnnotation3: ParsedLocationAnnotation = { + type: 'gitlab', + target: 'https://gitlab.com/org/repo', + }; + + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation3)).toBe( + 'https://gitlab.com/org/repo', + ); + + const parsedLocationAnnotation4: ParsedLocationAnnotation = { + type: 'github', + target: + 'github.com/backstage/backstage/blob/master/plugins/techdocs-backend/examples/documented-component', + }; + + expect(getRepoUrlFromLocationAnnotation(parsedLocationAnnotation4)).toBe( + 'github.com/backstage/backstage/blob/master/plugins/techdocs-backend/examples/documented-component', + ); + }); + }); + + describe('pathMkdocsPreBuild', () => { + beforeEach(() => { + mockFs({ + '/mkdocs.yml': mkdocsYml, + '/mkdocs_with_repo_url.yml': mkdocsYmlWithRepoUrl, + }); + }); + + afterEach(() => { + mockFs.restore(); + }); + + it('should add repo_url to mkdocs.yml', () => { + const parsedLocationAnnotation: ParsedLocationAnnotation = { + type: 'github', + target: 'https://github.com/backstage/backstage', + }; + + patchMkdocsYmlPreBuild( + '/mkdocs.yml', + mockLogger, + parsedLocationAnnotation, + ); + + const updatedMkdocsYml = fs.readFileSync('/mkdocs.yml').toString(); + + expect(updatedMkdocsYml).toContain( + "repo_url: 'https://github.com/backstage/backstage'", + ); + }); + + it('should not override existing repo_url in mkdocs.yml', () => { + const parsedLocationAnnotation: ParsedLocationAnnotation = { + type: 'github', + target: 'https://github.com/neworg/newrepo', + }; + + patchMkdocsYmlPreBuild( + '/mkdocs_with_repo_url.yml', + mockLogger, + parsedLocationAnnotation, + ); + + const updatedMkdocsYml = fs + .readFileSync('/mkdocs_with_repo_url.yml') + .toString(); + + expect(updatedMkdocsYml).toContain( + "repo_url: 'https://github.com/backstage/backstage'", + ); + expect(updatedMkdocsYml).not.toContain( + "repo_url: 'https://github.com/neworg/newrepo'", + ); + }); + }); }); diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts index d0ffa72f1c..254186b484 100644 --- a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts @@ -14,11 +14,16 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import fs from 'fs'; +import { spawn } from 'child_process'; import { Writable, PassThrough } from 'stream'; import Docker from 'dockerode'; +import yaml from 'js-yaml'; +import { Logger } from 'winston'; +import { Entity } from '@backstage/catalog-model'; import { SupportedGeneratorKey } from './types'; -import { spawn } from 'child_process'; +import { ParsedLocationAnnotation } from '../../../helpers'; +import { RemoteProtocol } from '../prepare/types'; // TODO: Implement proper support for more generators. export function getGeneratorKey(entity: Entity): SupportedGeneratorKey { @@ -142,3 +147,131 @@ export const runCommand = async ({ }); }); }; + +/** + * Return true if mkdocs can compile docs with provided repo_url + * + * Valid repo_url examples in mkdocs.yml + * - https://github.com/backstage/backstage + * - https://gitlab.com/org/repo/ + * - http://github.com/backstage/backstage + * - A http(s) protocol URL to the root of the repository + * + * Invalid repo_url examples in mkdocs.yml + * - https://github.com/backstage/backstage/blob/master/plugins/techdocs-backend/examples/documented-component + * - (anything that is not valid as described above) + * + * @param {string} repoUrl URL supposed to be used as repo_url in mkdocs.yml + * @param {RemoteProtocol} locationType Type of source code host - github, gitlab, dir, url, etc. + * @returns {boolean} + */ +export const isValidRepoUrlForMkdocs = ( + repoUrl: string, + locationType: RemoteProtocol, +): boolean => { + // Trim trailing slash + const cleanRepoUrl = repoUrl.replace(/\/$/, ''); + + if (locationType === 'github' || locationType === 'gitlab') { + // A valid repoUrl to the root of the repository will be split into 5 strings if split using the / delimiter. + // We do not want URLs which have more than that number of forward slashes since they will signify a non-root location + // Note: This is not the best possible implementation but will work most of the times.. Feel free to improve or + // highlight edge cases. + return cleanRepoUrl.split('/').length === 5; + } + + return false; +}; + +/** + * Return a valid URL of the repository used in backstage.io/techdocs-ref annotation. + * Return undefined if the `target` is not valid in context of repo_url in mkdocs.yml + * Alter URL so that it is a valid repo_url config in mkdocs.yml + * + * @param {ParsedLocationAnnotation} parsedLocationAnnotation Object with location url and type + * @returns {string | undefined} + */ +export const getRepoUrlFromLocationAnnotation = ( + parsedLocationAnnotation: ParsedLocationAnnotation, +): string | undefined => { + const { type: locationType, target } = parsedLocationAnnotation; + + // Add more options from the RemoteProtocol type of parsedLocationAnnotation.type here + // when TechDocs supports more hosts and if mkdocs can generated an Edit URL for them. + const supportedHosts = ['github', 'gitlab']; + + if (supportedHosts.includes(locationType)) { + // Trim .git or .git/ from the end of repository url + return target.replace(/.git\/*$/, ''); + } + + return undefined; +}; + +/** + * Update the mkdocs.yml file before TechDocs generator uses it to build docs site. + * + * List of tasks: + * - Add repo_url if it does not exists + * If mkdocs.yml has a repo_url, the generated docs site gets an Edit button on the pages by default. + * If repo_url is missing in mkdocs.yml, we will use techdocs annotation of the entity to possibly get + * the repository URL. + * + * This function will not throw an error since this is not critical to the whole TechDocs pipeline. + * Instead it will log warnings if there are any errors in reading, parsing or writing YAML. + * + * @param {string} mkdocsYmlPath Absolute path to mkdocs.yml or equivalent of a docs site + * @param {Logger} logger + * @param {ParsedLocationAnnotation} parsedLocationAnnotation Object with location url and type + */ +export const patchMkdocsYmlPreBuild = async ( + mkdocsYmlPath: string, + logger: Logger, + parsedLocationAnnotation: ParsedLocationAnnotation, +) => { + let mkdocsYmlFileString; + try { + mkdocsYmlFileString = fs.readFileSync(mkdocsYmlPath, 'utf8'); + } catch (error) { + logger.warn( + `Could not read file ${mkdocsYmlPath} before running the generator. ${error.message}`, + ); + return; + } + + let mkdocsYml: any; + try { + mkdocsYml = yaml.safeLoad(mkdocsYmlFileString); + + // mkdocsYml should be an object type after successful parsing. + // But based on its type definition, it can also be a string or undefined, which we don't want. + if (typeof mkdocsYml === 'string' || typeof mkdocsYml === 'undefined') { + throw new Error('Bad YAML format.'); + } + } catch (error) { + logger.warn( + `Error in parsing YAML at ${mkdocsYmlPath} before running the generator. ${error.message}`, + ); + return; + } + + // Add repo_url to mkdocs.yml if it is missing. This will enable the Page edit button generated by MkDocs. + if (!('repo_url' in mkdocsYml)) { + const repoUrl = getRepoUrlFromLocationAnnotation(parsedLocationAnnotation); + if (repoUrl !== undefined) { + // mkdocs.yml will not build with invalid repo_url. So, make sure it is valid. + if (isValidRepoUrlForMkdocs(repoUrl, parsedLocationAnnotation.type)) { + mkdocsYml.repo_url = repoUrl; + } + } + } + + try { + fs.writeFileSync(mkdocsYmlPath, yaml.safeDump(mkdocsYml), 'utf8'); + } catch (error) { + logger.warn( + `Could not write to ${mkdocsYmlPath} after updating it before running the generator. ${error.message}`, + ); + return; + } +}; diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/techdocs.ts b/plugins/techdocs-backend/src/techdocs/stages/generate/techdocs.ts index 1160ff1ade..faefd8ac16 100644 --- a/plugins/techdocs-backend/src/techdocs/stages/generate/techdocs.ts +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/techdocs.ts @@ -26,7 +26,11 @@ import { GeneratorRunOptions, GeneratorRunResult, } from './types'; -import { runDockerContainer, runCommand } from './helpers'; +import { + runDockerContainer, + runCommand, + patchMkdocsYmlPreBuild, +} from './helpers'; type TechdocsGeneratorOptions = { // This option enables users to configure if they want to use TechDocs container @@ -62,6 +66,7 @@ export class TechdocsGenerator implements GeneratorBase { public async run({ directory, dockerClient, + parsedLocationAnnotation, }: GeneratorRunOptions): Promise { const tmpdirPath = os.tmpdir(); // Fixes a problem with macOS returning a path that is a symlink @@ -71,6 +76,15 @@ export class TechdocsGenerator implements GeneratorBase { ); 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, + ); + try { switch (this.options.runGeneratorIn) { case 'local': diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/types.ts b/plugins/techdocs-backend/src/techdocs/stages/generate/types.ts index 398e4dcdd0..6d9ce5afca 100644 --- a/plugins/techdocs-backend/src/techdocs/stages/generate/types.ts +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/types.ts @@ -13,9 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import type { Writable } from 'stream'; +import { Writable } from 'stream'; import Docker from 'dockerode'; import { Entity } from '@backstage/catalog-model'; +import { ParsedLocationAnnotation } from '../../../helpers'; /** * The returned directory from the generator which is ready @@ -26,14 +27,18 @@ export type GeneratorRunResult = { }; /** - * The values that the generator will receive. The directory of the - * uncompiled documentation, with the values from the frontend. A dedicated log stream and a docker - * client to run any generator on top of your directory. + * The values that the generator will receive. + * + * @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 {ParsedLocationAnnotation} parsedLocationAnnotation backstage.io/techdocs-ref annotation of an entity + * @param {Writable} [logStream] A dedicated log stream */ export type GeneratorRunOptions = { directory: string; - logStream?: Writable; dockerClient: Docker; + parsedLocationAnnotation: ParsedLocationAnnotation; + logStream?: Writable; }; export type GeneratorBase = { From dbca620ff9ea28456df27439c9ee77b0f402a63b Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 26 Nov 2020 14:56:23 +0100 Subject: [PATCH 3/3] Use fs-extra and async file read method --- .../techdocs/stages/generate/helpers.test.ts | 22 +++++++++---------- .../src/techdocs/stages/generate/helpers.ts | 6 ++--- plugins/techdocs/src/api.ts | 2 +- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts index 4938739fdf..54ef0b1e83 100644 --- a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import fs from 'fs'; +import fs from 'fs-extra'; import os from 'os'; import { resolve as resolvePath } from 'path'; import Stream, { PassThrough } from 'stream'; @@ -288,45 +288,43 @@ describe('helpers', () => { mockFs.restore(); }); - it('should add repo_url to mkdocs.yml', () => { + it('should add repo_url to mkdocs.yml', async () => { const parsedLocationAnnotation: ParsedLocationAnnotation = { type: 'github', target: 'https://github.com/backstage/backstage', }; - patchMkdocsYmlPreBuild( + await patchMkdocsYmlPreBuild( '/mkdocs.yml', mockLogger, parsedLocationAnnotation, ); - const updatedMkdocsYml = fs.readFileSync('/mkdocs.yml').toString(); + const updatedMkdocsYml = await fs.readFile('/mkdocs.yml'); - expect(updatedMkdocsYml).toContain( + expect(updatedMkdocsYml.toString()).toContain( "repo_url: 'https://github.com/backstage/backstage'", ); }); - it('should not override existing repo_url in mkdocs.yml', () => { + it('should not override existing repo_url in mkdocs.yml', async () => { const parsedLocationAnnotation: ParsedLocationAnnotation = { type: 'github', target: 'https://github.com/neworg/newrepo', }; - patchMkdocsYmlPreBuild( + await patchMkdocsYmlPreBuild( '/mkdocs_with_repo_url.yml', mockLogger, parsedLocationAnnotation, ); - const updatedMkdocsYml = fs - .readFileSync('/mkdocs_with_repo_url.yml') - .toString(); + const updatedMkdocsYml = await fs.readFile('/mkdocs_with_repo_url.yml'); - expect(updatedMkdocsYml).toContain( + expect(updatedMkdocsYml.toString()).toContain( "repo_url: 'https://github.com/backstage/backstage'", ); - expect(updatedMkdocsYml).not.toContain( + expect(updatedMkdocsYml.toString()).not.toContain( "repo_url: 'https://github.com/neworg/newrepo'", ); }); diff --git a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts index 254186b484..0f60712f5e 100644 --- a/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts +++ b/plugins/techdocs-backend/src/techdocs/stages/generate/helpers.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import fs from 'fs'; +import fs from 'fs-extra'; import { spawn } from 'child_process'; import { Writable, PassThrough } from 'stream'; import Docker from 'dockerode'; @@ -231,7 +231,7 @@ export const patchMkdocsYmlPreBuild = async ( ) => { let mkdocsYmlFileString; try { - mkdocsYmlFileString = fs.readFileSync(mkdocsYmlPath, 'utf8'); + mkdocsYmlFileString = await fs.readFile(mkdocsYmlPath, 'utf8'); } catch (error) { logger.warn( `Could not read file ${mkdocsYmlPath} before running the generator. ${error.message}`, @@ -267,7 +267,7 @@ export const patchMkdocsYmlPreBuild = async ( } try { - fs.writeFileSync(mkdocsYmlPath, yaml.safeDump(mkdocsYml), 'utf8'); + await fs.writeFile(mkdocsYmlPath, yaml.safeDump(mkdocsYml), 'utf8'); } catch (error) { logger.warn( `Could not write to ${mkdocsYmlPath} after updating it before running the generator. ${error.message}`, diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index 98e1e6c310..368705fa22 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -58,7 +58,7 @@ export class TechDocsApi implements TechDocs { * * When docs are built, we generate a techdocs_metadata.json and store it along with the generated * static files. It includes necessary data about the docs site. This method requests techdocs-backend - * which retries the TechDocs metadata. + * which retrieves the TechDocs metadata. * * @param {ParsedEntityId} entityId Object containing entity data like name, namespace, etc. */