diff --git a/.changeset/honest-steaks-beam.md b/.changeset/honest-steaks-beam.md new file mode 100644 index 0000000000..71c3193c99 --- /dev/null +++ b/.changeset/honest-steaks-beam.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-model': patch +--- + +Add parseLocationReference, stringifyLocationReference diff --git a/.changeset/little-crabs-burn.md b/.changeset/little-crabs-burn.md new file mode 100644 index 0000000000..2bc3fe9e96 --- /dev/null +++ b/.changeset/little-crabs-burn.md @@ -0,0 +1,9 @@ +--- +'@backstage/catalog-client': patch +'@backstage/plugin-catalog-backend': patch +'@backstage/plugin-catalog': patch +'@backstage/plugin-scaffolder-backend': patch +'@backstage/techdocs-common': patch +--- + +Make use of parseLocationReference/stringifyLocationReference diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 04206edcdb..a1af0240f6 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -19,15 +19,16 @@ import { EntityName, Location, LOCATION_ANNOTATION, + stringifyLocationReference, } from '@backstage/catalog-model'; import fetch from 'cross-fetch'; import { AddLocationRequest, AddLocationResponse, - CatalogRequestOptions, CatalogApi, CatalogEntitiesRequest, CatalogListResponse, + CatalogRequestOptions, DiscoveryApi, } from './types'; @@ -135,7 +136,7 @@ export class CatalogClient implements CatalogApi { ); return all .map(r => r.data) - .find(l => locationCompound === `${l.type}:${l.target}`); + .find(l => locationCompound === stringifyLocationReference(l)); } async removeEntityByUid( diff --git a/packages/catalog-model/src/location/helpers.test.ts b/packages/catalog-model/src/location/helpers.test.ts new file mode 100644 index 0000000000..888c79b328 --- /dev/null +++ b/packages/catalog-model/src/location/helpers.test.ts @@ -0,0 +1,70 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { parseLocationReference, stringifyLocationReference } from './helpers'; + +describe('parseLocationReference', () => { + it('works for the simple case', () => { + expect(parseLocationReference('url:https://www.google.com')).toEqual({ + type: 'url', + target: 'https://www.google.com', + }); + }); + + it('rejects faulty inputs', () => { + expect(() => parseLocationReference(7 as any)).toThrow( + "Unable to parse location reference '7', unexpected argument number", + ); + expect(() => parseLocationReference('')).toThrow( + "Unable to parse location reference '', expected ':', e.g. 'url:https://host/path'", + ); + expect(() => parseLocationReference('hello')).toThrow( + "Unable to parse location reference 'hello', expected ':', e.g. 'url:https://host/path'", + ); + expect(() => parseLocationReference(':hello')).toThrow( + "Unable to parse location reference ':hello', expected ':', e.g. 'url:https://host/path'", + ); + expect(() => parseLocationReference('hello:')).toThrow( + "Unable to parse location reference 'hello:', expected ':', e.g. 'url:https://host/path'", + ); + expect(() => parseLocationReference('http://blah')).toThrow( + "Invalid location reference 'http://blah', please prefix it with 'url:', e.g. 'url:http://blah'", + ); + expect(() => parseLocationReference('https://bleh')).toThrow( + "Invalid location reference 'https://bleh', please prefix it with 'url:', e.g. 'url:https://bleh'", + ); + }); +}); + +describe('stringifyLocationReference', () => { + it('works for the simple case', () => { + expect( + stringifyLocationReference({ + type: 'url', + target: 'https://www.google.com', + }), + ).toEqual('url:https://www.google.com'); + }); + + it('rejects faulty inputs', () => { + expect(() => + stringifyLocationReference({ type: '', target: 'hello' }), + ).toThrow('Unable to stringify location reference, empty type'); + expect(() => + stringifyLocationReference({ type: 'hello', target: '' }), + ).toThrow('Unable to stringify location reference, empty target'); + }); +}); diff --git a/packages/catalog-model/src/location/helpers.ts b/packages/catalog-model/src/location/helpers.ts new file mode 100644 index 0000000000..fb1be5abad --- /dev/null +++ b/packages/catalog-model/src/location/helpers.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Parses a string form location reference. + * + * Note that the return type is not `LocationSpec`, because we do not want to + * conflate the string form with the additional properties of that type. + * + * @param ref A string-form location reference, e.g. 'url:https://host' + * @returns A location reference, e.g. { type: 'url', target: 'https://host' } + */ +export function parseLocationReference( + ref: string, +): { type: string; target: string } { + if (typeof ref !== 'string') { + throw new TypeError( + `Unable to parse location reference '${ref}', unexpected argument ${typeof ref}`, + ); + } + + const splitIndex = ref.indexOf(':'); + if (splitIndex < 0) { + throw new TypeError( + `Unable to parse location reference '${ref}', expected ':', e.g. 'url:https://host/path'`, + ); + } + + const type = ref.substr(0, splitIndex).trim(); + const target = ref.substr(splitIndex + 1).trim(); + + if (!type || !target) { + throw new TypeError( + `Unable to parse location reference '${ref}', expected ':', e.g. 'url:https://host/path'`, + ); + } + + if (type === 'http' || type === 'https') { + throw new TypeError( + `Invalid location reference '${ref}', please prefix it with 'url:', e.g. 'url:${ref}'`, + ); + } + + return { type, target }; +} + +/** + * Turns a location reference into its string form. + * + * Note that the input type is not `LocationSpec`, because we do not want to + * conflate the string form with the additional properties of that type. + * + * @param ref A location reference, e.g. { type: 'url', target: 'https://host' } + * @returns A string-form location reference, e.g. 'url:https://host' + */ +export function stringifyLocationReference(ref: { + type: string; + target: string; +}): string { + const { type, target } = ref; + + if (!type) { + throw new TypeError(`Unable to stringify location reference, empty type`); + } else if (!target) { + throw new TypeError(`Unable to stringify location reference, empty target`); + } + + return `${type}:${target}`; +} diff --git a/packages/catalog-model/src/location/index.ts b/packages/catalog-model/src/location/index.ts index 6cfb074613..fddc8bde37 100644 --- a/packages/catalog-model/src/location/index.ts +++ b/packages/catalog-model/src/location/index.ts @@ -14,14 +14,15 @@ * limitations under the License. */ -export type { Location, LocationSpec } from './types'; -export { - locationSchema, - locationSpecSchema, - analyzeLocationSchema, -} from './validation'; export { LOCATION_ANNOTATION, ORIGIN_LOCATION_ANNOTATION, SOURCE_LOCATION_ANNOTATION, } from './annotation'; +export { parseLocationReference, stringifyLocationReference } from './helpers'; +export type { Location, LocationSpec } from './types'; +export { + analyzeLocationSchema, + locationSchema, + locationSpecSchema, +} from './validation'; diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index 6520f0f8e4..c31b1266fa 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -105,7 +105,7 @@ describe('parseReferenceAnnotation', () => { 'backstage.io/techdocs-ref', mockEntityWithBadAnnotation, ); - }).toThrow(/Failure to parse/); + }).toThrow(/Unable to parse/); }); }); diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts index 16f2876efb..856624c568 100644 --- a/packages/techdocs-common/src/helpers.ts +++ b/packages/techdocs-common/src/helpers.ts @@ -15,7 +15,7 @@ */ import { Git, InputError, UrlReader } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, parseLocationReference } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import fs from 'fs-extra'; import parseGitUrl from 'git-url-parse'; @@ -36,28 +36,15 @@ export const parseReferenceAnnotation = ( entity: Entity, ): ParsedLocationAnnotation => { const annotation = entity.metadata.annotations?.[annotationName]; - if (!annotation) { throw new InputError( `No location annotation provided in entity: ${entity.metadata.name}`, ); } - // split on the first colon for the protocol and the rest after the first split - // is the location. - const [type, target] = annotation.split(/:(.+)/) as [ - RemoteProtocol?, - string?, - ]; - - if (!type || !target) { - throw new InputError( - `Failure to parse either protocol or location for entity: ${entity.metadata.name}`, - ); - } - + const { type, target } = parseLocationReference(annotation); return { - type, + type: type as RemoteProtocol, target, }; }; @@ -77,8 +64,9 @@ export const getLocationForEntity = ( case 'url': return { type, target }; case 'dir': - if (path.isAbsolute(target)) return { type, target }; - + if (path.isAbsolute(target)) { + return { type, target }; + } return parseReferenceAnnotation( 'backstage.io/managed-by-location', entity, diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 467c0eb2e8..9d167603d0 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { Location, LocationSpec } from '@backstage/catalog-model'; +import { + Location, + LocationSpec, + stringifyLocationReference, +} from '@backstage/catalog-model'; import { v4 as uuidv4 } from 'uuid'; import { Logger } from 'winston'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; @@ -127,14 +131,18 @@ export class HigherOrderOperations implements HigherOrderOperation { for (const { data: location } of locations) { logger.info( - `Locations Refresh: Refreshing location ${location.type}:${location.target}`, + `Locations Refresh: Refreshing location ${stringifyLocationReference( + location, + )}`, ); try { await this.refreshSingleLocation(location, logger); await this.locationsCatalog.logUpdateSuccess(location.id, undefined); } catch (e) { logger.warn( - `Locations Refresh: Failed to refresh location ${location.type}:${location.target}, ${e.stack}`, + `Locations Refresh: Failed to refresh location ${stringifyLocationReference( + location, + )}, ${e.stack}`, ); await this.locationsCatalog.logUpdateFailure(location.id, e); } @@ -162,14 +170,18 @@ export class HigherOrderOperations implements HigherOrderOperation { for (const item of readerOutput.errors) { logger.warn( - `Failed item in location ${item.location.type}:${item.location.target}, ${item.error.stack}`, + `Failed item in location ${stringifyLocationReference( + item.location, + )}, ${item.error.stack}`, ); } logger.info( - `Read ${readerOutput.entities.length} entities from location ${ - location.type - }:${location.target} in ${durationText(startTimestamp)}`, + `Read ${ + readerOutput.entities.length + } entities from location ${stringifyLocationReference( + location, + )} in ${durationText(startTimestamp)}`, ); startTimestamp = process.hrtime(); @@ -198,9 +210,11 @@ export class HigherOrderOperations implements HigherOrderOperation { ); logger.info( - `Wrote ${readerOutput.entities.length} entities from location ${ - location.type - }:${location.target} in ${durationText(startTimestamp)}`, + `Wrote ${ + readerOutput.entities.length + } entities from location ${stringifyLocationReference( + location, + )} in ${durationText(startTimestamp)}`, ); } } diff --git a/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts index b40378226a..1d941e0040 100644 --- a/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts @@ -16,9 +16,10 @@ import { Entity, - LOCATION_ANNOTATION, LocationSpec, + LOCATION_ANNOTATION, ORIGIN_LOCATION_ANNOTATION, + stringifyLocationReference, } from '@backstage/catalog-model'; import lodash from 'lodash'; import { CatalogProcessor, CatalogProcessorEmit } from './types'; @@ -34,8 +35,10 @@ export class AnnotateLocationEntityProcessor implements CatalogProcessor { { metadata: { annotations: { - [LOCATION_ANNOTATION]: `${location.type}:${location.target}`, - [ORIGIN_LOCATION_ANNOTATION]: `${originLocation.type}:${originLocation.target}`, + [LOCATION_ANNOTATION]: stringifyLocationReference(location), + [ORIGIN_LOCATION_ANNOTATION]: stringifyLocationReference( + originLocation, + ), }, }, }, diff --git a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts index e1d7e22ccf..8a3951428e 100644 --- a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts @@ -15,7 +15,11 @@ */ import { NotFoundError, UrlReader } from '@backstage/backend-common'; -import { Entity, LocationSpec } from '@backstage/catalog-model'; +import { + Entity, + LocationSpec, + stringifyLocationReference, +} from '@backstage/catalog-model'; import * as codeowners from 'codeowners-utils'; import { CodeOwnersEntry } from 'codeowners-utils'; // NOTE: This can be removed when ES2021 is implemented @@ -108,11 +112,15 @@ export async function findRawCodeOwners( ); if (hardError) { options.logger.warn( - `Failed to read codeowners for location ${location.type}:${location.target}, ${hardError}`, + `Failed to read codeowners for location ${stringifyLocationReference( + location, + )}, ${hardError}`, ); } else { options.logger.debug( - `Failed to find codeowners for location ${location.type}:${location.target}`, + `Failed to find codeowners for location ${stringifyLocationReference( + location, + )}`, ); } return undefined; diff --git a/plugins/catalog/src/data/utils.ts b/plugins/catalog/src/data/utils.ts index ec63d94754..60b9144599 100644 --- a/plugins/catalog/src/data/utils.ts +++ b/plugins/catalog/src/data/utils.ts @@ -18,6 +18,7 @@ import { EntityMeta, LocationSpec, LOCATION_ANNOTATION, + parseLocationReference, } from '@backstage/catalog-model'; export function findLocationForEntityMeta( @@ -36,13 +37,9 @@ export function findLocationForEntityMeta( } export function parseLocation(reference: string): LocationSpec | undefined { - const separatorIndex = reference.indexOf(':'); - if (separatorIndex === -1) { + try { + return parseLocationReference(reference); + } catch { return undefined; } - - return { - type: reference.substring(0, separatorIndex), - target: reference.substring(separatorIndex + 1), - }; } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts index 5c809d229e..fc36eb64e5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts @@ -13,11 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { parseLocationAnnotation, joinGitUrlPath } from './helpers'; import { - TemplateEntityV1alpha1, LOCATION_ANNOTATION, + TemplateEntityV1alpha1, } from '@backstage/catalog-model'; +import { joinGitUrlPath, parseLocationAnnotation } from './helpers'; describe('Helpers', () => { describe('parseLocationAnnotation', () => { @@ -30,7 +30,7 @@ describe('Helpers', () => { name: 'graphql-starter', title: 'GraphQL Service', description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + 'A GraphQL starter template for backstage to get you up and running\nthe best practices with GraphQL\n', uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', generation: 1, @@ -78,7 +78,7 @@ describe('Helpers', () => { name: 'graphql-starter', title: 'GraphQL Service', description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + 'A GraphQL starter template for backstage to get you up and running\nthe best practices with GraphQL\n', uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', generation: 1, @@ -108,11 +108,13 @@ describe('Helpers', () => { expect(() => parseLocationAnnotation(mockEntity)).toThrow( expect.objectContaining({ - name: 'InputError', - message: `Failure to parse either protocol or location for entity: ${mockEntity.metadata.name}`, + name: 'TypeError', + message: + "Unable to parse location reference ':https://github.com/o/r/blob/master/template.yaml', expected ':', e.g. 'url:https://host/path'", }), ); }); + it('should throw an error when the location part is not set in the location annotation', () => { const mockEntity: TemplateEntityV1alpha1 = { apiVersion: 'backstage.io/v1alpha1', @@ -124,7 +126,7 @@ describe('Helpers', () => { name: 'graphql-starter', title: 'GraphQL Service', description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + 'A GraphQL starter template for backstage to get you up and running\nthe best practices with GraphQL\n', uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', generation: 1, @@ -154,8 +156,8 @@ describe('Helpers', () => { expect(() => parseLocationAnnotation(mockEntity)).toThrow( expect.objectContaining({ - name: 'InputError', - message: `Failure to parse either protocol or location for entity: ${mockEntity.metadata.name}`, + name: 'TypeError', + message: `Unable to parse location reference 'github:', expected ':', e.g. 'url:https://host/path'`, }), ); }); @@ -216,7 +218,7 @@ describe('Helpers', () => { name: 'graphql-starter', title: 'GraphQL Service', description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + 'A GraphQL starter template for backstage to get you up and running\nthe best practices with GraphQL\n', uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', generation: 1, diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts index 784e72ca87..924b9f5811 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts @@ -14,12 +14,13 @@ * limitations under the License. */ -import { posix as posixPath } from 'path'; -import { - TemplateEntityV1alpha1, - LOCATION_ANNOTATION, -} from '@backstage/catalog-model'; import { InputError } from '@backstage/backend-common'; +import { + LOCATION_ANNOTATION, + parseLocationReference, + TemplateEntityV1alpha1, +} from '@backstage/catalog-model'; +import { posix as posixPath } from 'path'; export type ParsedLocationAnnotation = { protocol: 'file' | 'url'; @@ -30,29 +31,16 @@ export const parseLocationAnnotation = ( entity: TemplateEntityV1alpha1, ): ParsedLocationAnnotation => { const annotation = entity.metadata.annotations?.[LOCATION_ANNOTATION]; - if (!annotation) { throw new InputError( `No location annotation provided in entity: ${entity.metadata.name}`, ); } - // split on the first colon for the protocol and the rest after the first split - // is the location. - const [protocol, location] = annotation.split(/:(.+)/) as [ - ('file' | 'url')?, - string?, - ]; - - if (!protocol || !location) { - throw new InputError( - `Failure to parse either protocol or location for entity: ${entity.metadata.name}`, - ); - } - + const { type, target } = parseLocationReference(annotation); return { - protocol, - location, + protocol: type as 'file' | 'url', + location: target, }; }; diff --git a/plugins/scaffolder-backend/src/service/helpers.ts b/plugins/scaffolder-backend/src/service/helpers.ts index 905d85c35d..cc0e2f3aaf 100644 --- a/plugins/scaffolder-backend/src/service/helpers.ts +++ b/plugins/scaffolder-backend/src/service/helpers.ts @@ -14,15 +14,16 @@ * limitations under the License. */ -import os from 'os'; -import fs from 'fs-extra'; -import { Logger } from 'winston'; -import { Config } from '@backstage/config'; import { Entity, LOCATION_ANNOTATION, + parseLocationReference, SOURCE_LOCATION_ANNOTATION, } from '@backstage/catalog-model'; +import { Config } from '@backstage/config'; +import fs from 'fs-extra'; +import os from 'os'; +import { Logger } from 'winston'; export async function getWorkingDirectory( config: Config, @@ -64,16 +65,13 @@ export function getEntityBaseUrl(entity: Entity): string | undefined { return undefined; } - const [type, url] = location.split(/:(.+)/); - if (!url) { - return undefined; + const { type, target } = parseLocationReference(location); + if (type === 'url') { + return target; + } else if (type === 'file') { + return `file://${target}`; } - if (type === 'url') { - return url; - } else if (type === 'file') { - return `file://${url}`; - } // Only url and file location are handled, as we otherwise don't know if // what the url is pointing to makes sense to use as a baseUrl return undefined;