Fix error handling in LdapOrgReaderProcessor, and support complex paging options

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/afraid-rockets-yell.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Fix error handling in `LdapOrgReaderProcessor`, and support complex paging options

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

@@ -69,7 +69,11 @@ export class LdapOrgReaderProcessor implements CatalogProcessor {
     // Be lazy and create the client each time; even though it's pretty
     // inefficient, we usually only do this once per entire refresh loop and
     // don't have to worry about timeouts and reconnects etc.
-    const client = await LdapClient.create(provider.target, provider.bind);
+    const client = await LdapClient.create(
+      this.logger,
+      provider.target,
+      provider.bind,
+    );
     const { users, groups } = await readLdapOrg(
       client,
       provider.users,

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

@@ -15,6 +15,7 @@
  */
 
 import ldap, { Client, SearchEntry, SearchOptions } from 'ldapjs';
+import { Logger } from 'winston';
 import { BindConfig } from './config';
 import { errorString } from './util';
 import {
@@ -31,8 +32,20 @@ import {
 export class LdapClient {
   private vendor: Promise<LdapVendor> | undefined;
 
-  static async create(target: string, bind?: BindConfig): Promise<LdapClient> {
+  static async create(
+    logger: Logger,
+    target: string,
+    bind?: BindConfig,
+  ): Promise<LdapClient> {
     const client = ldap.createClient({ url: target });
+
+    // We want to have a catch-all error handler at the top, since the default
+    // behavior of the client is to blow up the entire process when it fails,
+    // unless an error handler is set.
+    client.on('error', (err: ldap.Error) => {
+      logger.warn(`LDAP client threw an error, ${errorString(err)}`);
+    });
+
     if (!bind) {
       return new LdapClient(client);
     }
@@ -62,6 +75,10 @@ export class LdapClient {
       return await new Promise<SearchEntry[]>((resolve, reject) => {
         const output: SearchEntry[] = [];
 
+        this.client.on('error', (err: ldap.Error) => {
+          reject(new Error(errorString(err)));
+        });
+
         this.client.search(dn, options, (err, res) => {
           if (err) {
             reject(new Error(errorString(err)));
@@ -92,7 +109,7 @@ export class LdapClient {
         });
       });
     } catch (e) {
-      throw new Error(`LDAP search at ${dn} failed, ${e.message}`);
+      throw new Error(`LDAP search at DN "${dn}" failed, ${e.message}`);
     }
   }
 

plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts

@@ -105,7 +105,10 @@ describe('readLdapConfig', () => {
               scope: 'base',
               attributes: ['*'],
               filter: 'f',
-              paged: true,
+              paged: {
+                pageSize: 7,
+                pagePause: true,
+              },
             },
             set: { p: 'v' },
             map: {
@@ -153,7 +156,10 @@ describe('readLdapConfig', () => {
             scope: 'base',
             attributes: ['*'],
             filter: 'f',
-            paged: true,
+            paged: {
+              pageSize: 7,
+              pagePause: true,
+            },
           },
           set: { p: 'v' },
           map: {

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

@@ -180,11 +180,32 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
     if (!c) {
       return {};
     }
+
+    const paged = readOptionsPagedConfig(c);
+
     return {
       scope: c.getOptionalString('scope') as SearchOptions['scope'],
       filter: formatFilter(c.getOptionalString('filter')),
       attributes: c.getOptionalStringArray('attributes'),
-      paged: c.getOptionalBoolean('paged'),
+      ...(paged !== undefined ? { paged } : undefined),
+    };
+  }
+
+  function readOptionsPagedConfig(c: Config): SearchOptions['paged'] {
+    const pagedConfig = c.getOptional('paged');
+    if (pagedConfig === undefined) {
+      return undefined;
+    }
+
+    if (pagedConfig === true || pagedConfig === false) {
+      return pagedConfig;
+    }
+
+    const pageSize = c.getOptionalNumber('paged.pageSize');
+    const pagePause = c.getOptionalBoolean('paged.pagePause');
+    return {
+      ...(pageSize !== undefined ? { pageSize } : undefined),
+      ...(pagePause !== undefined ? { pagePause } : undefined),
     };
   }
 
@@ -258,7 +279,7 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
   }
 
   function formatFilter(filter?: string): string | undefined {
-    // Remove extra whitespaces between blocks to support multiline filters from the configuration
+    // Remove extra whitespace between blocks to support multiline filters from the configuration
     return filter?.replace(/\s*(\(|\))/g, '$1')?.trim();
   }