diff --git a/.changeset/four-paws-pull.md b/.changeset/four-paws-pull.md new file mode 100644 index 0000000000..183b3dc5d7 --- /dev/null +++ b/.changeset/four-paws-pull.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Make use of the `resolveUrl` facility of the `integration` package. + +Also rename the `LocationRefProcessor` to `LocationEntityProcessor`, to match the file name. This constitutes an interface change since the class is exported, but it is unlikely to be consumed outside of the package since it sits comfortably with the other default processors inside the catalog builder. diff --git a/.changeset/tame-points-search.md b/.changeset/tame-points-search.md new file mode 100644 index 0000000000..7fffafc1cf --- /dev/null +++ b/.changeset/tame-points-search.md @@ -0,0 +1,10 @@ +--- +'@backstage/integration': patch +--- + +Add a `resolveUrl` method to integrations, that works like the two-argument URL +constructor. The reason for using this is that Azure have their paths in a +query parameter, rather than the pathname of the URL. + +The implementation is optional (when not present, the URL constructor is used), +so this does not imply a breaking change. diff --git a/packages/integration/src/ScmIntegrations.test.ts b/packages/integration/src/ScmIntegrations.test.ts index b43e69eba4..0d949cfdc2 100644 --- a/packages/integration/src/ScmIntegrations.test.ts +++ b/packages/integration/src/ScmIntegrations.test.ts @@ -73,4 +73,19 @@ describe('ScmIntegrations', () => { expect(i.byHost('github.local')).toBe(github); expect(i.byHost('gitlab.local')).toBe(gitlab); }); + + it('can resolveUrl using fallback', () => { + expect( + i.resolveUrl({ + url: '../b.yaml', + base: 'https://no-matching-integration.com/x/a.yaml', + }), + ).toBe('https://no-matching-integration.com/b.yaml'); + expect( + i.resolveUrl({ + url: 'https://absolute.com/path', + base: 'https://no-matching-integration.com/x/a.yaml', + }), + ).toBe('https://absolute.com/path'); + }); }); diff --git a/packages/integration/src/ScmIntegrations.ts b/packages/integration/src/ScmIntegrations.ts index 102273a03b..c20fdcef9c 100644 --- a/packages/integration/src/ScmIntegrations.ts +++ b/packages/integration/src/ScmIntegrations.ts @@ -81,4 +81,13 @@ export class ScmIntegrations implements ScmIntegrationRegistry { .map(i => i.byHost(host)) .find(Boolean); } + + resolveUrl(options: { url: string; base: string }): string { + const resolve = this.byUrl(options.base)?.resolveUrl; + if (!resolve) { + return new URL(options.url, options.base).toString(); + } + + return resolve(options); + } } diff --git a/packages/integration/src/azure/AzureIntegration.test.ts b/packages/integration/src/azure/AzureIntegration.test.ts index 90c098b637..8a10e40bcf 100644 --- a/packages/integration/src/azure/AzureIntegration.test.ts +++ b/packages/integration/src/azure/AzureIntegration.test.ts @@ -41,4 +41,52 @@ describe('AzureIntegration', () => { expect(integration.type).toBe('azure'); expect(integration.title).toBe('h.com'); }); + + describe('resolveUrl', () => { + it('works for valid urls', () => { + const integration = new AzureIntegration({ + host: 'dev.azure.com', + } as any); + + expect( + integration.resolveUrl({ + url: '../a.yaml', + base: + 'https://dev.azure.com/organization/project/_git/repository?path=%2Ffolder%2Fcatalog-info.yaml', + }), + ).toBe( + 'https://dev.azure.com/organization/project/_git/repository?path=%2Fa.yaml', + ); + + expect( + integration.resolveUrl({ + url: './a.yaml', + base: 'https://dev.azure.com/organization/project/_git/repository', + }), + ).toBe( + 'https://dev.azure.com/organization/project/_git/repository?path=%2Fa.yaml', + ); + + expect( + integration.resolveUrl({ + url: 'https://absolute.com/path', + base: + 'https://dev.azure.com/organization/project/_git/repository?path=%2Fcatalog-info.yaml', + }), + ).toBe('https://absolute.com/path'); + }); + + it('falls back to regular URL resolution if not in a repo', () => { + const integration = new AzureIntegration({ + host: 'dev.azure.com', + } as any); + + expect( + integration.resolveUrl({ + url: './test', + base: 'https://dev.azure.com/organization/project/_git', + }), + ).toBe('https://dev.azure.com/organization/project/test'); + }); + }); }); diff --git a/packages/integration/src/azure/AzureIntegration.ts b/packages/integration/src/azure/AzureIntegration.ts index 446e6a9480..1413b7b343 100644 --- a/packages/integration/src/azure/AzureIntegration.ts +++ b/packages/integration/src/azure/AzureIntegration.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import parseGitUrl from 'git-url-parse'; import { basicIntegrations } from '../helpers'; import { ScmIntegration, ScmIntegrationsFactory } from '../types'; import { AzureIntegrationConfig, readAzureIntegrationConfigs } from './config'; @@ -42,4 +43,39 @@ export class AzureIntegration implements ScmIntegration { get config(): AzureIntegrationConfig { return this.integrationConfig; } + + /* + * Azure repo URLs on the form with a `path` query param are treated specially. + * + * Example base URL: https://dev.azure.com/organization/project/_git/repository?path=%2Fcatalog-info.yaml + */ + resolveUrl(options: { url: string; base: string }): string { + const { url, base } = options; + + // If we can parse the url, it is absolute - then return it verbatim + try { + // eslint-disable-next-line no-new + new URL(url); + return url; + } catch { + // Ignore intentionally - looks like a relative path + } + + const parsed = parseGitUrl(base); + const { organization, owner, name, filepath } = parsed; + + // If not an actual file path within a repo, treat the URL as raw + if (!organization || !owner || !name) { + return new URL(url, base).toString(); + } + + const path = filepath?.replace(/^\//, '') || ''; + const mockBaseUrl = new URL(`https://a.com/${path}`); + const updatedPath = new URL(url, mockBaseUrl).pathname; + + const newUrl = new URL(base); + newUrl.searchParams.set('path', updatedPath); + + return newUrl.toString(); + } } diff --git a/packages/integration/src/types.ts b/packages/integration/src/types.ts index d8fb7a14e2..7d5b0fbe74 100644 --- a/packages/integration/src/types.ts +++ b/packages/integration/src/types.ts @@ -34,6 +34,18 @@ export interface ScmIntegration { * differentiate between different integrations. */ title: string; + + /** + * Works like the two-argument form of the URL constructor, resolving an + * absolute or relative URL in relation to a base URL. + * + * If this method is not implemented, the URL constructor is used instead for + * URLs that match this integration. + * + * @param options.url The (absolute or relative) URL or path to resolve + * @param options.base The base URL onto which this resolution happens + */ + resolveUrl?(options: { url: string; base: string }): string; } /** @@ -69,6 +81,15 @@ export interface ScmIntegrationRegistry bitbucket: ScmIntegrationsGroup; github: ScmIntegrationsGroup; gitlab: ScmIntegrationsGroup; + + /** + * Works like the two-argument form of the URL constructor, resolving an + * absolute or relative URL in relation to a base URL. + * + * @param options.url The (absolute or relative) URL or path to resolve + * @param options.base The base URL onto which this resolution happens + */ + resolveUrl(options: { url: string; base: string }): string; } export type ScmIntegrationsFactory = (options: { diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index 264bda0556..cc0a9e412a 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -34,6 +34,7 @@ "@backstage/backend-common": "^0.5.1", "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", + "@backstage/integration": "^0.3.1", "@octokit/graphql": "^4.5.8", "@types/express": "^4.17.6", "@types/ldapjs": "^1.0.9", diff --git a/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.test.ts index 998dfb62a2..476510d757 100644 --- a/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.test.ts @@ -15,30 +15,73 @@ */ import { LocationSpec } from '@backstage/catalog-model'; -import { toAbsoluteUrl } from './LocationEntityProcessor'; +import { ConfigReader } from '@backstage/config'; +import { + ScmIntegrations, + ScmIntegrationRegistry, +} from '@backstage/integration'; import path from 'path'; +import { toAbsoluteUrl } from './LocationEntityProcessor'; describe('LocationEntityProcessor', () => { describe('toAbsoluteUrl', () => { it('handles files', () => { + const integrations = ({} as unknown) as ScmIntegrationRegistry; const base: LocationSpec = { type: 'file', target: `some${path.sep}path${path.sep}catalog-info.yaml`, }; - expect(toAbsoluteUrl(base, `.${path.sep}c`)).toBe( + expect(toAbsoluteUrl(integrations, base, `.${path.sep}c`)).toBe( `some${path.sep}path${path.sep}c`, ); - expect(toAbsoluteUrl(base, `${path.sep}c`)).toBe(`${path.sep}c`); + expect(toAbsoluteUrl(integrations, base, `${path.sep}c`)).toBe( + `${path.sep}c`, + ); }); it('handles urls', () => { + const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); const base: LocationSpec = { type: 'url', target: 'http://a.com/b/catalog-info.yaml', }; - expect(toAbsoluteUrl(base, './c/d')).toBe('http://a.com/b/c/d'); - expect(toAbsoluteUrl(base, 'c/d')).toBe('http://a.com/b/c/d'); - expect(toAbsoluteUrl(base, 'http://b.com/z')).toBe('http://b.com/z'); + jest.spyOn(integrations, 'resolveUrl'); + + expect(toAbsoluteUrl(integrations, base, './c/d')).toBe( + 'http://a.com/b/c/d', + ); + expect(toAbsoluteUrl(integrations, base, 'c/d')).toBe( + 'http://a.com/b/c/d', + ); + expect(toAbsoluteUrl(integrations, base, 'http://b.com/z')).toBe( + 'http://b.com/z', + ); + + expect(integrations.resolveUrl).toBeCalledTimes(3); + }); + + it('handles azure urls specifically', () => { + const integrations = ScmIntegrations.fromConfig( + new ConfigReader({ + integrations: { + azure: [{ host: 'dev.azure.com' }], + }, + }), + ); + + expect( + toAbsoluteUrl( + integrations, + { + type: 'url', + target: + 'https://dev.azure.com/organization/project/_git/repository?path=%2Fcatalog-info.yaml', + }, + './a.yaml', + ), + ).toBe( + 'https://dev.azure.com/organization/project/_git/repository?path=%2Fa.yaml', + ); }); }); }); diff --git a/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.ts index 1c7e13f1c4..f4c8c82957 100644 --- a/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/LocationEntityProcessor.ts @@ -15,11 +15,16 @@ */ import { Entity, LocationEntity, LocationSpec } from '@backstage/catalog-model'; +import { ScmIntegrationRegistry } from '@backstage/integration'; +import path from 'path'; import * as result from './results'; import { CatalogProcessor, CatalogProcessorEmit } from './types'; -import path from 'path'; -export function toAbsoluteUrl(base: LocationSpec, target: string): string { +export function toAbsoluteUrl( + integrations: ScmIntegrationRegistry, + base: LocationSpec, + target: string, +): string { try { if (base.type === 'file') { if (target.startsWith('.')) { @@ -27,13 +32,19 @@ export function toAbsoluteUrl(base: LocationSpec, target: string): string { } return target; } - return new URL(target, base.target).toString(); + return integrations.resolveUrl({ url: target, base: base.target }); } catch (e) { return target; } } -export class LocationRefProcessor implements CatalogProcessor { +type Options = { + integrations: ScmIntegrationRegistry; +}; + +export class LocationEntityProcessor implements CatalogProcessor { + constructor(private readonly options: Options) {} + async postProcessEntity( entity: Entity, location: LocationSpec, @@ -47,7 +58,7 @@ export class LocationRefProcessor implements CatalogProcessor { emit( result.inputError( location, - `LocationRefProcessor cannot handle ${type} type location with target ${location.target} that ends with a path separator`, + `LocationEntityProcessor cannot handle ${type} type location with target ${location.target} that ends with a path separator`, ), ); } @@ -61,7 +72,11 @@ export class LocationRefProcessor implements CatalogProcessor { } for (const maybeRelativeTarget of targets) { - const target = toAbsoluteUrl(location, maybeRelativeTarget); + const target = toAbsoluteUrl( + this.options.integrations, + location, + maybeRelativeTarget, + ); emit(result.location({ type, target }, false)); } } diff --git a/plugins/catalog-backend/src/ingestion/processors/index.ts b/plugins/catalog-backend/src/ingestion/processors/index.ts index a7b00d7065..05dd327f0e 100644 --- a/plugins/catalog-backend/src/ingestion/processors/index.ts +++ b/plugins/catalog-backend/src/ingestion/processors/index.ts @@ -23,7 +23,7 @@ export { CodeOwnersProcessor } from './CodeOwnersProcessor'; export { FileReaderProcessor } from './FileReaderProcessor'; export { GithubOrgReaderProcessor } from './GithubOrgReaderProcessor'; export { LdapOrgReaderProcessor } from './LdapOrgReaderProcessor'; -export { LocationRefProcessor } from './LocationEntityProcessor'; +export { LocationEntityProcessor } from './LocationEntityProcessor'; export { MicrosoftGraphOrgReaderProcessor } from './MicrosoftGraphOrgReaderProcessor'; export { PlaceholderProcessor } from './PlaceholderProcessor'; export type { PlaceholderResolver } from './PlaceholderProcessor'; diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 938a406e56..f7544ef42e 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -26,6 +26,7 @@ import { Validators, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; import lodash from 'lodash'; import { Logger } from 'winston'; import { @@ -46,8 +47,8 @@ import { HigherOrderOperation, HigherOrderOperations, LdapOrgReaderProcessor, + LocationEntityProcessor, LocationReaders, - LocationRefProcessor, MicrosoftGraphOrgReaderProcessor, PlaceholderProcessor, PlaceholderResolver, @@ -280,6 +281,7 @@ export class CatalogBuilder { private buildProcessors(): CatalogProcessor[] { const { config, logger, reader } = this.env; + const integrations = ScmIntegrations.fromConfig(config); this.checkDeprecatedReaderProcessors(); @@ -306,7 +308,7 @@ export class CatalogBuilder { MicrosoftGraphOrgReaderProcessor.fromConfig(config, { logger }), new UrlReaderProcessor({ reader, logger }), new CodeOwnersProcessor({ reader, logger }), - new LocationRefProcessor(), + new LocationEntityProcessor({ integrations }), new AnnotateLocationEntityProcessor(), ); }