diff --git a/.changeset/afraid-rockets-yell.md b/.changeset/afraid-rockets-yell.md new file mode 100644 index 0000000000..363c4dcd9f --- /dev/null +++ b/.changeset/afraid-rockets-yell.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Fix error handling in `LdapOrgReaderProcessor`, and support complex paging options diff --git a/plugins/catalog-backend/src/ingestion/processors/LdapOrgReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/LdapOrgReaderProcessor.ts index d863142169..474e813590 100644 --- a/plugins/catalog-backend/src/ingestion/processors/LdapOrgReaderProcessor.ts +++ b/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, diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/client.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/client.ts index bf0d0f740e..0557f21003 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/client.ts +++ b/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 | undefined; - static async create(target: string, bind?: BindConfig): Promise { + static async create( + logger: Logger, + target: string, + bind?: BindConfig, + ): Promise { 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); } @@ -92,7 +105,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}`); } } diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts index 607581e153..755c173d19 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts +++ b/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: { diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts index aca02e6e10..417657b244 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts +++ b/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(); }