diff --git a/.changeset/techdocs-eleven-planes-teach.md b/.changeset/techdocs-eleven-planes-teach.md new file mode 100644 index 0000000000..2cc4e42175 --- /dev/null +++ b/.changeset/techdocs-eleven-planes-teach.md @@ -0,0 +1,15 @@ +--- +'@backstage/techdocs-common': patch +'@backstage/plugin-techdocs-backend': patch +--- + +Applies only if you use TechDocs local builder instead of building on CI/CD i.e. if `techdocs.builder` in your `app-config.yaml` is set to `'local'` + +Improvements + +1. Do not check for updates in the repository if a check has been made in the last 60 seconds. This is to prevent the annoying check for update on every page switch or load. +2. No need to maintain an in-memory etag storage, and use the one stored in `techdocs_metadata.json` file alongside generated docs. + +New feature + +1. You can now use a mix of basic and recommended setup i.e. `techdocs.builder` is `'local'` but using an external cloud storage instead of local storage. Previously, in this setup, the docs would never get updated. diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 218d3dd88f..d3281c2645 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -192,13 +192,14 @@ describe('AwsS3Publish', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - '{"site_name": "backstage", "site_description": "site_content"}', + '{"site_name": "backstage", "site_description": "site_content", "etag": "etag"}', }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -213,13 +214,14 @@ describe('AwsS3Publish', () => { mockFs({ [entityRootDir]: { - 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content'}`, + 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag'}`, }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 44262768a5..9442fb0e37 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -184,12 +184,13 @@ describe('publishing with valid credentials', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - '{"site_name": "backstage", "site_description": "site_content"}', + '{"site_name": "backstage", "site_description": "site_content", "etag": "etag"}', }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -204,13 +205,14 @@ describe('publishing with valid credentials', () => { mockFs({ [entityRootDir]: { - 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content'}`, + 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag'}`, }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -342,12 +344,13 @@ describe('error reporting', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - '{"site_name": "backstage", "site_description": "site_content"}', + '{"site_name": "backstage", "site_description": "site_content", "etag": "etag"}', }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -362,13 +365,14 @@ describe('error reporting', () => { mockFs({ [entityRootDir]: { - 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content'}`, + 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag'}`, }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 10da0d476a..07bc5c06b9 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -193,13 +193,14 @@ describe('GoogleGCSPublish', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - '{"site_name": "backstage", "site_description": "site_content"}', + '{"site_name": "backstage", "site_description": "site_content", "etag": "etag"}', }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -214,13 +215,14 @@ describe('GoogleGCSPublish', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - "{'site_name': 'backstage', 'site_description': 'site_content'}", + "{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag'}", }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index ac09331dcd..3088c1a695 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -141,7 +141,7 @@ export class LocalPublish implements PublisherBase { // Check if the file exists try { - fs.access(indexHtmlPath, fs.constants.F_OK); + await fs.access(indexHtmlPath, fs.constants.F_OK); return true; } catch (err) { return false; diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts index b831012acf..f3b788916f 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts @@ -191,13 +191,14 @@ describe('OpenStackSwiftPublish', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - '{"site_name": "backstage", "site_description": "site_content"}', + '{"site_name": "backstage", "site_description": "site_content", "etag": "etag"}', }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -212,13 +213,14 @@ describe('OpenStackSwiftPublish', () => { mockFs({ [entityRootDir]: { - 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content'}`, + 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag'}`, }, }); const expectedMetadata: TechDocsMetadata = { site_name: 'backstage', site_description: 'site_content', + etag: 'etag', }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), diff --git a/packages/techdocs-common/src/stages/publish/types.ts b/packages/techdocs-common/src/stages/publish/types.ts index f0ecb3cb2b..83fee55133 100644 --- a/packages/techdocs-common/src/stages/publish/types.ts +++ b/packages/techdocs-common/src/stages/publish/types.ts @@ -39,10 +39,12 @@ export type PublishResponse = { /** * Type to hold metadata found in techdocs_metadata.json and associated with each site + * @param etag ETag of the resource used to generate the site. Usually the latest commit sha of the source repository. */ export type TechDocsMetadata = { site_name: string; site_description: string; + etag: string; }; /** diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts index 54ea860fb9..a6b7294866 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts @@ -18,10 +18,10 @@ import { BuildMetadataStorage } from './BuildMetadataStorage'; describe('BuildMetadataStorage', () => { it('should return build timestamp', () => { const newMetadataStorage = new BuildMetadataStorage('entityID123abc'); - newMetadataStorage.setEtag('etag123abc'); + newMetadataStorage.setLastUpdated(); - const timestamp = newMetadataStorage.getEtag(); + const timestamp = newMetadataStorage.getLastUpdated(); - expect(timestamp).toBe('etag123abc'); + expect(timestamp).toBeLessThanOrEqual(Date.now()); }); }); diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts index a1bb918fae..ce42716e3b 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts @@ -13,37 +13,42 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -type buildInfo = { - // Entity uid: etag - [key: string]: string; -}; -// TODO: Build info should be part of TechDocs storage, inside `techdocs_metadata.json` -// instead of in-memory storage of the Backstage instance. -// In case of multi-region Backstage deployments, or even using multiple Kubernetes pods, -// if each instance creates its separate build info in-memory, it will result in duplicate -// builds per instance. Also if the pod restarts, all the sites will have to be re-built. -const builds = {} as buildInfo; +// Entity uid: unix timestamp +const lastUpdatedRecord = {} as Record; /** - * Store timestamps of the most recent TechDocs build of each Entity. This is - * used to invalidate cache if the latest commit in the documentation source - * repository is later than the timestamp. + * Store timestamps of the most recent TechDocs update of each Entity. This is + * used to avoid checking for an update on each and every request to TechDocs. */ export class BuildMetadataStorage { - public entityUid: string; - private builds: buildInfo; + private entityUid: string; + private lastUpdatedRecord: Record; constructor(entityUid: string) { this.entityUid = entityUid; - this.builds = builds; + this.lastUpdatedRecord = lastUpdatedRecord; } - setEtag(etag: string): void { - this.builds[this.entityUid] = etag; + setLastUpdated(): void { + this.lastUpdatedRecord[this.entityUid] = Date.now(); } - getEtag(): string | undefined { - return this.builds[this.entityUid]; + getLastUpdated(): number | undefined { + return this.lastUpdatedRecord[this.entityUid]; } } + +/** + * Return false if a check for update has happened in last 60 seconds. + */ +export const shouldCheckForUpdate = (entityUid: string) => { + const lastUpdated = new BuildMetadataStorage(entityUid).getLastUpdated(); + if (lastUpdated) { + // The difference is in milliseconds + if (Date.now() - lastUpdated < 60 * 1000) { + return false; + } + } + return true; +}; diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index f42a708bb2..5d8ee3612d 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -13,8 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { + Entity, + ENTITY_DEFAULT_NAMESPACE, + serializeEntityRef, +} from '@backstage/catalog-model'; import { NotModifiedError } from '@backstage/errors'; -import { Entity, serializeEntityRef } from '@backstage/catalog-model'; import { GeneratorBase, GeneratorBuilder, @@ -81,28 +85,41 @@ export class DocsBuilder { )}`, ); - // Use the in-memory storage for setting and getting etag for this entity. - const buildMetadataStorage = new BuildMetadataStorage( - this.entity.metadata.uid, - ); + // If available, use the etag stored in techdocs_metadata.json to + // check if docs are outdated and need to be regenerated. + let storedEtag: string | undefined; + if (await this.publisher.hasDocsBeenGenerated(this.entity)) { + try { + storedEtag = ( + await this.publisher.fetchTechDocsMetadata({ + namespace: + this.entity.metadata.namespace ?? ENTITY_DEFAULT_NAMESPACE, + kind: this.entity.kind, + name: this.entity.metadata.name, + }) + ).etag; + } catch (err) { + // Proceed with a fresh build + this.logger.warn( + `Unable to read techdocs_metadata.json, error ${err}.`, + ); + } + } - // TODO: As of now, this happens on each and every request to TechDocs. - // In a high traffic environment, this will cause a lot of requests to the source code provider. - // After Async build is implemented https://github.com/backstage/backstage/issues/3717, - // make sure to limit checking for cache invalidation to once per minute or so. let preparedDir: string; - let etag: string; - + let newEtag: string; try { const preparerResponse = await this.preparer.prepare(this.entity, { - etag: buildMetadataStorage.getEtag(), + etag: storedEtag, }); preparedDir = preparerResponse.preparedDir; - etag = preparerResponse.etag; + newEtag = preparerResponse.etag; } catch (err) { if (err instanceof NotModifiedError) { // No need to prepare anymore since cache is valid. + // Set last check happened to now + new BuildMetadataStorage(this.entity.metadata.uid).setLastUpdated(); this.logger.debug( `Docs for ${serializeEntityRef( this.entity, @@ -142,7 +159,7 @@ export class DocsBuilder { outputDir, dockerClient: this.dockerClient, parsedLocationAnnotation, - etag, + etag: newEtag, }); // Remove Prepared directory since it is no longer needed. @@ -185,7 +202,7 @@ export class DocsBuilder { this.logger.debug(`Error removing generated directory ${error.message}`); } - // Store the latest build etag for the entity - new BuildMetadataStorage(this.entity.metadata.uid).setEtag(etag); + // Update the last check time for the entity + new BuildMetadataStorage(this.entity.metadata.uid).setLastUpdated(); } } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 834a3a78ef..4b10063e03 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -29,6 +29,7 @@ import Router from 'express-promise-router'; import { Knex } from 'knex'; import { Logger } from 'winston'; import { DocsBuilder } from '../DocsBuilder'; +import { shouldCheckForUpdate } from '../DocsBuilder/BuildMetadataStorage'; import { getEntityNameFromUrlPath } from './helpers'; type RouterOptions = { @@ -140,7 +141,11 @@ 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 only try to fetch and 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') { + if ( + config.getString('techdocs.builder') === 'local' && + entity.metadata.uid && + shouldCheckForUpdate(entity.metadata.uid) + ) { const docsBuilder = new DocsBuilder({ preparers, generators, @@ -149,56 +154,42 @@ export async function createRouter({ logger, entity, }); + let foundDocs = false; switch (publisherType) { case 'local': - await docsBuilder.build(); - break; case 'awsS3': case 'azureBlobStorage': case 'openStackSwift': case 'googleGcs': - // This block should be valid for all external storage implementations. So no need to duplicate in future, + // This block should be valid for all storage implementations. So no need to duplicate in future, // add the publisher type in the list here. - if (!(await publisher.hasDocsBeenGenerated(entity))) { - logger.info( - 'No pre-generated documentation files found for the entity in the storage. Building docs...', - ); - await docsBuilder.build(); - // With a maximum of ~5 seconds wait, check if the files got published and if docs will be fetched - // on the user's page. If not, respond with a message asking them to check back later. - // The delay here is to make sure GCS/AWS/etc. registers newly uploaded files which is usually <1 second - let foundDocs = false; - for (let attempt = 0; attempt < 5; attempt++) { - if (await publisher.hasDocsBeenGenerated(entity)) { - foundDocs = true; - break; - } - await new Promise(r => setTimeout(r, 1000)); + await docsBuilder.build(); + // With a maximum of ~5 seconds wait, check if the files got published and if docs will be fetched + // on the user's page. If not, respond with a message asking them to check back later. + // The delay here is to make sure GCS/AWS/etc. registers newly uploaded files which is usually <1 second + for (let attempt = 0; attempt < 5; attempt++) { + if (await publisher.hasDocsBeenGenerated(entity)) { + foundDocs = true; + break; } - if (!foundDocs) { - logger.error( - 'Published files are taking longer to show up in storage. Something went wrong.', - ); - res - .status(408) - .send( - 'Sorry! It is taking longer for the generated docs to show up in storage. Check back later.', - ); - return; - } - } else { - logger.info( - 'Found pre-generated docs for this entity. Serving them.', + await new Promise(r => setTimeout(r, 1000)); + } + if (!foundDocs) { + logger.error( + 'Published files are taking longer to show up in storage. Something went wrong.', ); - // TODO: re-trigger build for cache invalidation. - // Add build info in techdocs_metadata.json and compare it against - // the etag/commit in the repository. - // Without this, docs will not be re-built once they have been generated. - // Although it is unconventional that anyone will face this issue - because - // if you have an external storage, you should be using CI/CD to build and publish docs. + res.status(408).json({ + error: + 'Sorry! It took too long for the generated docs to show up in storage. Check back later.', + }); + return; } break; default: + res.status(400).json({ + error: `Publisher type ${publisherType} is not supported by techdocs-backend docs builder.`, + }); + break; } }