diff --git a/.changeset/techdocs-green-singers-jump.md b/.changeset/techdocs-green-singers-jump.md new file mode 100644 index 0000000000..79d1e82a81 --- /dev/null +++ b/.changeset/techdocs-green-singers-jump.md @@ -0,0 +1,6 @@ +--- +'@backstage/techdocs-common': minor +'@backstage/plugin-techdocs-backend': minor +--- + +URL Preparer will now use proper etag based caching introduced in https://github.com/backstage/backstage/pull/4120. Previously, builds used to be cached for 30 minutes. diff --git a/.changeset/techdocs-new-needles-arrive.md b/.changeset/techdocs-new-needles-arrive.md new file mode 100644 index 0000000000..c00df772ad --- /dev/null +++ b/.changeset/techdocs-new-needles-arrive.md @@ -0,0 +1,8 @@ +--- +'@backstage/techdocs-common': patch +'@backstage/plugin-techdocs-backend': patch +--- + +TechDocs will throw warning in backend logs when legacy git preparer or dir preparer is used to preparer docs. Migrate to URL Preparer by updating `backstage.io/techdocs-ref` annotation to be prefixed with `url:`. +Detailed docs are here https://backstage.io/docs/features/techdocs/how-to-guides#how-to-use-url-reader-in-techdocs-prepare-step +See benefits and reason for doing so https://github.com/backstage/backstage/issues/4409 diff --git a/docs/features/techdocs/getting-started.md b/docs/features/techdocs/getting-started.md index e0b323857d..efed896fd0 100644 --- a/docs/features/techdocs/getting-started.md +++ b/docs/features/techdocs/getting-started.md @@ -157,15 +157,17 @@ techdocs: builder: 'local' ``` -Set `techdocs.builder` to `'local'` if you want your TechDocs Backend to be -responsible for generating documentation sites. If set to `'external'`, -Backstage will assume that the sites are being generated on each entity's CI/CD -pipeline, and are being stored in a storage somewhere. +Note that we recommend generating docs on CI/CD instead. Read more in the +"Basic" and "Recommended" sections of the +[TechDocs Architecture](architecture.md). But if you want to get started quickly +set `techdocs.builder` to `'local'` so that TechDocs Backend is responsible for +generating documentation sites. If set to `'external'`, Backstage will assume +that the sites are being generated on each entity's CI/CD pipeline, and are +being stored in a storage somewhere. When `techdocs.builder` is set to `'external'`, TechDocs becomes more or less a read-only experience where it serves static files from a storage containing all -the generated documentation. Read more in the "Basic" and "Recommended" sections -of the [TechDocs Architecture](architecture.md). +the generated documentation. ### Choosing storage (publisher) diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index eba1386b11..6520f0f8e4 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -139,7 +139,7 @@ describe('getDocFilesFromRepository', () => { archive: async () => { return Readable.from(''); }, - etag: '', + etag: 'etag123abc', }; } @@ -156,6 +156,7 @@ describe('getDocFilesFromRepository', () => { mockEntityWithAnnotation, ); - expect(output).toBe('/tmp/testfolder'); + expect(output.preparedDir).toBe('/tmp/testfolder'); + expect(output.etag).toBe('etag123abc'); }); }); diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts index f70b0f3840..beff0b2a6e 100644 --- a/packages/techdocs-common/src/helpers.ts +++ b/packages/techdocs-common/src/helpers.ts @@ -14,17 +14,17 @@ * limitations under the License. */ -import os from 'os'; -import path from 'path'; -import parseGitUrl from 'git-url-parse'; -import fs from 'fs-extra'; -import { InputError, UrlReader, Git } from '@backstage/backend-common'; +import { Git, InputError, UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +import fs from 'fs-extra'; +import parseGitUrl from 'git-url-parse'; +import os from 'os'; +import path from 'path'; +import { Logger } from 'winston'; import { getDefaultBranch } from './default-branch'; import { getGitRepoType, getTokenForGitRepo } from './git-auth'; -import { RemoteProtocol } from './stages/prepare/types'; -import { Logger } from 'winston'; +import { PreparerResponse, RemoteProtocol } from './stages/prepare/types'; export type ParsedLocationAnnotation = { type: RemoteProtocol; @@ -196,16 +196,9 @@ export const checkoutGitRepository = async ( }; export const getLastCommitTimestamp = async ( - repositoryUrl: string, - config: Config, + repositoryLocation: string, logger: Logger, ): Promise => { - const repositoryLocation = await checkoutGitRepository( - repositoryUrl, - config, - logger, - ); - const git = Git.fromAuth({ logger }); const sha = await git.resolveRef({ dir: repositoryLocation, ref: 'HEAD' }); const commit = await git.readCommit({ dir: repositoryLocation, sha }); @@ -216,13 +209,22 @@ export const getLastCommitTimestamp = async ( export const getDocFilesFromRepository = async ( reader: UrlReader, entity: Entity, -): Promise => { + opts?: { etag?: string; logger?: Logger }, +): Promise => { const { target } = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, ); - const response = await reader.readTree(target); + opts?.logger?.info(`Reading files from ${target}`); + // readTree will throw NotModifiedError if etag has not changed. + const readTreeResponse = await reader.readTree(target, { etag: opts?.etag }); + const preparedDir = await readTreeResponse.dir(); - return await response.dir(); + opts?.logger?.info(`Tree downloaded and stored at ${preparedDir}`); + + return { + preparedDir, + etag: readTreeResponse.etag, + }; }; diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.test.ts b/packages/techdocs-common/src/stages/prepare/commonGit.test.ts index 704cd23daa..dcc06313bd 100644 --- a/packages/techdocs-common/src/stages/prepare/commonGit.test.ts +++ b/packages/techdocs-common/src/stages/prepare/commonGit.test.ts @@ -16,8 +16,8 @@ import { getVoidLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; -import { CommonGitPreparer } from './commonGit'; import { checkoutGitRepository } from '../../helpers'; +import { CommonGitPreparer } from './commonGit'; function normalizePath(path: string) { return path @@ -29,6 +29,7 @@ function normalizePath(path: string) { jest.mock('../../helpers', () => ({ ...jest.requireActual<{}>('../../helpers'), checkoutGitRepository: jest.fn(() => '/tmp/backstage-repo/org/name/branch'), + getLastCommitTimestamp: jest.fn(() => 12345678), })); const createMockEntity = (annotations = {}) => { @@ -57,9 +58,9 @@ describe('commonGit preparer', () => { 'github:https://github.com/backstage/backstage/blob/master/plugins/techdocs-backend/examples/documented-component', }); - const tempDocsPath = await preparer.prepare(mockEntity); + const { preparedDir } = await preparer.prepare(mockEntity); expect(checkoutGitRepository).toHaveBeenCalledTimes(1); - expect(normalizePath(tempDocsPath)).toEqual( + expect(normalizePath(preparedDir)).toEqual( '/tmp/backstage-repo/org/name/branch/plugins/techdocs-backend/examples/documented-component', ); }); @@ -72,9 +73,9 @@ describe('commonGit preparer', () => { 'gitlab:https://gitlab.com/xesjkeee/go-logger/blob/master/catalog-info.yaml', }); - const tempDocsPath = await preparer.prepare(mockEntity); + const { preparedDir } = await preparer.prepare(mockEntity); expect(checkoutGitRepository).toHaveBeenCalledTimes(2); - expect(normalizePath(tempDocsPath)).toEqual( + expect(normalizePath(preparedDir)).toEqual( '/tmp/backstage-repo/org/name/branch/catalog-info.yaml', ); }); @@ -87,9 +88,9 @@ describe('commonGit preparer', () => { 'azure/api:https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=%2Ftemplate.yaml', }); - const tempDocsPath = await preparer.prepare(mockEntity); + const { preparedDir } = await preparer.prepare(mockEntity); expect(checkoutGitRepository).toHaveBeenCalledTimes(3); - expect(normalizePath(tempDocsPath)).toEqual( + expect(normalizePath(preparedDir)).toEqual( '/tmp/backstage-repo/org/name/branch/template.yaml', ); }); diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.ts b/packages/techdocs-common/src/stages/prepare/commonGit.ts index 7eac07d76f..46b96675f3 100644 --- a/packages/techdocs-common/src/stages/prepare/commonGit.ts +++ b/packages/techdocs-common/src/stages/prepare/commonGit.ts @@ -13,14 +13,18 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import path from 'path'; -import parseGitUrl from 'git-url-parse'; +import { NotModifiedError } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { PreparerBase } from './types'; -import { parseReferenceAnnotation, checkoutGitRepository } from '../../helpers'; - +import parseGitUrl from 'git-url-parse'; +import path from 'path'; import { Logger } from 'winston'; +import { + checkoutGitRepository, + getLastCommitTimestamp, + parseReferenceAnnotation, +} from '../../helpers'; +import { PreparerBase, PreparerResponse } from './types'; export class CommonGitPreparer implements PreparerBase { private readonly config: Config; @@ -31,23 +35,45 @@ export class CommonGitPreparer implements PreparerBase { this.logger = logger; } - async prepare(entity: Entity): Promise { + async prepare( + entity: Entity, + options?: { etag?: string }, + ): Promise { + this.logger.warn( + 'You are using the legacy git preparer in TechDocs which will be removed in near future (March 2021). ' + + 'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' + + 'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ', + ); + const { target } = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, ); try { + // Update repository or do a fresh clone. const repoPath = await checkoutGitRepository( target, this.config, this.logger, ); - const parsedGitLocation = parseGitUrl(target); + // Check if etag has changed for cache invalidation. + const etag = await getLastCommitTimestamp(repoPath, this.logger); + if (options?.etag === etag.toString()) { + throw new NotModifiedError(); + } - return path.join(repoPath, parsedGitLocation.filepath); + const parsedGitLocation = parseGitUrl(target); + return { + preparedDir: path.join(repoPath, parsedGitLocation.filepath), + etag: etag.toString(), + }; } catch (error) { - this.logger.debug(`Repo checkout failed with error ${error.message}`); + if (error instanceof NotModifiedError) { + this.logger.debug(`Cache is valid for etag ${options?.etag}`); + } else { + this.logger.debug(`Repo checkout failed with error ${error.message}`); + } throw error; } } diff --git a/packages/techdocs-common/src/stages/prepare/dir.test.ts b/packages/techdocs-common/src/stages/prepare/dir.test.ts index cd8d358afe..67fa0dd21f 100644 --- a/packages/techdocs-common/src/stages/prepare/dir.test.ts +++ b/packages/techdocs-common/src/stages/prepare/dir.test.ts @@ -15,8 +15,8 @@ */ import { getVoidLogger, UrlReader } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; -import { DirectoryPreparer } from './dir'; import { checkoutGitRepository } from '../../helpers'; +import { DirectoryPreparer } from './dir'; function normalizePath(path: string) { return path @@ -28,6 +28,7 @@ function normalizePath(path: string) { jest.mock('../../helpers', () => ({ ...jest.requireActual<{}>('../../helpers'), checkoutGitRepository: jest.fn(() => '/tmp/backstage-repo/org/name/branch/'), + getLastCommitTimestamp: jest.fn(() => 12345678), })); const logger = getVoidLogger(); @@ -66,9 +67,8 @@ describe('directory preparer', () => { 'backstage.io/techdocs-ref': 'dir:./our-documentation', }); - expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual( - '/directory/our-documentation', - ); + const { preparedDir } = await directoryPreparer.prepare(mockEntity); + expect(normalizePath(preparedDir)).toEqual('/directory/our-documentation'); }); it('should merge managed-by-location and techdocs-ref when techdocs-ref is absolute', async () => { @@ -84,9 +84,8 @@ describe('directory preparer', () => { 'backstage.io/techdocs-ref': 'dir:/our-documentation/techdocs', }); - expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual( - '/our-documentation/techdocs', - ); + const { preparedDir } = await directoryPreparer.prepare(mockEntity); + expect(normalizePath(preparedDir)).toEqual('/our-documentation/techdocs'); }); it('should merge managed-by-location and techdocs-ref when managed-by-location is a git repository', async () => { @@ -102,7 +101,8 @@ describe('directory preparer', () => { 'backstage.io/techdocs-ref': 'dir:./docs', }); - expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual( + const { preparedDir } = await directoryPreparer.prepare(mockEntity); + expect(normalizePath(preparedDir)).toEqual( '/tmp/backstage-repo/org/name/branch/docs', ); expect(checkoutGitRepository).toHaveBeenCalledTimes(1); diff --git a/packages/techdocs-common/src/stages/prepare/dir.ts b/packages/techdocs-common/src/stages/prepare/dir.ts index ab277fda9a..08832938df 100644 --- a/packages/techdocs-common/src/stages/prepare/dir.ts +++ b/packages/techdocs-common/src/stages/prepare/dir.ts @@ -13,14 +13,22 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { PreparerBase } from './types'; +import { + InputError, + NotModifiedError, + UrlReader, +} from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import path from 'path'; -import { parseReferenceAnnotation, checkoutGitRepository } from '../../helpers'; -import { UrlReader, InputError } from '@backstage/backend-common'; import parseGitUrl from 'git-url-parse'; +import path from 'path'; import { Logger } from 'winston'; +import { + checkoutGitRepository, + getLastCommitTimestamp, + parseReferenceAnnotation, +} from '../../helpers'; +import { PreparerBase, PreparerResponse } from './types'; export class DirectoryPreparer implements PreparerBase { constructor( @@ -33,7 +41,10 @@ export class DirectoryPreparer implements PreparerBase { this.reader = reader; } - private async resolveManagedByLocationToDir(entity: Entity) { + private async resolveManagedByLocationToDir( + entity: Entity, + options?: { etag?: string }, + ): Promise { const { type, target } = parseReferenceAnnotation( 'backstage.io/managed-by-location', entity, @@ -44,8 +55,14 @@ export class DirectoryPreparer implements PreparerBase { ); switch (type) { case 'url': { - const response = await this.reader.readTree(target); - return await response.dir(); + const response = await this.reader.readTree(target, { + etag: options?.etag, + }); + const preparedDir = await response.dir(); + return { + preparedDir, + etag: response.etag, + }; } case 'github': case 'gitlab': @@ -57,29 +74,47 @@ export class DirectoryPreparer implements PreparerBase { this.logger, ); - return path.dirname( - path.join(repoLocation, parsedGitLocation.filepath), - ); + // Check if etag has changed for cache invalidation. + const etag = await getLastCommitTimestamp(repoLocation, this.logger); + if (options?.etag === etag.toString()) { + throw new NotModifiedError(); + } + return { + preparedDir: path.dirname( + path.join(repoLocation, parsedGitLocation.filepath), + ), + etag: etag.toString(), + }; } case 'file': - return path.dirname(target); + return { + preparedDir: path.dirname(target), + // Instead of supporting caching on local sources, use techdocs-cli for local development and debugging. + etag: '', + }; default: throw new InputError(`Unable to resolve location type ${type}`); } } - async prepare(entity: Entity): Promise { + async prepare(entity: Entity): Promise { + this.logger.warn( + 'You are using the legacy dir preparer in TechDocs which will be removed in near future (March 2021). ' + + 'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' + + 'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ', + ); + const { target } = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, ); - const managedByLocationDirectory = await this.resolveManagedByLocationToDir( - entity, - ); + // This will throw NotModified error if etag has not changed. + const response = await this.resolveManagedByLocationToDir(entity); - return new Promise(resolve => { - resolve(path.resolve(managedByLocationDirectory, target)); - }); + return { + preparedDir: path.resolve(response.preparedDir, target), + etag: response.etag, + }; } } diff --git a/packages/techdocs-common/src/stages/prepare/types.ts b/packages/techdocs-common/src/stages/prepare/types.ts index 97e75306c3..96510e8eec 100644 --- a/packages/techdocs-common/src/stages/prepare/types.ts +++ b/packages/techdocs-common/src/stages/prepare/types.ts @@ -16,13 +16,31 @@ import type { Entity } from '@backstage/catalog-model'; import { Logger } from 'winston'; +export type PreparerResponse = { + /** + * The path to directory where the tree is downloaded. + */ + preparedDir: string; + /** + * A unique identifer of the tree blob, usually the commit SHA or etag from the target. + */ + etag: string; +}; + export type PreparerBase = { /** * Given an Entity definition from the Service Catalog, go and prepare a directory - * with contents from the location in temporary storage and return the path + * with contents from the location in temporary storage and return the path. + * * @param entity The entity from the Service Catalog + * @param options.etag (Optional) If etag is provider, it will be used to check if the target has + * updated since the last build. + * @throws {NotModifiedError} when the prepared directory has not been changed since the last build. */ - prepare(entity: Entity, opts?: { logger: Logger }): Promise; + prepare( + entity: Entity, + options?: { logger?: Logger; etag?: string }, + ): Promise; }; export type PreparerBuilder = { @@ -30,10 +48,14 @@ export type PreparerBuilder = { get(entity: Entity): PreparerBase; }; +/** + * Everything except `url` will be deprecated. + * Read more https://github.com/backstage/backstage/issues/4409 + */ export type RemoteProtocol = + | 'url' | 'dir' | 'github' | 'gitlab' | 'file' - | 'azure/api' - | 'url'; + | 'azure/api'; diff --git a/packages/techdocs-common/src/stages/prepare/url.ts b/packages/techdocs-common/src/stages/prepare/url.ts index b938c6b5bc..9e4715a4ee 100644 --- a/packages/techdocs-common/src/stages/prepare/url.ts +++ b/packages/techdocs-common/src/stages/prepare/url.ts @@ -13,11 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Logger } from 'winston'; +import { NotModifiedError, UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; -import { UrlReader } from '@backstage/backend-common'; -import { PreparerBase } from './types'; +import { Logger } from 'winston'; import { getDocFilesFromRepository } from '../../helpers'; +import { PreparerBase, PreparerResponse } from './types'; export class UrlPreparer implements PreparerBase { private readonly logger: Logger; @@ -28,13 +28,25 @@ export class UrlPreparer implements PreparerBase { this.reader = reader; } - async prepare(entity: Entity): Promise { + async prepare( + entity: Entity, + options?: { etag?: string }, + ): Promise { try { - return getDocFilesFromRepository(this.reader, entity); + return await getDocFilesFromRepository(this.reader, entity, { + etag: options?.etag, + logger: this.logger, + }); } catch (error) { - this.logger.debug( - `Unable to fetch files for building docs ${error.message}`, - ); + // NotModifiedError means that etag based cache is still valid. + if (error instanceof NotModifiedError) { + this.logger.debug(`Cache is valid for etag ${options?.etag}`); + } else { + this.logger.debug( + `Unable to fetch files for building docs ${error.message}`, + ); + } + throw error; } } diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index e349a2119a..ac09331dcd 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -88,7 +88,7 @@ export class LocalPublish implements PublisherBase { ); reject(err); } - + this.logger.info(`Published site stored at ${publishDir}`); this.discovery .getBaseUrl('techdocs') .then(techdocsApiUrl => { diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts index 87de9d1d02..54ea860fb9 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts @@ -17,11 +17,11 @@ import { BuildMetadataStorage } from './BuildMetadataStorage'; describe('BuildMetadataStorage', () => { it('should return build timestamp', () => { - const newMetadataStorage = new BuildMetadataStorage('123abc'); - newMetadataStorage.storeBuildTimestamp(); + const newMetadataStorage = new BuildMetadataStorage('entityID123abc'); + newMetadataStorage.setEtag('etag123abc'); - const timestamp = newMetadataStorage.getTimestamp(); + const timestamp = newMetadataStorage.getEtag(); - expect(timestamp).toBeLessThanOrEqual(Date.now()); + expect(timestamp).toBe('etag123abc'); }); }); diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts index d6a19764d9..a1bb918fae 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts @@ -14,10 +14,15 @@ * limitations under the License. */ type buildInfo = { - // uid: timestamp - [key: string]: number; + // 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; /** @@ -34,11 +39,11 @@ export class BuildMetadataStorage { this.builds = builds; } - storeBuildTimestamp() { - this.builds[this.entityUid] = Date.now(); + setEtag(etag: string): void { + this.builds[this.entityUid] = etag; } - getTimestamp() { + getEtag(): string | undefined { return this.builds[this.entityUid]; } } diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 33d3120c47..8c54a2b1e4 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -13,22 +13,22 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { NotModifiedError } from '@backstage/backend-common'; +import { Entity } from '@backstage/catalog-model'; +import { + GeneratorBase, + GeneratorBuilder, + getLocationForEntity, + PreparerBase, + PreparerBuilder, + PublisherBase, + UrlPreparer, +} from '@backstage/techdocs-common'; +import Docker from 'dockerode'; import fs from 'fs-extra'; import os from 'os'; import path from 'path'; -import Docker from 'dockerode'; import { Logger } from 'winston'; -import { Entity } from '@backstage/catalog-model'; -import { Config } from '@backstage/config'; -import { - PreparerBuilder, - PublisherBase, - GeneratorBuilder, - PreparerBase, - GeneratorBase, - getLocationForEntity, - getLastCommitTimestamp, -} from '@backstage/techdocs-common'; import { BuildMetadataStorage } from '.'; const getEntityId = (entity: Entity) => { @@ -44,7 +44,6 @@ type DocsBuilderArguments = { entity: Entity; logger: Logger; dockerClient: Docker; - config: Config; }; export class DocsBuilder { @@ -54,7 +53,6 @@ export class DocsBuilder { private entity: Entity; private logger: Logger; private dockerClient: Docker; - private config: Config; constructor({ preparers, @@ -63,7 +61,6 @@ export class DocsBuilder { entity, logger, dockerClient, - config, }: DocsBuilderArguments) { this.preparer = preparers.get(entity); this.generator = generators.get(entity); @@ -71,14 +68,53 @@ export class DocsBuilder { this.entity = entity; this.logger = logger; this.dockerClient = dockerClient; - this.config = config; } - public async build() { - this.logger.info(`Running preparer on entity ${getEntityId(this.entity)}`); - const preparedDir = await this.preparer.prepare(this.entity); + public async build(): Promise { + if (!this.entity.metadata.uid) { + throw new Error( + 'Trying to build documentation for entity not in service catalog', + ); + } - const parsedLocationAnnotation = getLocationForEntity(this.entity); + /** + * Prepare and cache check + */ + + // Use the in-memory storage for setting and getting etag for this entity. + const buildMetadataStorage = new BuildMetadataStorage( + this.entity.metadata.uid, + ); + + // 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; + try { + const preparerResponse = await this.preparer.prepare(this.entity, { + etag: buildMetadataStorage.getEtag(), + }); + + preparedDir = preparerResponse.preparedDir; + etag = preparerResponse.etag; + } catch (err) { + if (err instanceof NotModifiedError) { + // No need to prepare anymore since cache is valid. + return; + } + throw new Error(err.message); + } + + this.logger.info( + `TechDocs prepare step completed for entity ${getEntityId(this.entity)}.`, + ); + this.logger.debug(`Prepared files temporarily stored at ${preparedDir}`); + + /** + * Generate + */ this.logger.info(`Running generator on entity ${getEntityId(this.entity)}`); // Create a temporary directory to store the generated files in. @@ -88,7 +124,7 @@ export class DocsBuilder { const outputDir = await fs.mkdtemp( path.join(tmpdirResolvedPath, 'techdocs-tmp-'), ); - + const parsedLocationAnnotation = getLocationForEntity(this.entity); await this.generator.run({ inputDir: preparedDir, outputDir, @@ -96,69 +132,42 @@ export class DocsBuilder { parsedLocationAnnotation, }); + this.logger.debug(`Generated files temporarily stored at ${outputDir}`); + // Remove Prepared directory since it is no longer needed. + // Caveat: Can not remove prepared directory in case of git preparer since the + // local git repository is used to get etag on subsequent requests. + if (this.preparer instanceof UrlPreparer) { + this.logger.debug( + `Removing prepared directory ${preparedDir} since the site has been generated.`, + ); + try { + // Not a blocker hence no need to await this. + fs.remove(preparedDir); + } catch (error) { + this.logger.debug(`Error removing prepared directory ${error.message}`); + } + } + + /** + * Publish + */ + this.logger.info(`Running publisher on entity ${getEntityId(this.entity)}`); await this.publisher.publish({ entity: this.entity, directory: outputDir, }); - - // TODO: Remove the generated directory once published. - - if (!this.entity.metadata.uid) { - throw new Error( - 'Trying to build documentation for entity not in service catalog', - ); - } - - new BuildMetadataStorage(this.entity.metadata.uid).storeBuildTimestamp(); - } - - public async docsUpToDate() { - if (!this.entity.metadata.uid) { - throw new Error( - 'Trying to build documentation for entity not in service catalog', - ); - } - - const buildMetadataStorage = new BuildMetadataStorage( - this.entity.metadata.uid, - ); - const { type, target } = getLocationForEntity(this.entity); - - // Unless docs are stored locally - const nonAgeCheckTypes = ['dir', 'file', 'url']; - if (!nonAgeCheckTypes.includes(type)) { - const lastCommit = await getLastCommitTimestamp( - target, - this.config, - this.logger, - ); - const storageTimeStamp = buildMetadataStorage.getTimestamp(); - - // Check if documentation source is newer than what we have - if (storageTimeStamp && storageTimeStamp >= lastCommit) { - this.logger.debug( - `Docs for entity ${getEntityId(this.entity)} is up to date.`, - ); - return true; - } - } - - // Cache downloaded source files for 30 minutes. - // TODO: When urlReader/readTree supports some way to get latest commit timestamp, - // it should be used to invalidate cache. - if (type === 'url') { - const builtAt = buildMetadataStorage.getTimestamp(); - const now = Date.now(); - - if (builtAt > now - 1800000) { - return true; - } - } - this.logger.debug( - `Docs for entity ${getEntityId(this.entity)} was outdated.`, + `Removing generated directory ${outputDir} since the site has been published`, ); - return false; + try { + // Not a blocker hence no need to await this. + fs.remove(outputDir); + } catch (error) { + 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); } } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index fe7b52ab52..ef032e9f8b 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -13,23 +13,23 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Logger } from 'winston'; -import Router from 'express-promise-router'; -import express from 'express'; -import Knex from 'knex'; -import fetch from 'cross-fetch'; -import { Config } from '@backstage/config'; -import Docker from 'dockerode'; -import { - GeneratorBuilder, - PreparerBuilder, - PublisherBase, - getLocationForEntity, -} from '@backstage/techdocs-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; -import { getEntityNameFromUrlPath } from './helpers'; +import { Config } from '@backstage/config'; +import { + GeneratorBuilder, + getLocationForEntity, + PreparerBuilder, + PublisherBase, +} from '@backstage/techdocs-common'; +import fetch from 'cross-fetch'; +import Docker from 'dockerode'; +import express from 'express'; +import Router from 'express-promise-router'; +import Knex from 'knex'; +import { Logger } from 'winston'; import { DocsBuilder } from '../DocsBuilder'; +import { getEntityNameFromUrlPath } from './helpers'; type RouterOptions = { preparers: PreparerBuilder; @@ -141,13 +141,10 @@ export async function createRouter({ dockerClient, logger, entity, - config, }); switch (publisherType) { case 'local': - if (!(await docsBuilder.docsUpToDate())) { - await docsBuilder.build(); - } + await docsBuilder.build(); break; case 'awsS3': case 'azureBlobStorage': @@ -186,9 +183,11 @@ export async function createRouter({ 'Found pre-generated docs for this entity. Serving them.', ); // TODO: re-trigger build for cache invalidation. - // Compare the date modified of the requested file on storage and compare it against - // the last modified or last commit timestamp in the repository. + // 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. } break; default: