diff --git a/.changeset/sour-eels-dream.md b/.changeset/sour-eels-dream.md new file mode 100644 index 0000000000..75b9fb3c8d --- /dev/null +++ b/.changeset/sour-eels-dream.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Gracefully handle missing codeowners. + +The CodeOwnersProcessor now also takes a logger as a parameter. diff --git a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts index dfb470fa21..068a1e6e9a 100644 --- a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts @@ -16,6 +16,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { CodeOwnersEntry } from 'codeowners-utils'; +import { createLogger } from 'winston'; import { buildCodeOwnerUrl, buildUrl, @@ -27,6 +28,8 @@ import { resolveCodeOwner, } from './CodeOwnersProcessor'; +const logger = createLogger(); + describe('CodeOwnersProcessor', () => { const mockUrl = ({ basePath = '' } = {}): string => `https://github.com/backstage/backstage/blob/master/${basePath}catalog-info.yaml`; @@ -158,17 +161,20 @@ describe('CodeOwnersProcessor', () => { .fn() .mockResolvedValue(mockReadResult({ data: ownersText })); const reader = { read, readTree: jest.fn() }; - const result = await findRawCodeOwners(mockLocation(), reader); + const result = await findRawCodeOwners(mockLocation(), { + reader, + logger, + }); expect(result).toEqual(ownersText); }); - it('should raise error when no codeowner', async () => { + it('should return undefined when no codeowner', async () => { const read = jest.fn().mockRejectedValue(mockReadResult()); const reader = { read, readTree: jest.fn() }; await expect( - findRawCodeOwners(mockLocation(), reader), - ).rejects.toBeInstanceOf(Error); + findRawCodeOwners(mockLocation(), { reader, logger }), + ).resolves.toBeUndefined(); }); it('should look at known codeowner locations', async () => { @@ -180,7 +186,10 @@ describe('CodeOwnersProcessor', () => { .mockResolvedValue(mockReadResult({ data: ownersText })); const reader = { read, readTree: jest.fn() }; - const result = await findRawCodeOwners(mockLocation(), reader); + const result = await findRawCodeOwners(mockLocation(), { + reader, + logger, + }); expect(read.mock.calls.length).toBe(5); expect(read.mock.calls[0]).toEqual([mockReadUrl('')]); @@ -199,19 +208,19 @@ describe('CodeOwnersProcessor', () => { .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() })); const reader = { read, readTree: jest.fn() }; - const owner = await resolveCodeOwner(mockLocation(), reader); + const owner = await resolveCodeOwner(mockLocation(), { reader, logger }); expect(owner).toBe('backstage-core'); }); - it('should raise an error when no codeowner', async () => { + it('should return undefined when no codeowner', async () => { const read = jest .fn() .mockImplementation(() => mockReadResult({ error: 'error: foo' })); const reader = { read, readTree: jest.fn() }; await expect( - resolveCodeOwner(mockLocation(), reader), - ).rejects.toBeInstanceOf(Error); + resolveCodeOwner(mockLocation(), { reader, logger }), + ).resolves.toBeUndefined(); }); }); @@ -222,7 +231,7 @@ describe('CodeOwnersProcessor', () => { .fn() .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() })); const reader = { read, readTree: jest.fn() }; - const processor = new CodeOwnersProcessor({ reader }); + const processor = new CodeOwnersProcessor({ reader, logger }); return { entity, processor, read }; }; diff --git a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts index 8a10c4169b..e2b8a8b62c 100644 --- a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { UrlReader } from '@backstage/backend-common'; +import { NotFoundError, UrlReader } from '@backstage/backend-common'; import { Entity, LocationSpec } from '@backstage/catalog-model'; import * as codeowners from 'codeowners-utils'; import { CodeOwnersEntry } from 'codeowners-utils'; @@ -22,6 +22,7 @@ import { CodeOwnersEntry } from 'codeowners-utils'; import 'core-js/features/promise'; import parseGitUri from 'git-url-parse'; import { filter, get, head, pipe, reverse } from 'lodash/fp'; +import { Logger } from 'winston'; import { CatalogProcessor } from './types'; const ALLOWED_LOCATION_TYPES = [ @@ -41,6 +42,7 @@ const KNOWN_LOCATIONS = ['', '/docs', '/.bitbucket', '/.github', '/.gitlab']; type Options = { reader: UrlReader; + logger: Logger; }; export class CodeOwnersProcessor implements CatalogProcessor { @@ -60,7 +62,10 @@ export class CodeOwnersProcessor implements CatalogProcessor { return entity; } - const owner = await resolveCodeOwner(location, this.options.reader); + const owner = await resolveCodeOwner(location, this.options); + if (!owner) { + return entity; + } return { ...entity, @@ -71,12 +76,11 @@ export class CodeOwnersProcessor implements CatalogProcessor { export async function resolveCodeOwner( location: LocationSpec, - reader: UrlReader, + options: Options, ): Promise { - const ownersText = await findRawCodeOwners(location, reader); - + const ownersText = await findRawCodeOwners(location, options); if (!ownersText) { - throw Error(`Unable to find codeowners file for: ${location.target}`); + return undefined; } const owners = parseCodeOwners(ownersText); @@ -86,18 +90,33 @@ export async function resolveCodeOwner( export async function findRawCodeOwners( location: LocationSpec, - reader: UrlReader, + options: Options, ): Promise { const readOwnerLocation = async (basePath: string): Promise => { const ownerUrl = buildCodeOwnerUrl( location.target, `${basePath}/CODEOWNERS`, ); - const data = await reader.read(ownerUrl); + const data = await options.reader.read(ownerUrl); return data.toString(); }; - return Promise.any(KNOWN_LOCATIONS.map(readOwnerLocation)); + const candidates = KNOWN_LOCATIONS.map(readOwnerLocation); + return Promise.any(candidates).catch((aggregateError: AggregateError) => { + const hardError = aggregateError.errors.find( + error => !(error instanceof NotFoundError), + ); + if (hardError) { + options.logger.warn( + `Failed to read codeowners for location ${location.type}:${location.target}, ${hardError}`, + ); + } else { + options.logger.debug( + `Failed to find codeowners for location ${location.type}:${location.target}`, + ); + } + return undefined; + }); } export function buildCodeOwnerUrl( diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 4be0f1799b..7e1c4bb41e 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -280,7 +280,7 @@ export class CatalogBuilder { LdapOrgReaderProcessor.fromConfig(config, { logger }), MicrosoftGraphOrgReaderProcessor.fromConfig(config, { logger }), new UrlReaderProcessor({ reader, logger }), - new CodeOwnersProcessor({ reader }), + new CodeOwnersProcessor({ reader, logger }), new LocationRefProcessor(), new AnnotateLocationEntityProcessor(), );