diff --git a/.changeset/cyan-islands-swim.md b/.changeset/cyan-islands-swim.md new file mode 100644 index 0000000000..396cc6fc68 --- /dev/null +++ b/.changeset/cyan-islands-swim.md @@ -0,0 +1,14 @@ +--- +'@backstage/plugin-catalog-backend': major +'@backstage/create-app': patch +--- + +**BREAKING:** The experimental `catalog.useUrlReadersSearch` configuration flag (introduced in v1.36) has been removed. + +The `UrlReaderProcessor` now always uses the `search` method of `UrlReaders`. Built-in `UrlReaderService` implementations have been updated accordingly. +If you use custom `UrlReaderService` implementations, you need to adapt their `search` method to correctly handle both specific URLs and potential +search patterns (see changes on built-in readers [in the original PR](https://github.com/backstage/backstage/pull/28379/files#diff-68b0452f173ee54bdd40f7b5e047a9cb8bb59200425622c212c217b76dac1d1b)). + +Previous behavior was to call the `search` method only if the parsed Git URL's filename contained a wildcard and use `readUrl` otherwise. Each `UrlReaderService` must implement this logic in the `search` method instead. + +This allows each `UrlReaderService` implementation to check whether it's a search URL (that contains a wildcard pattern) or not using logic that is specific to each provider. diff --git a/app-config.yaml b/app-config.yaml index 89e417fad3..64c2fa5c4c 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -244,7 +244,6 @@ integrations: secretAccessKey: ${AWS_SECRET_ACCESS_KEY} catalog: - useUrlReadersSearch: true import: entityFilename: catalog-info.yaml pullRequestBranchName: backstage-integration diff --git a/packages/create-app/templates/default-app/app-config.yaml.hbs b/packages/create-app/templates/default-app/app-config.yaml.hbs index fc6918acea..ca52ec530c 100644 --- a/packages/create-app/templates/default-app/app-config.yaml.hbs +++ b/packages/create-app/templates/default-app/app-config.yaml.hbs @@ -103,9 +103,6 @@ catalog: # target: https://github.com/backstage/backstage/blob/master/packages/catalog-model/examples/acme-corp.yaml # rules: # - allow: [User, Group] - # Experimental: Always use the search method in UrlReaderProcessor. - # New adopters are encouraged to enable it as this behavior will be the default in a future release. - useUrlReadersSearch: true kubernetes: # see https://backstage.io/docs/features/kubernetes/configuration for kubernetes configuration options diff --git a/plugins/catalog-backend/config.d.ts b/plugins/catalog-backend/config.d.ts index 7e68c4bea9..3ba6047ada 100644 --- a/plugins/catalog-backend/config.d.ts +++ b/plugins/catalog-backend/config.d.ts @@ -218,19 +218,5 @@ export interface Config { * housing catalog-info files. */ processingInterval?: HumanDuration | false; - - /** - * Defines if the UrlReaderProcessor should always call the search method of the - * different UrlReaders. - * - * If set to false, the UrlReaderProcessor will use the legacy behavior that tries to - * parse a Git URL and calls search if there's wildcard patterns and readUrl otherwise. - * - * If set to true, the UrlReaderProcessor always call the search method and lets each UrlReader - * determine if it's a search pattern or not. - * - * This flag is temporary and will be enabled by default in future releases. - */ - useUrlReadersSearch?: boolean; }; } diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index 8f2f8afe94..1b68e7d58d 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -99,7 +99,6 @@ "@backstage/cli": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", "@backstage/repo-tools": "workspace:^", - "@backstage/test-utils": "workspace:^", "@types/core-js": "^2.5.4", "@types/express": "^4.17.6", "@types/git-url-parse": "^9.0.0", diff --git a/plugins/catalog-backend/report.api.md b/plugins/catalog-backend/report.api.md index 9bb94b2dd5..6e5ca956bc 100644 --- a/plugins/catalog-backend/report.api.md +++ b/plugins/catalog-backend/report.api.md @@ -135,11 +135,7 @@ export function transformLegacyPolicyToProcessor( // @public (undocumented) export class UrlReaderProcessor implements CatalogProcessor { - constructor(options: { - reader: UrlReaderService; - logger: LoggerService; - config?: Config; - }); + constructor(options: { reader: UrlReaderService; logger: LoggerService }); // (undocumented) getProcessorName(): string; // (undocumented) diff --git a/plugins/catalog-backend/src/processors/UrlReaderProcessor.test.ts b/plugins/catalog-backend/src/processors/UrlReaderProcessor.test.ts index 2d7268eb33..0b17ecb19f 100644 --- a/plugins/catalog-backend/src/processors/UrlReaderProcessor.test.ts +++ b/plugins/catalog-backend/src/processors/UrlReaderProcessor.test.ts @@ -30,8 +30,6 @@ import { import { defaultEntityDataParser } from '../util/parse'; import { UrlReaderProcessor } from './UrlReaderProcessor'; import { UrlReaders } from '@backstage/backend-defaults/urlReader'; -import { UrlReaderService } from '@backstage/backend-plugin-api'; -import { mockApis } from '@backstage/test-utils'; describe('UrlReaderProcessor', () => { const mockApiOrigin = 'http://localhost'; @@ -191,11 +189,11 @@ describe('UrlReaderProcessor', () => { expect(generated.location).toBe(spec); expect(generated.error.name).toBe('NotFoundError'); expect(generated.error.message).toBe( - `Unable to read url, NotFoundError: could not read ${mockApiOrigin}/component-notfound.yaml, 404 Not Found`, + `Unable to read url, no matching files found for ${mockApiOrigin}/component-notfound.yaml`, ); }); - it('uses search when there are globs', async () => { + it("uses reader' search method", async () => { const logger = mockServices.logger.mock(); const reader = mockServices.urlReader.mock({ @@ -206,38 +204,6 @@ describe('UrlReaderProcessor', () => { const emit = jest.fn(); - await processor.readLocation( - { type: 'url', target: 'https://github.com/a/b/blob/x/**/b.yaml' }, - false, - emit, - defaultEntityDataParser, - mockCache, - ); - - expect(reader.search).toHaveBeenCalledTimes(1); - }); - - it('uses search when catalog.useUrlReadersSearch flag is set to true', async () => { - const logger = mockServices.logger.mock(); - - const reader: jest.Mocked = { - readUrl: jest.fn(), - readTree: jest.fn(), - search: jest.fn().mockImplementation(async () => []), - }; - - const config = mockApis.config({ - data: { - catalog: { - useUrlReadersSearch: true, - }, - }, - }); - - const processor = new UrlReaderProcessor({ reader, logger, config }); - - const emit = jest.fn(); - await processor.readLocation( { type: 'url', target: 'https://github.com/a/b/blob/x/b.yaml' }, false, diff --git a/plugins/catalog-backend/src/processors/UrlReaderProcessor.ts b/plugins/catalog-backend/src/processors/UrlReaderProcessor.ts index bc383512be..4440a81bae 100644 --- a/plugins/catalog-backend/src/processors/UrlReaderProcessor.ts +++ b/plugins/catalog-backend/src/processors/UrlReaderProcessor.ts @@ -18,7 +18,6 @@ import { Entity } from '@backstage/catalog-model'; import { assertError } from '@backstage/errors'; import limiterFactory, { Limit } from 'p-limit'; import { LocationSpec } from '@backstage/plugin-catalog-common'; -import parseGitUrl from 'git-url-parse'; import { CatalogProcessor, CatalogProcessorCache, @@ -29,7 +28,6 @@ import { processingResult, } from '@backstage/plugin-catalog-node'; import { LoggerService, UrlReaderService } from '@backstage/backend-plugin-api'; -import { Config } from '@backstage/config'; const CACHE_KEY = 'v1'; @@ -48,25 +46,14 @@ export class UrlReaderProcessor implements CatalogProcessor { // This limiter is used for only consuming a limited number of read streams // concurrently. #limiter: Limit; - #useUrlReadersSearch: boolean; constructor( private readonly options: { reader: UrlReaderService; logger: LoggerService; - config?: Config; }, ) { this.#limiter = limiterFactory(5); - - this.#useUrlReadersSearch = - this.options.config?.getOptionalBoolean('catalog.useUrlReadersSearch') || - false; - if (!this.#useUrlReadersSearch) { - this.options.logger.warn( - 'UrlReaderProcessor uses the legacy readUrl/search behavior which will be removed in a future release. Set catalog.useUrlReadersSearch to true to adopt the new behavior.', - ); - } } getProcessorName() { @@ -149,35 +136,13 @@ export class UrlReaderProcessor implements CatalogProcessor { location: string, etag?: string, ): Promise<{ response: { data: Buffer; url: string }[]; etag?: string }> { - // New behavior: always use the search method - if (this.#useUrlReadersSearch) { - const response = await this.options.reader.search(location, { etag }); + const response = await this.options.reader.search(location, { etag }); - const output = response.files.map(async file => ({ - url: file.url, - data: await this.#limiter(file.content), - })); + const output = response.files.map(async file => ({ + url: file.url, + data: await this.#limiter(file.content), + })); - return { response: await Promise.all(output), etag: response.etag }; - } - - // Old behavior: Does it contain globs? I.e. does it contain asterisks or question marks - // (no curly braces for now) - - const { filepath } = parseGitUrl(location); - if (filepath?.match(/[*?]/)) { - const response = await this.options.reader.search(location, { etag }); - const output = response.files.map(async file => ({ - url: file.url, - data: await this.#limiter(file.content), - })); - return { response: await Promise.all(output), etag: response.etag }; - } - - const data = await this.options.reader.readUrl(location, { etag }); - return { - response: [{ url: location, data: await data.buffer() }], - etag: data.etag, - }; + return { response: await Promise.all(output), etag: response.etag }; } } diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index c9d75298c8..e50aa84d80 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -356,7 +356,7 @@ export class CatalogBuilder { return [ new FileReaderProcessor(), - new UrlReaderProcessor({ reader, logger, config }), + new UrlReaderProcessor({ reader, logger }), new AnnotateLocationEntityProcessor({ integrations }), ]; } diff --git a/yarn.lock b/yarn.lock index 6d4aa9e768..4081495be9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6135,7 +6135,6 @@ __metadata: "@backstage/plugin-permission-common": "workspace:^" "@backstage/plugin-permission-node": "workspace:^" "@backstage/repo-tools": "workspace:^" - "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" "@opentelemetry/api": "npm:^1.9.0" "@types/core-js": "npm:^2.5.4"