From d32d01e5bcfb73e4c7bd56ef662d48a37e0e9636 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Fri, 9 Jul 2021 09:54:42 +0200 Subject: [PATCH 1/4] Introduce the annotation `backstage.io/techdocs-ref: ` as an alias for `backstage.io/techdocs-ref: dir:` Signed-off-by: Dominik Henneke --- .changeset/techdocs-twenty-donuts-eat.md | 89 +++++++ packages/techdocs-common/api-report.md | 39 ++- packages/techdocs-common/src/helpers.test.ts | 239 +++++++++++++++++- packages/techdocs-common/src/helpers.ts | 128 +++++++++- .../src/stages/prepare/dir.test.ts | 18 +- .../techdocs-common/src/stages/prepare/dir.ts | 93 ++----- .../src/stages/prepare/preparers.ts | 30 ++- plugins/techdocs-backend/package.json | 1 + .../src/DocsBuilder/builder.ts | 10 +- .../src/service/DocsSynchronizer.test.ts | 2 + .../src/service/DocsSynchronizer.ts | 6 + .../techdocs-backend/src/service/router.ts | 11 +- 12 files changed, 556 insertions(+), 110 deletions(-) create mode 100644 .changeset/techdocs-twenty-donuts-eat.md diff --git a/.changeset/techdocs-twenty-donuts-eat.md b/.changeset/techdocs-twenty-donuts-eat.md new file mode 100644 index 0000000000..b4f9a067c3 --- /dev/null +++ b/.changeset/techdocs-twenty-donuts-eat.md @@ -0,0 +1,89 @@ +--- +'@backstage/techdocs-common': minor +'@backstage/plugin-techdocs-backend': minor +--- + +Introduce the annotation `backstage.io/techdocs-ref: ` as an alias for `backstage.io/techdocs-ref: dir:`. +This annotation works with both the basic and the recommended flow, however, it will be most useful with the basic approach. + +In addition, this change removes the support of the deprecated `github`, `gitlab`, and `azure/api` locations from the `dir` reference preparer. + +#### Example Usage + +The new annotation is convenient if the documentation is stored in the same location, i.e. the same git repository, as the `catalog-info.yaml`. +While it is still supported to add full URLs such as `backstage.io/techdocs-ref: url:https://...` for custom setups, documentation is mostly stored in the same repository as the entity definition. +By automatically resolving the target relative to the registration location of the entity, the configuration overhead for this default setup is minimized. +Since it leverages the `@backstage/integrations` package for the URL resolution, this is compatible with every supported source. + +Consider the following examples: + +> Note that the short version `` is only an alias for the still supported `dir:`. + +1. "I have a repository with a single `catalog-info.yaml` and a TechDocs page in the root folder!" + +``` +https://github.com/backstage/example/tree/main/ + |- catalog-info.yaml + | > apiVersion: backstage.io/v1alpha1 + | > kind: Component + | > metadata: + | > name: example + | > annotations: + | > backstage.io/techdocs-ref: . # -> same folder + | > spec: {} + |- docs/ + |- mkdocs.yml +``` + +2. "I have a repository with a single `catalog-info.yaml` and my TechDocs page in located in a folder!" + +``` +https://bitbucket.org/my-owner/my-project/src/master/ + |- catalog-info.yaml + | > apiVersion: backstage.io/v1alpha1 + | > kind: Component + | > metadata: + | > name: example + | > annotations: + | > backstage.io/techdocs-ref: ./some-folder # -> subfolder + | > spec: {} + |- some-folder/ + |- docs/ + |- mkdocs.yml +``` + +3. "I have a mono repository that hosts multiple components!" + +``` +https://dev.azure.com/organization/project/_git/repository + |- my-1st-module/ + |- catalog-info.yaml + | > apiVersion: backstage.io/v1alpha1 + | > kind: Component + | > metadata: + | > name: my-1st-module + | > annotations: + | > backstage.io/techdocs-ref: . # -> same folder + | > spec: {} + |- docs/ + |- mkdocs.yml + |- my-2nd-module/ + |- catalog-info.yaml + | > apiVersion: backstage.io/v1alpha1 + | > kind: Component + | > metadata: + | > name: my-2nd-module + | > annotations: + | > backstage.io/techdocs-ref: . # -> same folder + | > spec: {} + |- docs/ + |- mkdocs.yml + |- catalog-info.yaml + | > apiVersion: backstage.io/v1alpha1 + | > kind: Location + | > metadata: + | > name: example + | > spec: + | > targets: + | > - ./*/catalog-info.yaml +``` diff --git a/packages/techdocs-common/api-report.md b/packages/techdocs-common/api-report.md index 7c44dc5484..9fc1795c13 100644 --- a/packages/techdocs-common/api-report.md +++ b/packages/techdocs-common/api-report.md @@ -15,6 +15,7 @@ import { GitHubIntegrationConfig } from '@backstage/integration'; import { GitLabIntegrationConfig } from '@backstage/integration'; import { Logger as Logger_2 } from 'winston'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import { UrlReader } from '@backstage/backend-common'; import { Writable } from 'stream'; @@ -47,9 +48,15 @@ export class CommonGitPreparer implements PreparerBase { // // @public (undocumented) export class DirectoryPreparer implements PreparerBase { - constructor(config: Config, logger: Logger_2, reader: UrlReader); + constructor(config: Config, _logger: Logger_2, reader: UrlReader); // (undocumented) - prepare(entity: Entity): Promise; + prepare( + entity: Entity, + options?: { + logger?: Logger_2; + etag?: string; + }, + ): Promise; } // Warning: (ae-missing-release-tag) "GeneratorBase" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -130,6 +137,18 @@ export const getDefaultBranch: ( config: Config, ) => Promise; +// Warning: (ae-missing-release-tag) "getDirLocation" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public +export const getDirLocation: ( + entity: Entity, +) => + | { + type: 'dir'; + target: string; + } + | undefined; + // Warning: (ae-missing-release-tag) "getDocFilesFromRepository" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -189,7 +208,10 @@ export const getLastCommitTimestamp: ( // Warning: (ae-missing-release-tag) "getLocationForEntity" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) -export const getLocationForEntity: (entity: Entity) => ParsedLocationAnnotation; +export const getLocationForEntity: ( + entity: Entity, + scmIntegration: ScmIntegrationRegistry, +) => ParsedLocationAnnotation; // Warning: (ae-missing-release-tag) "getTokenForGitRepo" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -355,6 +377,17 @@ export type TechDocsMetadata = { etag: string; }; +// Warning: (ae-missing-release-tag) "transformDirLocation" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public +export const transformDirLocation: ( + entity: Entity, + scmIntegrations: ScmIntegrationRegistry, +) => { + type: 'dir' | 'url'; + target: string; +}; + // Warning: (ae-missing-release-tag) "UrlPreparer" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index b94cbba572..6b677c6f5e 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -19,14 +19,27 @@ import { SearchResponse, UrlReader, } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, getEntitySourceLocation } from '@backstage/catalog-model'; +import { ConfigReader } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; +import os from 'os'; +import path from 'path'; import { Readable } from 'stream'; import { + getDirLocation, getDocFilesFromRepository, getLocationForEntity, parseReferenceAnnotation, + transformDirLocation, } from './helpers'; +jest.mock('@backstage/catalog-model', () => ({ + ...jest.requireActual('@backstage/catalog-model'), + getEntitySourceLocation: jest.fn(), +})); + +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; + const entityBase: Entity = { metadata: { namespace: 'default', @@ -81,6 +94,10 @@ const mockEntityWithBadAnnotation: Entity = { }, }; +const scmIntegrations = ScmIntegrations.fromConfig(new ConfigReader({})); + +afterEach(() => jest.resetAllMocks()); + describe('parseReferenceAnnotation', () => { it('should parse annotation', () => { const parsedLocationAnnotation = parseReferenceAnnotation( @@ -109,10 +126,230 @@ describe('parseReferenceAnnotation', () => { }); }); +describe('getDirLocation', () => { + it.each` + techdocsRef | responseTarget + ${undefined} | ${undefined} + ${16} | ${undefined} + ${'.'} | ${'.'} + ${'dir:.'} | ${'.'} + ${'./relative'} | ${'./relative'} + ${'dir:./relative'} | ${'./relative'} + ${'dir:https://github.com...'} | ${'https://github.com...'} + ${'url:https://github.com...'} | ${undefined} + `( + 'should handle "backstage.io/techdocs-ref: $techdocsRef" correctly', + ({ techdocsRef, responseTarget }) => { + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': techdocsRef, + }, + }, + }; + + const result = getDirLocation(entity); + + expect(result).toEqual( + responseTarget && { type: 'dir', target: responseTarget }, + ); + }, + ); + + it('Reject https urls and hint to the url: location type', async () => { + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': 'https://github.com/backstage/backstage', + }, + }, + }; + + expect(() => getDirLocation(entity)).toThrow( + /please prefix it with 'url:'/, + ); + }); +}); + +describe('transformDirLocation', () => { + it('should reject missing annotation', () => { + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + }, + }; + + expect(() => transformDirLocation(entity, scmIntegrations)).toThrow( + /No techdocs location annotation provided in entity: component:default\/test/, + ); + }); + + it.each` + techdocsRef | target + ${'.'} | ${'https://my-url/folder/'} + ${'./sub-folder'} | ${'https://my-url/folder/sub-folder'} + `( + 'should transform "$techdocsRef" for url type locations', + ({ techdocsRef, target }) => { + (getEntitySourceLocation as jest.Mock).mockReturnValue({ + type: 'url', + target: 'https://my-url/folder/', + }); + + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': techdocsRef, + }, + }, + }; + + const result = transformDirLocation(entity, scmIntegrations); + + expect(result).toEqual({ type: 'url', target }); + }, + ); + + it.each` + techdocsRef | target + ${'.'} | ${path.join(rootDir, 'working-copy')} + ${'./sub-folder'} | ${path.join(rootDir, 'working-copy', 'sub-folder')} + `( + 'should transform "$techdocsRef" for file type locations', + ({ techdocsRef, target }) => { + (getEntitySourceLocation as jest.Mock).mockReturnValue({ + type: 'file', + target: path.join(rootDir, 'working-copy', 'catalog-info.yaml'), + }); + + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': techdocsRef, + }, + }, + }; + + const result = transformDirLocation(entity, scmIntegrations); + + expect(result).toEqual({ type: 'dir', target }); + }, + ); + + it('should reject unsafe file location', () => { + (getEntitySourceLocation as jest.Mock).mockReturnValue({ + type: 'file', + target: '/tmp/catalog-info.yaml', + }); + + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': '..', + }, + }, + }; + + expect(() => transformDirLocation(entity, scmIntegrations)).toThrow( + /Relative path is not allowed to refer to a directory outside its parent/, + ); + }); + + it('should reject other location types', () => { + (getEntitySourceLocation as jest.Mock).mockReturnValue({ + type: 'other', + target: '/', + }); + + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': '.', + }, + }, + }; + + expect(() => transformDirLocation(entity, scmIntegrations)).toThrow( + /Unable to resolve location type other/, + ); + }); +}); + describe('getLocationForEntity', () => { + it('should handle implicit dir locations', () => { + (getEntitySourceLocation as jest.Mock).mockReturnValue({ + type: 'url', + target: 'https://my-url/folder/', + }); + + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': '.', + }, + }, + }; + + const parsedLocationAnnotation = getLocationForEntity( + entity, + scmIntegrations, + ); + expect(parsedLocationAnnotation.type).toBe('url'); + expect(parsedLocationAnnotation.target).toBe('https://my-url/folder/'); + }); + + it('should handle dir locations', () => { + (getEntitySourceLocation as jest.Mock).mockReturnValue({ + type: 'url', + target: 'https://my-url/folder/', + }); + + const entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/techdocs-ref': 'dir:.', + }, + }, + }; + + const parsedLocationAnnotation = getLocationForEntity( + entity, + scmIntegrations, + ); + expect(parsedLocationAnnotation.type).toBe('url'); + expect(parsedLocationAnnotation.target).toBe('https://my-url/folder/'); + }); + it('should get location for entity', () => { const parsedLocationAnnotation = getLocationForEntity( mockEntityWithAnnotation, + scmIntegrations, ); expect(parsedLocationAnnotation.type).toBe('url'); expect(parsedLocationAnnotation.target).toBe( diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts index cac8a7047f..bb640c1cce 100644 --- a/packages/techdocs-common/src/helpers.ts +++ b/packages/techdocs-common/src/helpers.ts @@ -14,10 +14,20 @@ * limitations under the License. */ -import { Git, UrlReader } from '@backstage/backend-common'; -import { InputError } from '@backstage/errors'; -import { Entity, parseLocationReference } from '@backstage/catalog-model'; +import { + Git, + resolveSafeChildPath, + UrlReader, +} from '@backstage/backend-common'; +import { + Entity, + getEntitySourceLocation, + parseLocationReference, + stringifyEntityRef, +} from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +import { InputError } from '@backstage/errors'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import fs from 'fs-extra'; import parseGitUrl from 'git-url-parse'; import os from 'os'; @@ -50,9 +60,113 @@ export const parseReferenceAnnotation = ( }; }; +/** + * Check if the entity provides a `backstage.io/techdocs-ref` annotation of type `dir` + * and return the annotation. It accepts the two variants `` and `dir:`. + * + * @param entity - the entity to check + */ +export const getDirLocation = ( + entity: Entity, +): { type: 'dir'; target: string } | undefined => { + const annotation = entity.metadata.annotations?.['backstage.io/techdocs-ref']; + + if (!annotation) { + return undefined; + } + + if (typeof annotation !== 'string') { + return undefined; + } + + // if the string doesn't contain `:`, interpret it as the target of a dir type + if (!annotation.includes(':')) { + return { type: 'dir', target: annotation }; + } + + // note that `backstage.io/techdocs-ref: https://...` is invalid and will throw + const reference = parseLocationReference(annotation); + + if (reference.type === 'dir') { + return { + type: 'dir', + target: reference.target, + }; + } + + // ignore any other types + return undefined; +}; + +/** + * TechDocs references of type `dir` are relative the source location of the entity. + * This function transforms relative references to absolute ones, based on the + * location the entity was ingested from. If the entity was registered by a `url` + * location, it returns a `url` location with a resolved target that points to the + * targeted subfolder. If the entity was registered by a `file` location, it returns + * an absolute `dir` location. + * + * @param entity - the entity with annotations + * @param scmIntegrations - access to the scmIntegrationt to do url transformations + * @throws if the entity doesn't specify a `dir` location or is ingested from an unsupported location. + * @returns the transformed location with an absolute target. + */ +export const transformDirLocation = ( + entity: Entity, + scmIntegrations: ScmIntegrationRegistry, +): { type: 'dir' | 'url'; target: string } => { + const dirLocation = getDirLocation(entity); + + if (!dirLocation) { + throw new InputError( + `No techdocs location annotation provided in entity: ${stringifyEntityRef( + entity, + )}`, + ); + } + + const location = getEntitySourceLocation(entity); + + switch (location.type) { + case 'url': { + const target = scmIntegrations.resolveUrl({ + url: dirLocation.target, + base: location.target, + }); + + return { + type: 'url', + target, + }; + } + + case 'file': { + // only permit targets in the same folder as the target of the `file` location! + const target = resolveSafeChildPath( + path.dirname(location.target), + dirLocation.target, + ); + + return { + type: 'dir', + target, + }; + } + + default: + throw new InputError(`Unable to resolve location type ${location.type}`); + } +}; + export const getLocationForEntity = ( entity: Entity, + scmIntegration: ScmIntegrationRegistry, ): ParsedLocationAnnotation => { + // try to resolve relative references first + if (getDirLocation(entity) !== undefined) { + return transformDirLocation(entity, scmIntegration); + } + const { type, target } = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, @@ -64,14 +178,6 @@ export const getLocationForEntity = ( case 'azure/api': case 'url': return { type, target }; - case 'dir': - if (path.isAbsolute(target)) { - return { type, target }; - } - return parseReferenceAnnotation( - 'backstage.io/managed-by-location', - entity, - ); default: throw new Error(`Invalid reference annotation ${type}`); } diff --git a/packages/techdocs-common/src/stages/prepare/dir.test.ts b/packages/techdocs-common/src/stages/prepare/dir.test.ts index 6992a35c65..019bf51fe9 100644 --- a/packages/techdocs-common/src/stages/prepare/dir.test.ts +++ b/packages/techdocs-common/src/stages/prepare/dir.test.ts @@ -15,7 +15,6 @@ */ import { getVoidLogger, UrlReader } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; -import { checkoutGitRepository } from '../../helpers'; import { DirectoryPreparer } from './dir'; function normalizePath(path: string) { @@ -27,8 +26,6 @@ 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(); @@ -71,7 +68,7 @@ describe('directory preparer', () => { expect(normalizePath(preparedDir)).toEqual('/directory/our-documentation'); }); - it('should merge managed-by-location and techdocs-ref when techdocs-ref is absolute', async () => { + it('should reject when techdocs-ref is absolute', async () => { const directoryPreparer = new DirectoryPreparer( mockConfig, logger, @@ -84,11 +81,12 @@ describe('directory preparer', () => { 'backstage.io/techdocs-ref': 'dir:/our-documentation/techdocs', }); - const { preparedDir } = await directoryPreparer.prepare(mockEntity); - expect(normalizePath(preparedDir)).toEqual('/our-documentation/techdocs'); + await expect(directoryPreparer.prepare(mockEntity)).rejects.toThrow( + /Relative path is not allowed to refer to a directory outside its parent/, + ); }); - it('should merge managed-by-location and techdocs-ref when managed-by-location is a git repository', async () => { + it('should reject when managed-by-location is a git repository', async () => { const directoryPreparer = new DirectoryPreparer( mockConfig, logger, @@ -101,10 +99,8 @@ describe('directory preparer', () => { 'backstage.io/techdocs-ref': 'dir:./docs', }); - const { preparedDir } = await directoryPreparer.prepare(mockEntity); - expect(normalizePath(preparedDir)).toEqual( - '/tmp/backstage-repo/org/name/branch/docs', + await expect(directoryPreparer.prepare(mockEntity)).rejects.toThrow( + /Unable to resolve location type github/, ); - 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 9ec4b4d37a..b0ba2a36b3 100644 --- a/packages/techdocs-common/src/stages/prepare/dir.ts +++ b/packages/techdocs-common/src/stages/prepare/dir.ts @@ -15,104 +15,57 @@ */ import { UrlReader } from '@backstage/backend-common'; -import { InputError, NotModifiedError } from '@backstage/errors'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import parseGitUrl from 'git-url-parse'; -import path from 'path'; -import { Logger } from 'winston'; +import { InputError } from '@backstage/errors'; import { - checkoutGitRepository, - getLastCommitTimestamp, - parseReferenceAnnotation, -} from '../../helpers'; + ScmIntegrationRegistry, + ScmIntegrations, +} from '@backstage/integration'; +import { Logger } from 'winston'; +import { transformDirLocation } from '../../helpers'; import { PreparerBase, PreparerResponse } from './types'; export class DirectoryPreparer implements PreparerBase { - constructor( - private readonly config: Config, - private readonly logger: Logger, - private readonly reader: UrlReader, - ) { - this.config = config; - this.logger = logger; + private readonly scmIntegrations: ScmIntegrationRegistry; + private readonly reader: UrlReader; + + constructor(config: Config, _logger: Logger, reader: UrlReader) { this.reader = reader; + this.scmIntegrations = ScmIntegrations.fromConfig(config); } - private async resolveManagedByLocationToDir( + async prepare( entity: Entity, - options?: { etag?: string }, + options?: { logger?: Logger; etag?: string }, ): Promise { - const { type, target } = parseReferenceAnnotation( - 'backstage.io/managed-by-location', - entity, - ); + const { type, target } = transformDirLocation(entity, this.scmIntegrations); - this.logger.debug( - `Building docs for entity with type 'dir' and managed-by-location '${type}'`, - ); switch (type) { case 'url': { + options?.logger?.info(`Download documentation from ${target}`); + // the target is an absolute url since it has already been transformed const response = await this.reader.readTree(target, { etag: options?.etag, }); - const preparedDir = await response.dir(); + return { - preparedDir, + preparedDir: await response.dir(), etag: response.etag, }; } - case 'github': - case 'gitlab': - case 'azure/api': { - const parsedGitLocation = parseGitUrl(target); - const repoLocation = await checkoutGitRepository( - target, - this.config, - this.logger, - ); - // Check if etag has changed for cache invalidation. - const etag = await getLastCommitTimestamp(repoLocation, this.logger); - if (options?.etag === etag.toString()) { - throw new NotModifiedError(); - } + case 'dir': { return { - preparedDir: path.dirname( - path.join(repoLocation, parsedGitLocation.filepath), - ), - etag: etag.toString(), - }; - } - case 'file': - return { - preparedDir: path.dirname(target), + // the transformation already validated that the target is in a safe location + preparedDir: 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 { - 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, - ); - - // This will throw NotModified error if etag has not changed. - const response = await this.resolveManagedByLocationToDir(entity); - - return { - preparedDir: path.resolve(response.preparedDir, target), - etag: response.etag, - }; - } } diff --git a/packages/techdocs-common/src/stages/prepare/preparers.ts b/packages/techdocs-common/src/stages/prepare/preparers.ts index 5c78d96f15..48e99d48f5 100644 --- a/packages/techdocs-common/src/stages/prepare/preparers.ts +++ b/packages/techdocs-common/src/stages/prepare/preparers.ts @@ -13,15 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { UrlReader } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, parseLocationReference } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +import { InputError } from '@backstage/errors'; import { Logger } from 'winston'; -import { parseReferenceAnnotation } from '../../helpers'; -import { DirectoryPreparer } from './dir'; +import { getDirLocation } from '../../helpers'; import { CommonGitPreparer } from './commonGit'; -import { UrlPreparer } from './url'; +import { DirectoryPreparer } from './dir'; import { PreparerBase, PreparerBuilder, RemoteProtocol } from './types'; +import { UrlPreparer } from './url'; type factoryOptions = { logger: Logger; @@ -61,11 +63,21 @@ export class Preparers implements PreparerBuilder { } get(entity: Entity): PreparerBase { - const { type } = parseReferenceAnnotation( - 'backstage.io/techdocs-ref', - entity, - ); - const preparer = this.preparerMap.get(type); + const annotation = + entity.metadata.annotations?.['backstage.io/techdocs-ref']; + if (!annotation) { + throw new InputError( + `No location annotation provided in entity: ${entity.metadata.name}`, + ); + } + + // the dir processor handles both `` and `dir:` + if (getDirLocation(entity) !== undefined) { + return this.preparerMap.get('dir')!; + } + + const { type } = parseLocationReference(annotation); + const preparer = this.preparerMap.get(type as RemoteProtocol); if (!preparer) { throw new Error(`No preparer registered for type: "${type}"`); diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 76aadc79cf..4ae17d307c 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -35,6 +35,7 @@ "@backstage/catalog-model": "^0.9.0", "@backstage/config": "^0.1.5", "@backstage/errors": "^0.1.1", + "@backstage/integration": "^0.5.8", "@backstage/techdocs-common": "^0.6.8", "@types/express": "^4.17.6", "cross-fetch": "^3.0.6", diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 2cd6f57154..5327062233 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -20,6 +20,7 @@ import { } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { NotModifiedError } from '@backstage/errors'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import { GeneratorBase, GeneratorBuilder, @@ -43,6 +44,7 @@ type DocsBuilderArguments = { entity: Entity; logger: Logger; config: Config; + scmIntegrations: ScmIntegrationRegistry; logStream?: Writable; }; @@ -53,6 +55,7 @@ export class DocsBuilder { private entity: Entity; private logger: Logger; private config: Config; + private scmIntegrations: ScmIntegrationRegistry; private logStream: Writable | undefined; constructor({ @@ -62,6 +65,7 @@ export class DocsBuilder { entity, logger, config, + scmIntegrations, logStream, }: DocsBuilderArguments) { this.preparer = preparers.get(entity); @@ -70,6 +74,7 @@ export class DocsBuilder { this.entity = entity; this.logger = logger; this.config = config; + this.scmIntegrations = scmIntegrations; this.logStream = logStream; } @@ -166,7 +171,10 @@ export class DocsBuilder { path.join(tmpdirResolvedPath, 'techdocs-tmp-'), ); - const parsedLocationAnnotation = getLocationForEntity(this.entity); + const parsedLocationAnnotation = getLocationForEntity( + this.entity, + this.scmIntegrations, + ); await this.generator.run({ inputDir: preparedDir, outputDir, diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts index 409dc6e128..ac60f3da3a 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts @@ -19,6 +19,7 @@ import { PluginEndpointDiscovery, } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; import { GeneratorBuilder, PreparerBuilder, @@ -69,6 +70,7 @@ describe('DocsSynchronizer', () => { publisher, config: new ConfigReader({}), logger: getVoidLogger(), + scmIntegrations: ScmIntegrations.fromConfig(new ConfigReader({})), }); }); diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts index 9b51381748..5e5d91b625 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -17,6 +17,7 @@ import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { NotFoundError } from '@backstage/errors'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import { GeneratorBuilder, PreparerBuilder, @@ -36,19 +37,23 @@ export class DocsSynchronizer { private readonly publisher: PublisherBase; private readonly logger: winston.Logger; private readonly config: Config; + private readonly scmIntegrations: ScmIntegrationRegistry; constructor({ publisher, logger, config, + scmIntegrations, }: { publisher: PublisherBase; logger: winston.Logger; config: Config; + scmIntegrations: ScmIntegrationRegistry; }) { this.config = config; this.logger = logger; this.publisher = publisher; + this.scmIntegrations = scmIntegrations; } async doSync({ @@ -94,6 +99,7 @@ export class DocsSynchronizer { logger: taskLogger, entity, config: this.config, + scmIntegrations: this.scmIntegrations, logStream, }); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index b889312abd..e560a895b6 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -29,6 +29,7 @@ import express, { Response } from 'express'; import Router from 'express-promise-router'; import { Knex } from 'knex'; import { Logger } from 'winston'; +import { ScmIntegrations } from '@backstage/integration'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; /** @@ -79,10 +80,12 @@ export async function createRouter( const router = Router(); const { publisher, config, logger, discovery } = options; const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const scmIntegrations = ScmIntegrations.fromConfig(config); const docsSynchronizer = new DocsSynchronizer({ - publisher: publisher, - logger: logger, - config: config, + publisher, + logger, + config, + scmIntegrations, }); router.get('/metadata/techdocs/:namespace/:kind/:name', async (req, res) => { @@ -126,7 +129,7 @@ export async function createRouter( ) ).json()) as Entity; - const locationMetadata = getLocationForEntity(entity); + const locationMetadata = getLocationForEntity(entity, scmIntegrations); res.json({ ...entity, locationMetadata }); } catch (err) { logger.info( From ed6092f76a606d0d367788c96f4fe6eee8500110 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 21 Jul 2021 12:42:24 +0200 Subject: [PATCH 2/4] Handle and forward errors from the DocsBuilder instantiation to the user Signed-off-by: Dominik Henneke --- .../src/service/DocsSynchronizer.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts index 5e5d91b625..463b895564 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -92,20 +92,20 @@ export class DocsSynchronizer { return; } - const docsBuilder = new DocsBuilder({ - preparers, - generators, - publisher: this.publisher, - logger: taskLogger, - entity, - config: this.config, - scmIntegrations: this.scmIntegrations, - logStream, - }); - let foundDocs = false; try { + const docsBuilder = new DocsBuilder({ + preparers, + generators, + publisher: this.publisher, + logger: taskLogger, + entity, + config: this.config, + scmIntegrations: this.scmIntegrations, + logStream, + }); + const updated = await docsBuilder.build(); if (!updated) { From d591b764d0f9e2de01b6caadccd046a93d901b7e Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 21 Jul 2021 15:13:16 +0200 Subject: [PATCH 3/4] Remove the short-form annotation and only support the full `dir:.` option Signed-off-by: Dominik Henneke --- .changeset/techdocs-twenty-donuts-eat.md | 17 +-- packages/techdocs-common/api-report.md | 13 +- packages/techdocs-common/src/helpers.test.ts | 137 ++++-------------- packages/techdocs-common/src/helpers.ts | 72 ++------- .../techdocs-common/src/stages/prepare/dir.ts | 19 ++- .../src/stages/prepare/preparers.ts | 23 +-- 6 files changed, 73 insertions(+), 208 deletions(-) diff --git a/.changeset/techdocs-twenty-donuts-eat.md b/.changeset/techdocs-twenty-donuts-eat.md index b4f9a067c3..a95ec5a752 100644 --- a/.changeset/techdocs-twenty-donuts-eat.md +++ b/.changeset/techdocs-twenty-donuts-eat.md @@ -3,22 +3,21 @@ '@backstage/plugin-techdocs-backend': minor --- -Introduce the annotation `backstage.io/techdocs-ref: ` as an alias for `backstage.io/techdocs-ref: dir:`. -This annotation works with both the basic and the recommended flow, however, it will be most useful with the basic approach. +Improve the annotation `backstage.io/techdocs-ref: dir:` that links to a path that is relative to the source of the annotated entity. +This annotation works with the basic and the recommended flow, however, it will be most useful with the basic approach. +This change remove the deprecation of the `dir` reference and provides first-class support for it. In addition, this change removes the support of the deprecated `github`, `gitlab`, and `azure/api` locations from the `dir` reference preparer. #### Example Usage -The new annotation is convenient if the documentation is stored in the same location, i.e. the same git repository, as the `catalog-info.yaml`. +The annotation is convenient if the documentation is stored in the same location, i.e. the same git repository, as the `catalog-info.yaml`. While it is still supported to add full URLs such as `backstage.io/techdocs-ref: url:https://...` for custom setups, documentation is mostly stored in the same repository as the entity definition. By automatically resolving the target relative to the registration location of the entity, the configuration overhead for this default setup is minimized. Since it leverages the `@backstage/integrations` package for the URL resolution, this is compatible with every supported source. Consider the following examples: -> Note that the short version `` is only an alias for the still supported `dir:`. - 1. "I have a repository with a single `catalog-info.yaml` and a TechDocs page in the root folder!" ``` @@ -29,7 +28,7 @@ https://github.com/backstage/example/tree/main/ | > metadata: | > name: example | > annotations: - | > backstage.io/techdocs-ref: . # -> same folder + | > backstage.io/techdocs-ref: dir:. # -> same folder | > spec: {} |- docs/ |- mkdocs.yml @@ -45,7 +44,7 @@ https://bitbucket.org/my-owner/my-project/src/master/ | > metadata: | > name: example | > annotations: - | > backstage.io/techdocs-ref: ./some-folder # -> subfolder + | > backstage.io/techdocs-ref: dir:./some-folder # -> subfolder | > spec: {} |- some-folder/ |- docs/ @@ -63,7 +62,7 @@ https://dev.azure.com/organization/project/_git/repository | > metadata: | > name: my-1st-module | > annotations: - | > backstage.io/techdocs-ref: . # -> same folder + | > backstage.io/techdocs-ref: dir:. # -> same folder | > spec: {} |- docs/ |- mkdocs.yml @@ -74,7 +73,7 @@ https://dev.azure.com/organization/project/_git/repository | > metadata: | > name: my-2nd-module | > annotations: - | > backstage.io/techdocs-ref: . # -> same folder + | > backstage.io/techdocs-ref: dir:. # -> same folder | > spec: {} |- docs/ |- mkdocs.yml diff --git a/packages/techdocs-common/api-report.md b/packages/techdocs-common/api-report.md index 9fc1795c13..6e845c9797 100644 --- a/packages/techdocs-common/api-report.md +++ b/packages/techdocs-common/api-report.md @@ -137,18 +137,6 @@ export const getDefaultBranch: ( config: Config, ) => Promise; -// Warning: (ae-missing-release-tag) "getDirLocation" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public -export const getDirLocation: ( - entity: Entity, -) => - | { - type: 'dir'; - target: string; - } - | undefined; - // Warning: (ae-missing-release-tag) "getDocFilesFromRepository" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -382,6 +370,7 @@ export type TechDocsMetadata = { // @public export const transformDirLocation: ( entity: Entity, + dirAnnotation: ParsedLocationAnnotation, scmIntegrations: ScmIntegrationRegistry, ) => { type: 'dir' | 'url'; diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index 6b677c6f5e..69f09d92ed 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -26,7 +26,6 @@ import os from 'os'; import path from 'path'; import { Readable } from 'stream'; import { - getDirLocation, getDocFilesFromRepository, getLocationForEntity, parseReferenceAnnotation, @@ -126,76 +125,11 @@ describe('parseReferenceAnnotation', () => { }); }); -describe('getDirLocation', () => { - it.each` - techdocsRef | responseTarget - ${undefined} | ${undefined} - ${16} | ${undefined} - ${'.'} | ${'.'} - ${'dir:.'} | ${'.'} - ${'./relative'} | ${'./relative'} - ${'dir:./relative'} | ${'./relative'} - ${'dir:https://github.com...'} | ${'https://github.com...'} - ${'url:https://github.com...'} | ${undefined} - `( - 'should handle "backstage.io/techdocs-ref: $techdocsRef" correctly', - ({ techdocsRef, responseTarget }) => { - const entity = { - apiVersion: '1', - kind: 'Component', - metadata: { - name: 'test', - annotations: { - 'backstage.io/techdocs-ref': techdocsRef, - }, - }, - }; - - const result = getDirLocation(entity); - - expect(result).toEqual( - responseTarget && { type: 'dir', target: responseTarget }, - ); - }, - ); - - it('Reject https urls and hint to the url: location type', async () => { - const entity = { - apiVersion: '1', - kind: 'Component', - metadata: { - name: 'test', - annotations: { - 'backstage.io/techdocs-ref': 'https://github.com/backstage/backstage', - }, - }, - }; - - expect(() => getDirLocation(entity)).toThrow( - /please prefix it with 'url:'/, - ); - }); -}); - describe('transformDirLocation', () => { - it('should reject missing annotation', () => { - const entity = { - apiVersion: '1', - kind: 'Component', - metadata: { - name: 'test', - }, - }; - - expect(() => transformDirLocation(entity, scmIntegrations)).toThrow( - /No techdocs location annotation provided in entity: component:default\/test/, - ); - }); - it.each` - techdocsRef | target - ${'.'} | ${'https://my-url/folder/'} - ${'./sub-folder'} | ${'https://my-url/folder/sub-folder'} + techdocsRef | target + ${'dir:.'} | ${'https://my-url/folder/'} + ${'dir:./sub-folder'} | ${'https://my-url/folder/sub-folder'} `( 'should transform "$techdocsRef" for url type locations', ({ techdocsRef, target }) => { @@ -215,16 +149,20 @@ describe('transformDirLocation', () => { }, }; - const result = transformDirLocation(entity, scmIntegrations); + const result = transformDirLocation( + entity, + parseReferenceAnnotation('backstage.io/techdocs-ref', entity), + scmIntegrations, + ); expect(result).toEqual({ type: 'url', target }); }, ); it.each` - techdocsRef | target - ${'.'} | ${path.join(rootDir, 'working-copy')} - ${'./sub-folder'} | ${path.join(rootDir, 'working-copy', 'sub-folder')} + techdocsRef | target + ${'dir:.'} | ${path.join(rootDir, 'working-copy')} + ${'dir:./sub-folder'} | ${path.join(rootDir, 'working-copy', 'sub-folder')} `( 'should transform "$techdocsRef" for file type locations', ({ techdocsRef, target }) => { @@ -244,7 +182,11 @@ describe('transformDirLocation', () => { }, }; - const result = transformDirLocation(entity, scmIntegrations); + const result = transformDirLocation( + entity, + parseReferenceAnnotation('backstage.io/techdocs-ref', entity), + scmIntegrations, + ); expect(result).toEqual({ type: 'dir', target }); }, @@ -262,12 +204,18 @@ describe('transformDirLocation', () => { metadata: { name: 'test', annotations: { - 'backstage.io/techdocs-ref': '..', + 'backstage.io/techdocs-ref': 'dir:..', }, }, }; - expect(() => transformDirLocation(entity, scmIntegrations)).toThrow( + expect(() => + transformDirLocation( + entity, + parseReferenceAnnotation('backstage.io/techdocs-ref', entity), + scmIntegrations, + ), + ).toThrow( /Relative path is not allowed to refer to a directory outside its parent/, ); }); @@ -284,43 +232,22 @@ describe('transformDirLocation', () => { metadata: { name: 'test', annotations: { - 'backstage.io/techdocs-ref': '.', + 'backstage.io/techdocs-ref': 'dir:.', }, }, }; - expect(() => transformDirLocation(entity, scmIntegrations)).toThrow( - /Unable to resolve location type other/, - ); + expect(() => + transformDirLocation( + entity, + parseReferenceAnnotation('backstage.io/techdocs-ref', entity), + scmIntegrations, + ), + ).toThrow(/Unable to resolve location type other/); }); }); describe('getLocationForEntity', () => { - it('should handle implicit dir locations', () => { - (getEntitySourceLocation as jest.Mock).mockReturnValue({ - type: 'url', - target: 'https://my-url/folder/', - }); - - const entity = { - apiVersion: '1', - kind: 'Component', - metadata: { - name: 'test', - annotations: { - 'backstage.io/techdocs-ref': '.', - }, - }, - }; - - const parsedLocationAnnotation = getLocationForEntity( - entity, - scmIntegrations, - ); - expect(parsedLocationAnnotation.type).toBe('url'); - expect(parsedLocationAnnotation.target).toBe('https://my-url/folder/'); - }); - it('should handle dir locations', () => { (getEntitySourceLocation as jest.Mock).mockReturnValue({ type: 'url', diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts index bb640c1cce..bb6461a37c 100644 --- a/packages/techdocs-common/src/helpers.ts +++ b/packages/techdocs-common/src/helpers.ts @@ -23,7 +23,6 @@ import { Entity, getEntitySourceLocation, parseLocationReference, - stringifyEntityRef, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { InputError } from '@backstage/errors'; @@ -60,44 +59,6 @@ export const parseReferenceAnnotation = ( }; }; -/** - * Check if the entity provides a `backstage.io/techdocs-ref` annotation of type `dir` - * and return the annotation. It accepts the two variants `` and `dir:`. - * - * @param entity - the entity to check - */ -export const getDirLocation = ( - entity: Entity, -): { type: 'dir'; target: string } | undefined => { - const annotation = entity.metadata.annotations?.['backstage.io/techdocs-ref']; - - if (!annotation) { - return undefined; - } - - if (typeof annotation !== 'string') { - return undefined; - } - - // if the string doesn't contain `:`, interpret it as the target of a dir type - if (!annotation.includes(':')) { - return { type: 'dir', target: annotation }; - } - - // note that `backstage.io/techdocs-ref: https://...` is invalid and will throw - const reference = parseLocationReference(annotation); - - if (reference.type === 'dir') { - return { - type: 'dir', - target: reference.target, - }; - } - - // ignore any other types - return undefined; -}; - /** * TechDocs references of type `dir` are relative the source location of the entity. * This function transforms relative references to absolute ones, based on the @@ -107,30 +68,22 @@ export const getDirLocation = ( * an absolute `dir` location. * * @param entity - the entity with annotations - * @param scmIntegrations - access to the scmIntegrationt to do url transformations + * @param dirAnnotation - the parsed techdocs-ref annotation of type 'dir' + * @param scmIntegrations - access to the scmIntegration to do url transformations * @throws if the entity doesn't specify a `dir` location or is ingested from an unsupported location. * @returns the transformed location with an absolute target. */ export const transformDirLocation = ( entity: Entity, + dirAnnotation: ParsedLocationAnnotation, scmIntegrations: ScmIntegrationRegistry, ): { type: 'dir' | 'url'; target: string } => { - const dirLocation = getDirLocation(entity); - - if (!dirLocation) { - throw new InputError( - `No techdocs location annotation provided in entity: ${stringifyEntityRef( - entity, - )}`, - ); - } - const location = getEntitySourceLocation(entity); switch (location.type) { case 'url': { const target = scmIntegrations.resolveUrl({ - url: dirLocation.target, + url: dirAnnotation.target, base: location.target, }); @@ -144,7 +97,7 @@ export const transformDirLocation = ( // only permit targets in the same folder as the target of the `file` location! const target = resolveSafeChildPath( path.dirname(location.target), - dirLocation.target, + dirAnnotation.target, ); return { @@ -162,24 +115,21 @@ export const getLocationForEntity = ( entity: Entity, scmIntegration: ScmIntegrationRegistry, ): ParsedLocationAnnotation => { - // try to resolve relative references first - if (getDirLocation(entity) !== undefined) { - return transformDirLocation(entity, scmIntegration); - } - - const { type, target } = parseReferenceAnnotation( + const annotation = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, ); - switch (type) { + switch (annotation.type) { case 'github': case 'gitlab': case 'azure/api': case 'url': - return { type, target }; + return annotation; + case 'dir': + return transformDirLocation(entity, annotation, scmIntegration); default: - throw new Error(`Invalid reference annotation ${type}`); + throw new Error(`Invalid reference annotation ${annotation.type}`); } }; diff --git a/packages/techdocs-common/src/stages/prepare/dir.ts b/packages/techdocs-common/src/stages/prepare/dir.ts index b0ba2a36b3..ade0b00162 100644 --- a/packages/techdocs-common/src/stages/prepare/dir.ts +++ b/packages/techdocs-common/src/stages/prepare/dir.ts @@ -23,7 +23,7 @@ import { ScmIntegrations, } from '@backstage/integration'; import { Logger } from 'winston'; -import { transformDirLocation } from '../../helpers'; +import { parseReferenceAnnotation, transformDirLocation } from '../../helpers'; import { PreparerBase, PreparerResponse } from './types'; export class DirectoryPreparer implements PreparerBase { @@ -39,18 +39,29 @@ export class DirectoryPreparer implements PreparerBase { entity: Entity, options?: { logger?: Logger; etag?: string }, ): Promise { - const { type, target } = transformDirLocation(entity, this.scmIntegrations); + const annotation = parseReferenceAnnotation( + 'backstage.io/techdocs-ref', + entity, + ); + const { type, target } = transformDirLocation( + entity, + annotation, + this.scmIntegrations, + ); switch (type) { case 'url': { - options?.logger?.info(`Download documentation from ${target}`); + options?.logger?.debug(`Reading files from ${target}`); // the target is an absolute url since it has already been transformed const response = await this.reader.readTree(target, { etag: options?.etag, }); + const preparedDir = await response.dir(); + + options?.logger?.debug(`Tree downloaded and stored at ${preparedDir}`); return { - preparedDir: await response.dir(), + preparedDir, etag: response.etag, }; } diff --git a/packages/techdocs-common/src/stages/prepare/preparers.ts b/packages/techdocs-common/src/stages/prepare/preparers.ts index 48e99d48f5..12fcb01013 100644 --- a/packages/techdocs-common/src/stages/prepare/preparers.ts +++ b/packages/techdocs-common/src/stages/prepare/preparers.ts @@ -15,11 +15,10 @@ */ import { UrlReader } from '@backstage/backend-common'; -import { Entity, parseLocationReference } from '@backstage/catalog-model'; +import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { InputError } from '@backstage/errors'; import { Logger } from 'winston'; -import { getDirLocation } from '../../helpers'; +import { parseReferenceAnnotation } from '../../helpers'; import { CommonGitPreparer } from './commonGit'; import { DirectoryPreparer } from './dir'; import { PreparerBase, PreparerBuilder, RemoteProtocol } from './types'; @@ -63,20 +62,10 @@ export class Preparers implements PreparerBuilder { } get(entity: Entity): PreparerBase { - const annotation = - entity.metadata.annotations?.['backstage.io/techdocs-ref']; - if (!annotation) { - throw new InputError( - `No location annotation provided in entity: ${entity.metadata.name}`, - ); - } - - // the dir processor handles both `` and `dir:` - if (getDirLocation(entity) !== undefined) { - return this.preparerMap.get('dir')!; - } - - const { type } = parseLocationReference(annotation); + const { type } = parseReferenceAnnotation( + 'backstage.io/techdocs-ref', + entity, + ); const preparer = this.preparerMap.get(type as RemoteProtocol); if (!preparer) { From 296d8e3832774ad92629f65dd208e00d24b157db Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 21 Jul 2021 15:18:22 +0200 Subject: [PATCH 4/4] Delete obsolete changes Signed-off-by: Dominik Henneke --- packages/techdocs-common/src/stages/prepare/preparers.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/techdocs-common/src/stages/prepare/preparers.ts b/packages/techdocs-common/src/stages/prepare/preparers.ts index 12fcb01013..5c78d96f15 100644 --- a/packages/techdocs-common/src/stages/prepare/preparers.ts +++ b/packages/techdocs-common/src/stages/prepare/preparers.ts @@ -13,16 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { Logger } from 'winston'; import { parseReferenceAnnotation } from '../../helpers'; -import { CommonGitPreparer } from './commonGit'; import { DirectoryPreparer } from './dir'; -import { PreparerBase, PreparerBuilder, RemoteProtocol } from './types'; +import { CommonGitPreparer } from './commonGit'; import { UrlPreparer } from './url'; +import { PreparerBase, PreparerBuilder, RemoteProtocol } from './types'; type factoryOptions = { logger: Logger; @@ -66,7 +65,7 @@ export class Preparers implements PreparerBuilder { 'backstage.io/techdocs-ref', entity, ); - const preparer = this.preparerMap.get(type as RemoteProtocol); + const preparer = this.preparerMap.get(type); if (!preparer) { throw new Error(`No preparer registered for type: "${type}"`);