fix(catalog-backend): address comments and add docs

plugins/catalog-backend/src/ingestion/LocationReaders.ts

@@ -66,17 +66,19 @@ export class LocationReaders implements LocationReader {
   private readonly rulesEnforcer: CatalogRulesEnforcer;
 
   static defaultProcessors(options: {
+    logger: Logger;
     config?: Config;
     entityPolicy?: EntityPolicy;
   }): LocationProcessor[] {
     const {
+      logger,
       config = new ConfigReader({}, 'missing-config'),
       entityPolicy = new EntityPolicies(),
     } = options;
     return [
       StaticLocationProcessor.fromConfig(config),
       new FileReaderProcessor(),
-      GithubReaderProcessor.fromConfig(config),
+      GithubReaderProcessor.fromConfig(config, logger),
       new GitlabApiReaderProcessor(config),
       new GitlabReaderProcessor(),
       new BitbucketApiReaderProcessor(config),
@@ -92,7 +94,7 @@ export class LocationReaders implements LocationReader {
   constructor({
     logger = getVoidLogger(),
     config,
-    processors = LocationReaders.defaultProcessors({ config }),
+    processors = LocationReaders.defaultProcessors({ logger, config }),
   }: Options) {
     this.logger = logger;
     this.processors = processors;

plugins/catalog-backend/src/ingestion/processors/GithubReaderProcessor.test.ts

@@ -14,13 +14,14 @@
  * limitations under the License.
  */
 
+import { getVoidLogger } from '@backstage/backend-common';
 import { LocationSpec } from '@backstage/catalog-model';
 import { ConfigReader } from '@backstage/config';
 import {
-  getApiUrl,
   getApiRequestOptions,
-  getRawUrl,
+  getApiUrl,
   getRawRequestOptions,
+  getRawUrl,
   GithubReaderProcessor,
   ProviderConfig,
   readConfig,
@@ -179,7 +180,7 @@ describe('GithubReaderProcessor', () => {
     }
 
     it('adds a default GitHub entry when missing', () => {
-      const output = readConfig(config([]));
+      const output = readConfig(config([]), getVoidLogger());
       expect(output).toEqual([
         {
           target: 'https://github.com',
@@ -190,7 +191,10 @@ describe('GithubReaderProcessor', () => {
     });
 
     it('injects the correct GitHub API base URL when missing', () => {
-      const output = readConfig(config([{ target: 'https://github.com' }]));
+      const output = readConfig(
+        config([{ target: 'https://github.com' }]),
+        getVoidLogger(),
+      );
       expect(output).toEqual([
         {
           target: 'https://github.com',
@@ -202,26 +206,33 @@ describe('GithubReaderProcessor', () => {
 
     it('rejects custom targets with no base URLs', () => {
       expect(() =>
-        readConfig(config([{ target: 'https://ghe.company.com' }])),
+        readConfig(
+          config([{ target: 'https://ghe.company.com' }]),
+          getVoidLogger(),
+        ),
       ).toThrow(
         'Provider at https://ghe.company.com must configure an explicit apiBaseUrl or rawBaseUrl',
       );
     });
 
     it('rejects funky configs', () => {
-      expect(() => readConfig(config([{ target: 7 } as any]))).toThrow(
-        /target/,
-      );
-      expect(() => readConfig(config([{ noTarget: '7' } as any]))).toThrow(
-        /target/,
-      );
+      expect(() =>
+        readConfig(config([{ target: 7 } as any]), getVoidLogger()),
+      ).toThrow(/target/);
+      expect(() =>
+        readConfig(config([{ noTarget: '7' } as any]), getVoidLogger()),
+      ).toThrow(/target/);
       expect(() =>
         readConfig(
           config([{ target: 'https://github.com', apiBaseUrl: 7 } as any]),
+          getVoidLogger(),
         ),
       ).toThrow(/apiBaseUrl/);
       expect(() =>
-        readConfig(config([{ target: 'https://github.com', token: 7 } as any])),
+        readConfig(
+          config([{ target: 'https://github.com', token: 7 } as any]),
+          getVoidLogger(),
+        ),
       ).toThrow(/token/);
     });
   });

plugins/catalog-backend/src/ingestion/processors/GithubReaderProcessor.ts

@@ -18,6 +18,7 @@ import { LocationSpec } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
 import parseGitUri from 'git-url-parse';
 import fetch, { HeadersInit, RequestInit } from 'node-fetch';
+import { Logger } from 'winston';
 import * as result from './results';
 import { LocationProcessor, LocationProcessorEmit } from './types';
 
@@ -139,9 +140,16 @@ export function getRawUrl(target: string, provider: ProviderConfig): URL {
   }
 }
 
-export function readConfig(config: Config): ProviderConfig[] {
+export function readConfig(config: Config, logger: Logger): ProviderConfig[] {
   const providers: ProviderConfig[] = [];
 
+  // TODO(freben): Deprecate the old config root entirely in a later release
+  if (config.has('catalog.processors.githubApi')) {
+    logger.warn(
+      'The catalog.processors.githubApi configuration key has been deprecated, please use catalog.processors.github instead',
+    );
+  }
+
   // In a previous version of the configuration, we only supported github,
   // and the "privateToken" key held the token to use for it. The new
   // configuration method is to use the "providers" key instead.
@@ -202,8 +210,8 @@ export function readConfig(config: Config): ProviderConfig[] {
 export class GithubReaderProcessor implements LocationProcessor {
   private providers: ProviderConfig[];
 
-  static fromConfig(config: Config) {
-    return new GithubReaderProcessor(readConfig(config));
+  static fromConfig(config: Config, logger: Logger) {
+    return new GithubReaderProcessor(readConfig(config, logger));
   }
 
   constructor(providers: ProviderConfig[]) {
@@ -238,7 +246,6 @@ export class GithubReaderProcessor implements LocationProcessor {
       const options = useApi
         ? getApiRequestOptions(provider)
         : getRawRequestOptions(provider);
-      console.log(url.toString());
       const response = await fetch(url.toString(), options);
 
       if (response.ok) {