Merge pull request #3483 from backstage/freben/fix-no-codeowners

catalog-backend: gracefully handle missing codeowners

.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.

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 };
     };

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<string | undefined> {
-  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<string | undefined> {
   const readOwnerLocation = async (basePath: string): Promise<string> => {
     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(

plugins/catalog-backend/src/service/CatalogBuilder.ts

@@ -285,7 +285,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(),
       );