From 2a865343c2bcaa6c70fe123b0eff32debde42a0d Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Wed, 16 Feb 2022 16:42:33 +1100 Subject: [PATCH 01/12] Add a new interface: DocsBuildStrategy This adds a new interface called DocsBuildStrategy. This strategy allows for different answers to the question: should the TechDocs backend perform a build of the given entity? The default implementation replicates the existing functionality, in that if the techdocs.builder config value is set to local, then we do trigger builds, and if the value is set to anything else, we don't trigger builds. However the strategy has access to the entity, and so more complex strategies are possible. This strategy is added as an optional parameter on the RouterOptions types, and defaults to the DefaultDocsBuildStrategy if unspecified, allowing for backwards compatibility. TODO: rename some of the config options, and reword errors to reflect the new interface Signed-off-by: Nikolas Skoufis --- .../src/service/DocsBuildStrategy.test.ts | 63 +++++++++++++++++++ .../src/service/DocsBuildStrategy.ts | 33 ++++++++++ .../src/service/router.test.ts | 30 +++++---- .../techdocs-backend/src/service/router.ts | 11 +++- 4 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts create mode 100644 plugins/techdocs-backend/src/service/DocsBuildStrategy.ts diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts new file mode 100644 index 0000000000..71e5377a03 --- /dev/null +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts @@ -0,0 +1,63 @@ +/* + * Copyright 2021 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 { DefaultDocsBuildStrategy } from './DocsBuildStrategy'; +import { ConfigReader } from '@backstage/config'; + +const MockedConfigReader = ConfigReader as jest.MockedClass< + typeof ConfigReader +>; + +jest.mock('@backstage/config'); + +describe('DefaultDocsBuildStrategy', () => { + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + uid: '0', + name: 'test', + }, + }; + + const config = new ConfigReader({}); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('shouldBuild', () => { + it('should return true when techdocs.build is set to local', async () => { + const defaultDocsBuildStrategy = new DefaultDocsBuildStrategy(config); + + MockedConfigReader.prototype.getString.mockReturnValue('local'); + + const result = await defaultDocsBuildStrategy.shouldBuild(entity); + + expect(result).toBe(true); + }); + + it('should return false when techdocs.build is set to external', async () => { + const defaultDocsBuildStrategy = new DefaultDocsBuildStrategy(config); + + MockedConfigReader.prototype.getString.mockReturnValue('external'); + + const result = await defaultDocsBuildStrategy.shouldBuild(entity); + + expect(result).toBe(false); + }); + }); +}); diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts new file mode 100644 index 0000000000..69b0474a85 --- /dev/null +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts @@ -0,0 +1,33 @@ +/* + * Copyright 2022 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 { Entity } from '@backstage/catalog-model'; +import { Config } from '@backstage/config'; + +export interface DocsBuildStrategy { + shouldBuild(entity: Entity): Promise; +} + +export class DefaultDocsBuildStrategy { + private readonly config: Config; + + constructor(config: Config) { + this.config = config; + } + + async shouldBuild(_: Entity): Promise { + return this.config.getString('techdocs.builder') === 'local'; + } +} diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index 43e9641efb..521024252c 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -38,6 +38,7 @@ import { RouterOptions, } from './router'; import { TechDocsCache } from '../cache'; +import { DocsBuildStrategy } from './DocsBuildStrategy'; jest.mock('@backstage/catalog-client'); jest.mock('@backstage/config'); @@ -120,6 +121,9 @@ describe('createRouter', () => { const cache: jest.Mocked = { getClient: jest.fn(), }; + const docsBuildStrategy: jest.Mocked = { + shouldBuild: jest.fn(), + }; const outOfTheBoxOptions = { preparers, generators, @@ -128,6 +132,7 @@ describe('createRouter', () => { logger: getVoidLogger(), discovery, cache, + docsBuildStrategy, }; const recommendedOptions = { publisher, @@ -135,6 +140,7 @@ describe('createRouter', () => { logger: getVoidLogger(), discovery, cache, + docsBuildStrategy, }; beforeEach(() => { @@ -181,10 +187,10 @@ describe('createRouter', () => { expect(response.status).toBe(404); }); - it('should not check for an update without local builder', async () => { + it('should not check for an update when shouldBuild returns false', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('external'); + docsBuildStrategy.shouldBuild.mockResolvedValue(false); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doCacheSync.mockImplementation( async ({ responseHandler }) => @@ -198,10 +204,10 @@ describe('createRouter', () => { expect(response.status).toBe(304); }); - it('should error if missing builder', async () => { + it('should error if build is required and is missing preparer', async () => { const app = await createApp(recommendedOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) @@ -219,7 +225,7 @@ describe('createRouter', () => { it('should execute synchronization', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => @@ -244,7 +250,7 @@ describe('createRouter', () => { it('should return on updated', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => { @@ -297,10 +303,10 @@ describe('createRouter', () => { expect(response.status).toBe(404); }); - it('should not check for an update without local builder', async () => { + it('should not check for an update when shouldBuild returns false', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('external'); + docsBuildStrategy.shouldBuild.mockResolvedValue(false); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doCacheSync.mockImplementation( async ({ responseHandler }) => @@ -322,10 +328,10 @@ data: {"updated":false} ); }); - it('should error if missing builder', async () => { + it('should error if build is required and is missing preparer', async () => { const app = await createApp(recommendedOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) @@ -348,7 +354,7 @@ data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'prep it('should execute synchronization', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => @@ -376,7 +382,7 @@ data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'prep it('should return an event-stream', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => { diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 141ce93f79..96916bdc9b 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -35,6 +35,10 @@ import { ScmIntegrations } from '@backstage/integration'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; import { createCacheMiddleware, TechDocsCache } from '../cache'; import { CachedEntityLoader } from './CachedEntityLoader'; +import { + DefaultDocsBuildStrategy, + DocsBuildStrategy, +} from './DocsBuildStrategy'; /** * All of the required dependencies for running TechDocs in the "out-of-the-box" @@ -49,6 +53,7 @@ export type OutOfTheBoxDeploymentOptions = { database?: Knex; // TODO: Make database required when we're implementing database stuff. config: Config; cache: PluginCacheManager; + docsBuildStrategy?: DocsBuildStrategy; }; /** @@ -61,6 +66,7 @@ export type RecommendedDeploymentOptions = { discovery: PluginEndpointDiscovery; config: Config; cache: PluginCacheManager; + docsBuildStrategy?: DocsBuildStrategy; }; /** @@ -86,6 +92,8 @@ export async function createRouter( const router = Router(); const { publisher, config, logger, discovery } = options; const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const docsBuildStrategy = + options.docsBuildStrategy ?? new DefaultDocsBuildStrategy(config); // Entities are cached to optimize the /static/docs request path, which can be called many times // when loading a single techdocs page. @@ -200,7 +208,8 @@ export async function createRouter( // techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to 'local' // If set to 'external', it will assume that an external process (e.g. CI/CD pipeline // of the repository) is responsible for building and publishing documentation to the storage provider - if (config.getString('techdocs.builder') !== 'local') { + const shouldBuild = await docsBuildStrategy.shouldBuild(entity); + if (!shouldBuild) { // However, if caching is enabled, take the opportunity to check and // invalidate stale cache entries. if (cache) { From 6537a601c7de369c8aa9a3601b8fe780db46aa2b Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Sun, 27 Feb 2022 18:36:35 +1100 Subject: [PATCH 02/12] Add changeset for these changes Signed-off-by: Nikolas Skoufis --- .changeset/dull-months-knock.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dull-months-knock.md diff --git a/.changeset/dull-months-knock.md b/.changeset/dull-months-knock.md new file mode 100644 index 0000000000..3b957518c2 --- /dev/null +++ b/.changeset/dull-months-knock.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-backend': patch +--- + +Added a new interface that allows for customization of when to build techdocs From f7e6a2fbc7cd1a7c24016b205a77117e77000b42 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Sun, 27 Feb 2022 18:49:59 +1100 Subject: [PATCH 03/12] Add updated api report Signed-off-by: Nikolas Skoufis --- plugins/techdocs-backend/api-report.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/techdocs-backend/api-report.md b/plugins/techdocs-backend/api-report.md index 5d106cfafb..28700b245b 100644 --- a/plugins/techdocs-backend/api-report.md +++ b/plugins/techdocs-backend/api-report.md @@ -6,6 +6,7 @@ import { CatalogApi } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { DocumentCollator } from '@backstage/search-common'; +import { Entity } from '@backstage/catalog-model'; import express from 'express'; import { GeneratorBuilder } from '@backstage/techdocs-common'; import { Knex } from 'knex'; @@ -51,6 +52,7 @@ export type OutOfTheBoxDeploymentOptions = { database?: Knex; config: Config; cache: PluginCacheManager; + docsBuildStrategy?: DocsBuildStrategy; }; // @public @@ -60,6 +62,7 @@ export type RecommendedDeploymentOptions = { discovery: PluginEndpointDiscovery; config: Config; cache: PluginCacheManager; + docsBuildStrategy?: DocsBuildStrategy; }; // @public @@ -81,4 +84,8 @@ export type TechDocsCollatorOptions = { export { TechDocsDocument }; export * from '@backstage/techdocs-common'; + +// Warnings were encountered during analysis: +// +// src/service/router.d.ts:24:5 - (ae-forgotten-export) The symbol "DocsBuildStrategy" needs to be exported by the entry point index.d.ts ``` From 0bedff8c9abecdc13730fe54b816e9cc6674306a Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Sun, 27 Feb 2022 19:11:09 +1100 Subject: [PATCH 04/12] Add docs for build strategy stuff Signed-off-by: Nikolas Skoufis --- docs/features/techdocs/concepts.md | 18 +++++++++ docs/features/techdocs/configuration.md | 15 +++++--- docs/features/techdocs/how-to-guides.md | 50 +++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/docs/features/techdocs/concepts.md b/docs/features/techdocs/concepts.md index 408f092ebd..f8e813d89e 100644 --- a/docs/features/techdocs/concepts.md +++ b/docs/features/techdocs/concepts.md @@ -46,6 +46,24 @@ between `techdocs-backend` and the storage) [TechDocs Backend](https://github.com/backstage/backstage/tree/master/plugins/techdocs-backend) +## TechDocs Build Strategy + +To accommodate more complex logic surrounding whether or not to build TechDocs, the TechDocs backend +supports selecting a Build Strategy. +The Build Strategy is responsible for deciding whether the documentation requested should be built locally +by the TechDocs backend or not. +Customization of the Build Strategy allows for more complex behaviour regarding whether the TechDocs backend +is responsible for building TechDocs, whether an external process is responsible, or whether a combination +of local builds and an external process is responsible, on an entity-by-entity basis. + +The default Build Strategy results in the TechDocs backend building documentation locally if the +`techdocs.builder` configuration option is set to `'local'`, and skipping any building otherwise. +However any logic that satisfies the Build Strategy interface can be implemented, using the Backstage +config as well as the entity being processed to make a decision. + +For an example of how the Build Strategy can be used to implement a 'hybrid' build model, refer to +the [How to implement a hybrid build strategy](./how-to-guides#how-to-implement-a-hybrid-build-strategy) guide. + ## TechDocs Container The TechDocs container is a Docker container available at diff --git a/docs/features/techdocs/configuration.md b/docs/features/techdocs/configuration.md index 6317ae365b..9509a9a545 100644 --- a/docs/features/techdocs/configuration.md +++ b/docs/features/techdocs/configuration.md @@ -38,11 +38,16 @@ techdocs: pullImage: true # techdocs.builder can be either 'local' or 'external. - # If builder is set to 'local' and you open a TechDocs page, techdocs-backend will try to generate the docs, publish to storage - # and show the generated docs afterwords. This is the "Basic" setup of the TechDocs Architecture. - # If builder is set to 'external', techdocs-backend will only fetch the docs and will NOT try to generate and publish. In this case of 'external', - # we assume that docs are being built by an external process (e.g. in the CI/CD pipeline of the repository). This is the "Recommended" setup of - # the architecture. Read more here https://backstage.io/docs/features/techdocs/architecture + # Using the default build strategy, if builder is set to 'local' and you open a TechDocs page, + # techdocs-backend will try to generate the docs, publish to storage and show the generated docs afterwords. + # This is the "Basic" setup of the TechDocs Architecture. + # Using the default build strategy, if builder is set to 'external' (or anything other than 'local'), techdocs-backend + # will only fetch the docs and will NOT try to generate and publish. + # In this case, we assume that docs are being built by an external process (e.g. in the CI/CD pipeline of the repository). + # This is the "Recommended" setup of the architecture. + # Note that custom build strategies may alter this behaviour. + # Read more about the "Basic" and "Recommended" setups here https://backstage.io/docs/features/techdocs/architecture + # Read more about build strategies here: https://backstage.io/docs/features/techdocs/concepts#techdocs-build-strategy builder: 'local' diff --git a/docs/features/techdocs/how-to-guides.md b/docs/features/techdocs/how-to-guides.md index 863b4b3ecb..5ec8aac307 100644 --- a/docs/features/techdocs/how-to-guides.md +++ b/docs/features/techdocs/how-to-guides.md @@ -538,3 +538,53 @@ Done! Now you have a support of the following diagrams along with mermaid: - `Vega` - `Vega-Lite` - `WaveDrom` + +## How to implement a hybrid build strategy + +One limitation of the [Recommended deployment](./architecture#recommended-deployment) is that +the experience for users requires modifying their CI/CD process to publish +their TechDocs. For some users, this may be unnecessary, and provides a barrier +to entry for onboarding users to Backstage. However, a purely local TechDocs +build restricts TechDocs creators to using the tooling provided in Backstage, +as well as the plugins and features provided in the Backstage-included `mkdocs` +installation. + +To accommodate both of these use-cases, users can implement a custom [Build Strategy](./concepts#techdocs-build-strategy) +with logic to encode which TechDocs should be built locally, and which will be +built externally. + +To achieve this hybrid build model: + +1. In your Backstage instance's `app-config.yaml`, set `techdocs.builder` to + `'local'`. This ensures that Backstage will build docs for users who want the + 'out-of-the-box' experience. +2. Configure external storage of TechDocs as normal for a production deployment. + This allows Backstage to publish documentation to your storage, as well as + allowing other users to publish documentation from their CI/CD pipelines. +3. Create a custom build strategy, that implements the `DocsBuildStrategy` interface, + and which implements your custom logic for determining whether to build docs for + a given entity. + For example, to only build docs when an entity has the `company.com/techdocs-builder` + annotation set to `'local'`: + ```typescript + export class AnnotationBasedBuildStrategy { + private readonly config: Config; + + constructor(config: Config) { + this.config = config; + } + + async shouldBuild(_: Entity): Promise { + return this.entity.metadata?.annotations?.["company.com/techdocs-builder"] === 'local' + } + } + ``` +4. Pass an instance of this Build Strategy as the `docsBuildStrategy` parameter of the + TechDocs backend `createRouter` method. + +Users should now be able to choose to have their documentation built and published by +the TechDocs backend by adding the `company.com/techdocs-builder` annotation to their +entity. If the value of this annotation is `'local'`, the TechDocs backend will build +and publish the documentation for them. If the value of the `company.com/techdocs-builder` +annotation is anything other than `'local'`, the user is responsible for publishing +documentation to the appropriate location in the TechDocs external storage. From 55031596f23c5ef65462594f4f23fc4755924c57 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Tue, 1 Mar 2022 09:05:59 +1100 Subject: [PATCH 05/12] Prettify how to Signed-off-by: Nikolas Skoufis --- docs/features/techdocs/how-to-guides.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/features/techdocs/how-to-guides.md b/docs/features/techdocs/how-to-guides.md index 5ec8aac307..5a5fe36526 100644 --- a/docs/features/techdocs/how-to-guides.md +++ b/docs/features/techdocs/how-to-guides.md @@ -566,6 +566,7 @@ To achieve this hybrid build model: a given entity. For example, to only build docs when an entity has the `company.com/techdocs-builder` annotation set to `'local'`: + ```typescript export class AnnotationBasedBuildStrategy { private readonly config: Config; @@ -575,10 +576,14 @@ To achieve this hybrid build model: } async shouldBuild(_: Entity): Promise { - return this.entity.metadata?.annotations?.["company.com/techdocs-builder"] === 'local' + return ( + this.entity.metadata?.annotations?.['company.com/techdocs-builder'] === + 'local' + ); } } ``` + 4. Pass an instance of this Build Strategy as the `docsBuildStrategy` parameter of the TechDocs backend `createRouter` method. From 588445b175d70a40810fbaa5724226254d833bf7 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Tue, 1 Mar 2022 09:18:44 +1100 Subject: [PATCH 06/12] Exporting interface and adding docs This makes the api reporter happy, with good reason Signed-off-by: Nikolas Skoufis --- plugins/techdocs-backend/api-report.md | 10 ++++++---- plugins/techdocs-backend/src/index.ts | 1 + .../techdocs-backend/src/service/DocsBuildStrategy.ts | 5 +++++ plugins/techdocs-backend/src/service/index.ts | 1 + 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/plugins/techdocs-backend/api-report.md b/plugins/techdocs-backend/api-report.md index 28700b245b..3ce3a2504b 100644 --- a/plugins/techdocs-backend/api-report.md +++ b/plugins/techdocs-backend/api-report.md @@ -42,6 +42,12 @@ export class DefaultTechDocsCollator implements DocumentCollator { readonly visibilityPermission: Permission; } +// @public +export interface DocsBuildStrategy { + // (undocumented) + shouldBuild(entity: Entity): Promise; +} + // @public export type OutOfTheBoxDeploymentOptions = { preparers: PreparerBuilder; @@ -84,8 +90,4 @@ export type TechDocsCollatorOptions = { export { TechDocsDocument }; export * from '@backstage/techdocs-common'; - -// Warnings were encountered during analysis: -// -// src/service/router.d.ts:24:5 - (ae-forgotten-export) The symbol "DocsBuildStrategy" needs to be exported by the entry point index.d.ts ``` diff --git a/plugins/techdocs-backend/src/index.ts b/plugins/techdocs-backend/src/index.ts index 12e7df1e9e..2a17bf2736 100644 --- a/plugins/techdocs-backend/src/index.ts +++ b/plugins/techdocs-backend/src/index.ts @@ -25,6 +25,7 @@ export type { RouterOptions, RecommendedDeploymentOptions, OutOfTheBoxDeploymentOptions, + DocsBuildStrategy, } from './service'; export { DefaultTechDocsCollator } from './search'; diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts index 69b0474a85..2e8cc52d3e 100644 --- a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts @@ -16,6 +16,11 @@ import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +/** + * A strategy for when to build TechDocs locally, and when to skip building TechDocs (allowing for an external build) + * + * @public + */ export interface DocsBuildStrategy { shouldBuild(entity: Entity): Promise; } diff --git a/plugins/techdocs-backend/src/service/index.ts b/plugins/techdocs-backend/src/service/index.ts index 0065e33a2a..29db2d2038 100644 --- a/plugins/techdocs-backend/src/service/index.ts +++ b/plugins/techdocs-backend/src/service/index.ts @@ -20,3 +20,4 @@ export type { RecommendedDeploymentOptions, OutOfTheBoxDeploymentOptions, } from './router'; +export type { DocsBuildStrategy } from './DocsBuildStrategy'; From 7a22a78180fffc4a1c82f27367344ffc10c6c800 Mon Sep 17 00:00:00 2001 From: Nik Skoufis Date: Tue, 1 Mar 2022 09:05:09 +1100 Subject: [PATCH 07/12] Apply suggestions from code review Co-authored-by: Emma Indal Signed-off-by: Nikolas Skoufis --- plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts index 71e5377a03..029d8d223f 100644 --- a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts @@ -1,5 +1,5 @@ /* - * Copyright 2021 The Backstage Authors + * Copyright 2022 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. From 5adcf26b0513d6958d736ab4117c810415093772 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Tue, 1 Mar 2022 09:23:47 +1100 Subject: [PATCH 08/12] Refactor to a private constructor and static fromConfig Signed-off-by: Nikolas Skoufis --- .../techdocs-backend/src/service/DocsBuildStrategy.test.ts | 6 ++++-- plugins/techdocs-backend/src/service/DocsBuildStrategy.ts | 6 +++++- plugins/techdocs-backend/src/service/router.ts | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts index 029d8d223f..76b70e67e9 100644 --- a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts @@ -41,7 +41,8 @@ describe('DefaultDocsBuildStrategy', () => { describe('shouldBuild', () => { it('should return true when techdocs.build is set to local', async () => { - const defaultDocsBuildStrategy = new DefaultDocsBuildStrategy(config); + const defaultDocsBuildStrategy = + DefaultDocsBuildStrategy.fromConfig(config); MockedConfigReader.prototype.getString.mockReturnValue('local'); @@ -51,7 +52,8 @@ describe('DefaultDocsBuildStrategy', () => { }); it('should return false when techdocs.build is set to external', async () => { - const defaultDocsBuildStrategy = new DefaultDocsBuildStrategy(config); + const defaultDocsBuildStrategy = + DefaultDocsBuildStrategy.fromConfig(config); MockedConfigReader.prototype.getString.mockReturnValue('external'); diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts index 2e8cc52d3e..e0220d7a32 100644 --- a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts @@ -28,10 +28,14 @@ export interface DocsBuildStrategy { export class DefaultDocsBuildStrategy { private readonly config: Config; - constructor(config: Config) { + private constructor(config: Config) { this.config = config; } + static fromConfig(config: Config): DefaultDocsBuildStrategy { + return new DefaultDocsBuildStrategy(config); + } + async shouldBuild(_: Entity): Promise { return this.config.getString('techdocs.builder') === 'local'; } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index d620020245..4d51ebd485 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -106,7 +106,7 @@ export async function createRouter( const { publisher, config, logger, discovery } = options; const catalogClient = new CatalogClient({ discoveryApi: discovery }); const docsBuildStrategy = - options.docsBuildStrategy ?? new DefaultDocsBuildStrategy(config); + options.docsBuildStrategy ?? DefaultDocsBuildStrategy.fromConfig(config); // Entities are cached to optimize the /static/docs request path, which can be called many times // when loading a single techdocs page. From 44a1a447cc37100cd6e175308e8b77b92f8f7a71 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Tue, 1 Mar 2022 19:28:53 +1100 Subject: [PATCH 09/12] Fix up links to other pages Signed-off-by: Nikolas Skoufis --- docs/features/techdocs/concepts.md | 2 +- docs/features/techdocs/how-to-guides.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/features/techdocs/concepts.md b/docs/features/techdocs/concepts.md index f8e813d89e..e34f58bc70 100644 --- a/docs/features/techdocs/concepts.md +++ b/docs/features/techdocs/concepts.md @@ -62,7 +62,7 @@ However any logic that satisfies the Build Strategy interface can be implemented config as well as the entity being processed to make a decision. For an example of how the Build Strategy can be used to implement a 'hybrid' build model, refer to -the [How to implement a hybrid build strategy](./how-to-guides#how-to-implement-a-hybrid-build-strategy) guide. +the [How to implement a hybrid build strategy](./how-to-guides.md#how-to-implement-a-hybrid-build-strategy) guide. ## TechDocs Container diff --git a/docs/features/techdocs/how-to-guides.md b/docs/features/techdocs/how-to-guides.md index 5a5fe36526..56ca2252d0 100644 --- a/docs/features/techdocs/how-to-guides.md +++ b/docs/features/techdocs/how-to-guides.md @@ -541,7 +541,7 @@ Done! Now you have a support of the following diagrams along with mermaid: ## How to implement a hybrid build strategy -One limitation of the [Recommended deployment](./architecture#recommended-deployment) is that +One limitation of the [Recommended deployment](./architecture.md#recommended-deployment) is that the experience for users requires modifying their CI/CD process to publish their TechDocs. For some users, this may be unnecessary, and provides a barrier to entry for onboarding users to Backstage. However, a purely local TechDocs @@ -549,7 +549,7 @@ build restricts TechDocs creators to using the tooling provided in Backstage, as well as the plugins and features provided in the Backstage-included `mkdocs` installation. -To accommodate both of these use-cases, users can implement a custom [Build Strategy](./concepts#techdocs-build-strategy) +To accommodate both of these use-cases, users can implement a custom [Build Strategy](./concepts.md#techdocs-build-strategy) with logic to encode which TechDocs should be built locally, and which will be built externally. From 4368196fba0d145fdb7bb478301fb5f8e92a89b0 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Wed, 2 Mar 2022 10:06:23 +1100 Subject: [PATCH 10/12] Accept object in shouldBuild for future extensibility Signed-off-by: Nikolas Skoufis --- plugins/techdocs-backend/api-report.md | 7 ++++++- plugins/techdocs-backend/src/index.ts | 1 + .../src/service/DocsBuildStrategy.test.ts | 4 ++-- .../src/service/DocsBuildStrategy.ts | 13 +++++++++++-- plugins/techdocs-backend/src/service/index.ts | 5 ++++- plugins/techdocs-backend/src/service/router.ts | 2 +- 6 files changed, 25 insertions(+), 7 deletions(-) diff --git a/plugins/techdocs-backend/api-report.md b/plugins/techdocs-backend/api-report.md index 3ce3a2504b..2bf4664f77 100644 --- a/plugins/techdocs-backend/api-report.md +++ b/plugins/techdocs-backend/api-report.md @@ -45,7 +45,7 @@ export class DefaultTechDocsCollator implements DocumentCollator { // @public export interface DocsBuildStrategy { // (undocumented) - shouldBuild(entity: Entity): Promise; + shouldBuild(params: ShouldBuildParameters): Promise; } // @public @@ -76,6 +76,11 @@ export type RouterOptions = | RecommendedDeploymentOptions | OutOfTheBoxDeploymentOptions; +// @public +export type ShouldBuildParameters = { + entity: Entity; +}; + // @public export type TechDocsCollatorOptions = { discovery: PluginEndpointDiscovery; diff --git a/plugins/techdocs-backend/src/index.ts b/plugins/techdocs-backend/src/index.ts index 2a17bf2736..01acbea2cc 100644 --- a/plugins/techdocs-backend/src/index.ts +++ b/plugins/techdocs-backend/src/index.ts @@ -26,6 +26,7 @@ export type { RecommendedDeploymentOptions, OutOfTheBoxDeploymentOptions, DocsBuildStrategy, + ShouldBuildParameters, } from './service'; export { DefaultTechDocsCollator } from './search'; diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts index 76b70e67e9..84bd960f8f 100644 --- a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts @@ -46,7 +46,7 @@ describe('DefaultDocsBuildStrategy', () => { MockedConfigReader.prototype.getString.mockReturnValue('local'); - const result = await defaultDocsBuildStrategy.shouldBuild(entity); + const result = await defaultDocsBuildStrategy.shouldBuild({ entity }); expect(result).toBe(true); }); @@ -57,7 +57,7 @@ describe('DefaultDocsBuildStrategy', () => { MockedConfigReader.prototype.getString.mockReturnValue('external'); - const result = await defaultDocsBuildStrategy.shouldBuild(entity); + const result = await defaultDocsBuildStrategy.shouldBuild({ entity }); expect(result).toBe(false); }); diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts index e0220d7a32..42a16234f9 100644 --- a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts @@ -16,13 +16,22 @@ import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +/** + * Parameters passed to the shouldBuild method on the DocsBuildStrategy interface + * + * @public + */ +export type ShouldBuildParameters = { + entity: Entity; +}; + /** * A strategy for when to build TechDocs locally, and when to skip building TechDocs (allowing for an external build) * * @public */ export interface DocsBuildStrategy { - shouldBuild(entity: Entity): Promise; + shouldBuild(params: ShouldBuildParameters): Promise; } export class DefaultDocsBuildStrategy { @@ -36,7 +45,7 @@ export class DefaultDocsBuildStrategy { return new DefaultDocsBuildStrategy(config); } - async shouldBuild(_: Entity): Promise { + async shouldBuild(_: ShouldBuildParameters): Promise { return this.config.getString('techdocs.builder') === 'local'; } } diff --git a/plugins/techdocs-backend/src/service/index.ts b/plugins/techdocs-backend/src/service/index.ts index 29db2d2038..7355a34e32 100644 --- a/plugins/techdocs-backend/src/service/index.ts +++ b/plugins/techdocs-backend/src/service/index.ts @@ -20,4 +20,7 @@ export type { RecommendedDeploymentOptions, OutOfTheBoxDeploymentOptions, } from './router'; -export type { DocsBuildStrategy } from './DocsBuildStrategy'; +export type { + DocsBuildStrategy, + ShouldBuildParameters, +} from './DocsBuildStrategy'; diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 4d51ebd485..abe83b3d47 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -221,7 +221,7 @@ export async function createRouter( // techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to 'local' // If set to 'external', it will assume that an external process (e.g. CI/CD pipeline // of the repository) is responsible for building and publishing documentation to the storage provider - const shouldBuild = await docsBuildStrategy.shouldBuild(entity); + const shouldBuild = await docsBuildStrategy.shouldBuild({ entity }); if (!shouldBuild) { // However, if caching is enabled, take the opportunity to check and // invalidate stale cache entries. From 63bb0a96449fd3ff9b2448596027c26446fd03d9 Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Wed, 2 Mar 2022 10:09:28 +1100 Subject: [PATCH 11/12] Update comments and logs for shouldBuild Signed-off-by: Nikolas Skoufis --- plugins/techdocs-backend/src/service/router.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index abe83b3d47..183e26c95d 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -218,9 +218,11 @@ export async function createRouter( responseHandler = createEventStream(res); } - // techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to 'local' - // If set to 'external', it will assume that an external process (e.g. CI/CD pipeline - // of the repository) is responsible for building and publishing documentation to the storage provider + // By default, techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to + // 'local'. If set to 'external', it will assume that an external process (e.g. CI/CD pipeline + // of the repository) is responsible for building and publishing documentation to the storage provider. + // Altering the implementation of the injected docsBuildStrategy allows for more complex behaviours, based on + // either config or the properties of the entity (e.g. annotations, labels, spec fields etc.). const shouldBuild = await docsBuildStrategy.shouldBuild({ entity }); if (!shouldBuild) { // However, if caching is enabled, take the opportunity to check and @@ -253,7 +255,7 @@ export async function createRouter( responseHandler.error( new Error( - "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'preparer' was provided to the router initialization.", + "Invalid configuration. 'docsBuildStrategy.shouldBuild returned 'true', but no 'preparer' was provided to the router initialization.", ), ); }); From eed6b57cf84dd6fe896684a8693ba2a2c71c9cec Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Wed, 2 Mar 2022 11:09:48 +1100 Subject: [PATCH 12/12] Update error message in tests Signed-off-by: Nikolas Skoufis --- plugins/techdocs-backend/src/service/router.test.ts | 4 ++-- plugins/techdocs-backend/src/service/router.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index 521024252c..c7cda203bb 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -216,7 +216,7 @@ describe('createRouter', () => { expect(response.status).toBe(500); expect(response.text).toMatch( - /Invalid configuration\. 'techdocs\.builder' was set to 'local' but no 'preparer' was provided to the router initialization/, + /Invalid configuration\. docsBuildStrategy\.shouldBuild returned 'true', but no 'preparer' was provided to the router initialization./, ); expect(MockDocsSynchronizer.prototype.doSync).toBeCalledTimes(0); @@ -343,7 +343,7 @@ data: {"updated":false} expect(response.get('content-type')).toBe('text/event-stream'); expect(response.text).toEqual( `event: error -data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'preparer' was provided to the router initialization." +data: "Invalid configuration. docsBuildStrategy.shouldBuild returned 'true', but no 'preparer' was provided to the router initialization." `, ); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 183e26c95d..73f212d049 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -255,7 +255,7 @@ export async function createRouter( responseHandler.error( new Error( - "Invalid configuration. 'docsBuildStrategy.shouldBuild returned 'true', but no 'preparer' was provided to the router initialization.", + "Invalid configuration. docsBuildStrategy.shouldBuild returned 'true', but no 'preparer' was provided to the router initialization.", ), ); });