diff --git a/.changeset/2798.md b/.changeset/2798.md new file mode 100644 index 0000000000..2b202142ef --- /dev/null +++ b/.changeset/2798.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Use the new `UrlReader` in `PlaceholderProcessor`. +This allows to use the placeholder processor to include API definitions in API entities. +Previously it was only possible to do this if the definition comes from the same location type as the entity itself. diff --git a/packages/catalog-model/examples/spotify-api.yaml b/packages/catalog-model/examples/spotify-api.yaml index 30524dfdd4..14deb17abe 100644 --- a/packages/catalog-model/examples/spotify-api.yaml +++ b/packages/catalog-model/examples/spotify-api.yaml @@ -7,8 +7,11 @@ metadata: - spotify - rest annotations: + # The annotation is deprecated, we use placeholders (see below) instead, remove it later. backstage.io/definition-at-location: 'url:https://raw.githubusercontent.com/APIs-guru/openapi-directory/master/APIs/spotify.com/v1/swagger.yaml' spec: type: openapi lifecycle: production owner: spotify@example.com + definition: + $text: https://github.com/APIs-guru/openapi-directory/blob/master/APIs/spotify.com/v1/swagger.yaml diff --git a/plugins/catalog-backend/src/ingestion/LocationReaders.ts b/plugins/catalog-backend/src/ingestion/LocationReaders.ts index fcc608eb51..45822f0ae6 100644 --- a/plugins/catalog-backend/src/ingestion/LocationReaders.ts +++ b/plugins/catalog-backend/src/ingestion/LocationReaders.ts @@ -125,7 +125,7 @@ export class LocationReaders implements LocationReader { LdapOrgReaderProcessor.fromConfig(config, { logger }), new UrlReaderProcessor(options), new YamlProcessor(), - PlaceholderProcessor.default(), + PlaceholderProcessor.default({ reader: options.reader }), new CodeOwnersProcessor(), new ApiDefinitionAtLocationProcessor(), new EntityPolicyProcessor(entityPolicy), diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts index 6893cae0e7..c196458e2a 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts @@ -13,20 +13,20 @@ * 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 { jsonPlaceholderResolver, PlaceholderProcessor, PlaceholderResolver, ResolverParams, + ResolverRead, yamlPlaceholderResolver, } from './PlaceholderProcessor'; -import { LocationProcessorEmit, LocationProcessorRead } from './types'; describe('PlaceholderProcessor', () => { - const emit: LocationProcessorEmit = jest.fn(); - const read: jest.MockedFunction = jest.fn(); + const read: jest.MockedFunction = jest.fn(); + const reader: UrlReader = { read }; beforeEach(() => { jest.resetAllMocks(); @@ -39,10 +39,13 @@ describe('PlaceholderProcessor', () => { metadata: { name: 'n' }, }; const processor = new PlaceholderProcessor({ - foo: async () => 'replaced', + resolvers: { + foo: async () => 'replaced', + }, + reader, }); await expect( - processor.processEntity(input, { type: 't', target: 'l' }, emit, read), + processor.processEntity(input, { type: 't', target: 'l' }), ).resolves.toBe(input); }); @@ -51,7 +54,10 @@ describe('PlaceholderProcessor', () => { value!.toString().toUpperCase(), ); const processor = new PlaceholderProcessor({ - upper: upperResolver, + resolvers: { + upper: upperResolver, + }, + reader, }); await expect( @@ -63,8 +69,6 @@ describe('PlaceholderProcessor', () => { spec: { a: [{ b: { $upper: 'text' } }] }, }, { type: 'fake', target: 'http://example.com' }, - emit, - read, ), ).resolves.toEqual({ apiVersion: 'a', @@ -73,20 +77,23 @@ describe('PlaceholderProcessor', () => { spec: { a: [{ b: 'TEXT' }] }, }); - expect(emit).not.toBeCalled(); expect(read).not.toBeCalled(); - expect(upperResolver).toBeCalledWith({ - key: 'upper', - value: 'text', - location: { type: 'fake', target: 'http://example.com' }, - read, - }); + expect(upperResolver).toBeCalledWith( + expect.objectContaining({ + key: 'upper', + value: 'text', + baseUrl: 'http://example.com', + }), + ); }); it('rejects multiple placeholders', async () => { const processor = new PlaceholderProcessor({ - foo: jest.fn(), - bar: jest.fn(), + resolvers: { + foo: jest.fn(), + bar: jest.fn(), + }, + reader, }); await expect( @@ -97,20 +104,20 @@ describe('PlaceholderProcessor', () => { metadata: { name: 'n', x: { $foo: 'a', $bar: 'b' } }, }, { type: 'a', target: 'b' }, - emit, - read, ), ).rejects.toThrow( 'Placeholders have to be on the form of a single $-prefixed key in an object', ); - expect(emit).not.toBeCalled(); expect(read).not.toBeCalled(); }); it('rejects unknown placeholders', async () => { const processor = new PlaceholderProcessor({ - bar: jest.fn(), + resolvers: { + bar: jest.fn(), + }, + reader, }); await expect( @@ -121,18 +128,15 @@ describe('PlaceholderProcessor', () => { metadata: { name: 'n', x: { $foo: 'a' } }, }, { type: 'a', target: 'b' }, - emit, - read, ), ).rejects.toThrow('Encountered unknown placeholder $foo'); - expect(emit).not.toBeCalled(); expect(read).not.toBeCalled(); }); it('has builtin text support', async () => { read.mockResolvedValue(Buffer.from('TEXT', 'utf-8')); - const processor = PlaceholderProcessor.default(); + const processor = PlaceholderProcessor.default({ reader }); await expect( processor.processEntity( @@ -146,8 +150,6 @@ describe('PlaceholderProcessor', () => { type: 'github', target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', }, - emit, - read, ), ).resolves.toEqual({ apiVersion: 'a', @@ -156,18 +158,16 @@ describe('PlaceholderProcessor', () => { spec: { data: 'TEXT' }, }); - expect(emit).not.toBeCalled(); - expect(read).toBeCalledWith({ - type: 'github', - target: 'https://github.com/spotify/backstage/a/file.txt', - }); + expect(read).toBeCalledWith( + 'https://github.com/spotify/backstage/a/file.txt', + ); }); it('has builtin json support', async () => { read.mockResolvedValue( Buffer.from(JSON.stringify({ a: ['b', 7] }), 'utf-8'), ); - const processor = PlaceholderProcessor.default(); + const processor = PlaceholderProcessor.default({ reader }); await expect( processor.processEntity( @@ -181,8 +181,6 @@ describe('PlaceholderProcessor', () => { type: 'github', target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', }, - emit, - read, ), ).resolves.toEqual({ apiVersion: 'a', @@ -191,16 +189,14 @@ describe('PlaceholderProcessor', () => { spec: { data: { a: ['b', 7] } }, }); - expect(emit).not.toBeCalled(); - expect(read).toBeCalledWith({ - type: 'github', - target: 'https://github.com/spotify/backstage/a/b/file.json', - }); + expect(read).toBeCalledWith( + 'https://github.com/spotify/backstage/a/b/file.json', + ); }); it('has builtin yaml support', async () => { read.mockResolvedValue(Buffer.from('foo:\n - bar: 7', 'utf-8')); - const processor = PlaceholderProcessor.default(); + const processor = PlaceholderProcessor.default({ reader }); await expect( processor.processEntity( @@ -214,8 +210,6 @@ describe('PlaceholderProcessor', () => { type: 'github', target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', }, - emit, - read, ), ).resolves.toEqual({ apiVersion: 'a', @@ -224,16 +218,14 @@ describe('PlaceholderProcessor', () => { spec: { data: { foo: [{ bar: 7 }] } }, }); - expect(emit).not.toBeCalled(); - expect(read).toBeCalledWith({ - type: 'github', - target: 'https://github.com/spotify/backstage/a/file.yaml', - }); + expect(read).toBeCalledWith( + 'https://github.com/spotify/backstage/a/file.yaml', + ); }); it('resolves absolute path for absolute location', async () => { read.mockResolvedValue(Buffer.from('TEXT', 'utf-8')); - const processor = PlaceholderProcessor.default(); + const processor = PlaceholderProcessor.default({ reader }); await expect( processor.processEntity( @@ -251,8 +243,6 @@ describe('PlaceholderProcessor', () => { type: 'github', target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', }, - emit, - read, ), ).resolves.toEqual({ apiVersion: 'a', @@ -261,16 +251,14 @@ describe('PlaceholderProcessor', () => { spec: { data: 'TEXT' }, }); - expect(emit).not.toBeCalled(); - expect(read).toBeCalledWith({ - type: 'github', - target: 'https://github.com/spotify/backstage/catalog-info.yaml', - }); + expect(read).toBeCalledWith( + 'https://github.com/spotify/backstage/catalog-info.yaml', + ); }); it('resolves absolute path for relative file location', async () => { read.mockResolvedValue(Buffer.from('TEXT', 'utf-8')); - const processor = PlaceholderProcessor.default(); + const processor = PlaceholderProcessor.default({ reader }); await expect( processor.processEntity( @@ -288,8 +276,6 @@ describe('PlaceholderProcessor', () => { type: 'github', target: './a/b/catalog-info.yaml', }, - emit, - read, ), ).resolves.toEqual({ apiVersion: 'a', @@ -298,11 +284,9 @@ describe('PlaceholderProcessor', () => { spec: { data: 'TEXT' }, }); - expect(emit).not.toBeCalled(); - expect(read).toBeCalledWith({ - type: 'github', - target: 'https://github.com/spotify/backstage/catalog-info.yaml', - }); + expect(read).toBeCalledWith( + 'https://github.com/spotify/backstage/catalog-info.yaml', + ); }); it('not resolves relative file path for relative file location', async () => { @@ -310,7 +294,7 @@ describe('PlaceholderProcessor', () => { // traversel attacks. If we want to implement this, we need to have additional // security measures in place! read.mockResolvedValue(Buffer.from('TEXT', 'utf-8')); - const processor = PlaceholderProcessor.default(); + const processor = PlaceholderProcessor.default({ reader }); await expect( processor.processEntity( @@ -328,27 +312,21 @@ describe('PlaceholderProcessor', () => { type: 'github', target: './a/b/catalog-info.yaml', }, - emit, - read, ), ).rejects.toThrow( 'Placeholder $text could not form an URL out of ./a/b/catalog-info.yaml and ../c/catalog-info.yaml', ); - expect(emit).not.toBeCalled(); expect(read).not.toBeCalled(); }); }); describe('yamlPlaceholderResolver', () => { - const read: jest.MockedFunction = jest.fn(); + const read: jest.MockedFunction = jest.fn(); const params: ResolverParams = { key: 'a', value: './file.yaml', - location: { - type: 'github', - target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', - }, + baseUrl: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', read, }; @@ -388,14 +366,11 @@ describe('yamlPlaceholderResolver', () => { }); describe('jsonPlaceholderResolver', () => { - const read: jest.MockedFunction = jest.fn(); + const read: jest.MockedFunction = jest.fn(); const params: ResolverParams = { key: 'a', value: './file.json', - location: { - type: 'github', - target: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', - }, + baseUrl: 'https://github.com/spotify/backstage/a/b/catalog-info.yaml', read, }; diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts index b811d217d5..91257b68a4 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts @@ -14,49 +14,49 @@ * limitations under the License. */ +import { UrlReader } from '@backstage/backend-common'; import { Entity, LocationSpec } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; import yaml from 'yaml'; -import { - LocationProcessor, - LocationProcessorEmit, - LocationProcessorRead, -} from './types'; +import { LocationProcessor } from './types'; + +export type ResolverRead = (url: string) => Promise; export type ResolverParams = { key: string; value: JsonValue; - location: LocationSpec; - read: LocationProcessorRead; + baseUrl: string; + read: ResolverRead; }; export type PlaceholderResolver = ( params: ResolverParams, ) => Promise; +type Options = { + resolvers: Record; + reader: UrlReader; +}; + /** * Traverses raw entity JSON looking for occurrences of $-prefixed placeholders * that it then fills in with actual data. */ export class PlaceholderProcessor implements LocationProcessor { - static default() { + static default({ reader }: { reader: UrlReader }) { return new PlaceholderProcessor({ - json: jsonPlaceholderResolver, - yaml: yamlPlaceholderResolver, - text: textPlaceholderResolver, + resolvers: { + json: jsonPlaceholderResolver, + yaml: yamlPlaceholderResolver, + text: textPlaceholderResolver, + }, + reader, }); } - constructor( - private readonly resolvers: Record, - ) {} + constructor(private readonly options: Options) {} - async processEntity( - entity: Entity, - location: LocationSpec, - _emit: LocationProcessorEmit, - read: LocationProcessorRead, - ): Promise { + async processEntity(entity: Entity, location: LocationSpec): Promise { const process = async (data: any): Promise<[any, boolean]> => { if (!data || !(data instanceof Object)) { // Scalars can't have placeholders @@ -90,7 +90,7 @@ export class PlaceholderProcessor implements LocationProcessor { } const resolverKey = keys[0].substr(1); - const resolver = this.resolvers[resolverKey]; + const resolver = this.options.resolvers[resolverKey]; if (!resolver) { throw new Error(`Encountered unknown placeholder \$${resolverKey}`); } @@ -99,8 +99,8 @@ export class PlaceholderProcessor implements LocationProcessor { await resolver({ key: resolverKey, value: data[keys[0]], - location, - read, + baseUrl: location.target, + read: this.options.reader.read.bind(this.options.reader), }), true, ]; @@ -171,10 +171,10 @@ export async function textPlaceholderResolver( */ async function readTextLocation(params: ResolverParams): Promise { - const newLocation = relativeLocation(params); + const newUrl = relativeUrl(params); try { - const data = await params.read(newLocation); + const data = await params.read(newUrl); return data.toString('utf-8'); } catch (e) { throw new Error( @@ -183,11 +183,7 @@ async function readTextLocation(params: ResolverParams): Promise { } } -function relativeLocation({ - key, - value, - location, -}: ResolverParams): LocationSpec { +function relativeUrl({ key, value, baseUrl }: ResolverParams): string { if (typeof value !== 'string') { throw new Error( `Placeholder \$${key} expected a string value parameter, in the form of an absolute URL or a relative path`, @@ -197,7 +193,7 @@ function relativeLocation({ let url: URL; try { // The two-value form of the URL constructor handles relative paths for us - url = new URL(value, location.target); + url = new URL(value, baseUrl); } catch { try { // Check whether value is a valid absolute URL on it's own, if not fail. @@ -208,13 +204,10 @@ function relativeLocation({ // path traversal attacks and access to any file on the host system. Implementing this // would require additional security measures. throw new Error( - `Placeholder \$${key} could not form an URL out of ${location.target} and ${value}`, + `Placeholder \$${key} could not form an URL out of ${baseUrl} and ${value}`, ); } } - return { - type: location.type, - target: url.toString(), - }; + return url.toString(); }