chore: address comments

packages/catalog-model/src/entity/ref.ts

@@ -173,7 +173,7 @@ export function serializeEntityRef(
   let namespace;
   let name;
 
-  if ('apiVersion' in ref) {
+  if ('metadata' in ref) {
     kind = ref.kind;
     namespace = ref.metadata.namespace;
     name = ref.metadata.name;

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

@@ -164,7 +164,7 @@ export class HigherOrderOperations implements HigherOrderOperation {
       `Read ${readerOutput.entities.length} entities from location ${location.type} ${location.target}`,
     );
 
-    const startTimestamp = Date.now();
+    const startTimestamp = process.hrtime();
     for (const item of readerOutput.entities) {
       const { entity } = item;
 
@@ -196,9 +196,10 @@ export class HigherOrderOperations implements HigherOrderOperation {
       }
     }
 
-    const duration = ((Date.now() - startTimestamp) / 1000).toFixed(1);
+    const delta = process.hrtime(startTimestamp);
+    const durationMs = ((delta[0] * 1e9 + delta[1]) / 1e6).toFixed(1);
     this.logger.info(
-      `Wrote ${readerOutput.entities.length} entities from location ${location.type} ${location.target} in ${duration} seconds`,
+      `Wrote ${readerOutput.entities.length} entities from location ${location.type} ${location.target} in ${durationMs} seconds`,
     );
   }
 }

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

@@ -121,8 +121,8 @@ export class LocationReaders implements LocationReader {
       StaticLocationProcessor.fromConfig(config),
       new FileReaderProcessor(),
       ...oldProcessors,
-      GithubOrgReaderProcessor.fromConfig(config, logger),
-      LdapOrgReaderProcessor.fromConfig(config, logger),
+      GithubOrgReaderProcessor.fromConfig(config, { logger }),
+      LdapOrgReaderProcessor.fromConfig(config, { logger }),
       new UrlReaderProcessor(options),
       new YamlProcessor(),
       PlaceholderProcessor.default(),

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

@@ -95,15 +95,15 @@ describe('GithubOrgReaderProcessor', () => {
 
   describe('implementation', () => {
     it('rejects unknown types', async () => {
-      const processor = new GithubOrgReaderProcessor(
-        [
+      const processor = new GithubOrgReaderProcessor({
+        providers: [
           {
             target: 'https://github.com',
             apiBaseUrl: 'https://api.github.com',
           },
         ],
-        getVoidLogger(),
-      );
+        logger: getVoidLogger(),
+      });
       const location: LocationSpec = {
         type: 'not-github-org',
         target: 'https://github.com',
@@ -114,15 +114,15 @@ describe('GithubOrgReaderProcessor', () => {
     });
 
     it('rejects unknown targets', async () => {
-      const processor = new GithubOrgReaderProcessor(
-        [
+      const processor = new GithubOrgReaderProcessor({
+        providers: [
           {
             target: 'https://github.com',
             apiBaseUrl: 'https://api.github.com',
           },
         ],
-        getVoidLogger(),
-      );
+        logger: getVoidLogger(),
+      });
       const location: LocationSpec = {
         type: 'github-org',
         target: 'https://not.github.com/apa',

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

@@ -27,14 +27,20 @@ import { buildOrgHierarchy } from './util/org';
  * Extracts teams and users out of a GitHub org.
  */
 export class GithubOrgReaderProcessor implements LocationProcessor {
-  static fromConfig(config: Config, logger: Logger) {
-    return new GithubOrgReaderProcessor(readConfig(config), logger);
+  private readonly providers: ProviderConfig[];
+  private readonly logger: Logger;
+
+  static fromConfig(config: Config, options: { logger: Logger }) {
+    return new GithubOrgReaderProcessor({
+      ...options,
+      providers: readConfig(config),
+    });
   }
 
-  constructor(
-    private readonly providers: ProviderConfig[],
-    private readonly logger: Logger,
-  ) {}
+  constructor(options: { providers: ProviderConfig[]; logger: Logger }) {
+    this.providers = options.providers;
+    this.logger = options.logger;
+  }
 
   async readLocation(
     location: LocationSpec,

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

@@ -30,15 +30,21 @@ import { LocationProcessor, LocationProcessorEmit } from './types';
  * Extracts teams and users out of an LDAP server.
  */
 export class LdapOrgReaderProcessor implements LocationProcessor {
-  static fromConfig(config: Config, logger: Logger) {
+  private readonly providers: LdapProviderConfig[];
+  private readonly logger: Logger;
+
+  static fromConfig(config: Config, options: { logger: Logger }) {
     const c = config.getOptionalConfig('catalog.processors.ldapOrg');
-    return new LdapOrgReaderProcessor(c ? readLdapConfig(c) : [], logger);
+    return new LdapOrgReaderProcessor({
+      ...options,
+      providers: c ? readLdapConfig(c) : [],
+    });
   }
 
-  constructor(
-    private readonly providers: LdapProviderConfig[],
-    private readonly logger: Logger,
-  ) {}
+  constructor(options: { providers: LdapProviderConfig[]; logger: Logger }) {
+    this.providers = options.providers;
+    this.logger = options.logger;
+  }
 
   async readLocation(
     location: LocationSpec,

plugins/catalog-backend/src/ingestion/processors/ldap/client.ts

@@ -24,14 +24,13 @@ import { errorString } from './util';
  * Helps out with promisifying calls, paging, binding etc.
  */
 export class LdapClient {
-  static create(target: string, bind?: BindConfig): Promise<LdapClient> {
-    return new Promise<LdapClient>((resolve, reject) => {
-      const client = ldap.createClient({ url: target });
-      if (!bind) {
-        resolve(new LdapClient(client));
-        return;
-      }
+  static async create(target: string, bind?: BindConfig): Promise<LdapClient> {
+    const client = ldap.createClient({ url: target });
+    if (!bind) {
+      return new LdapClient(client);
+    }
 
+    return new Promise<LdapClient>((resolve, reject) => {
       const { dn, secret } = bind;
       client.bind(dn, secret, err => {
         if (err) {
@@ -58,12 +57,12 @@ export class LdapClient {
 
         this.client.search(dn, options, (err, res) => {
           if (err) {
-            reject(errorString(err));
+            reject(new Error(errorString(err)));
             return;
           }
 
           res.on('searchReference', () => {
-            reject('Unable to handle referral');
+            reject(new Error('Unable to handle referral'));
           });
 
           res.on('searchEntry', entry => {
@@ -71,14 +70,14 @@ export class LdapClient {
           });
 
           res.on('error', e => {
-            reject(errorString(e));
+            reject(new Error(errorString(e)));
           });
 
           res.on('end', r => {
             if (!r) {
-              reject('Null response');
+              reject(new Error('Null response'));
             } else if (r.status !== 0) {
-              reject(`Got status ${r.status}: ${r.errorMessage}`);
+              reject(new Error(`Got status ${r.status}: ${r.errorMessage}`));
             } else {
               resolve(output);
             }
@@ -86,7 +85,7 @@ export class LdapClient {
         });
       });
     } catch (e) {
-      throw new Error(`LDAP search at ${dn} failed, ${e}`);
+      throw new Error(`LDAP search at ${dn} failed, ${e.message}`);
     }
   }
 }