From 9461f73643f9b5cee593420505bfeacd8de3009d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 5 Mar 2022 16:58:28 +0100 Subject: [PATCH] try a convenience thing for scheduling providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/new-books-protect.md | 30 ++++ .changeset/silent-cats-kneel.md | 2 +- .changeset/ten-queens-dance.md | 7 + docs/integrations/ldap/org.md | 68 +++------- packages/backend-tasks/api-report.md | 34 +++-- .../src/tasks/PluginTaskSchedulerImpl.test.ts | 27 ++++ .../src/tasks/PluginTaskSchedulerImpl.ts | 19 ++- packages/backend-tasks/src/tasks/index.ts | 4 +- packages/backend-tasks/src/tasks/types.ts | 81 ++++++++--- .../catalog-backend-module-ldap/api-report.md | 21 +-- .../catalog-backend-module-ldap/package.json | 2 + .../src/processors/LdapOrgEntityProvider.ts | 128 +++++++++++++----- .../src/processors/index.ts | 1 + 13 files changed, 301 insertions(+), 123 deletions(-) create mode 100644 .changeset/new-books-protect.md create mode 100644 .changeset/ten-queens-dance.md diff --git a/.changeset/new-books-protect.md b/.changeset/new-books-protect.md new file mode 100644 index 0000000000..2a561b5e5c --- /dev/null +++ b/.changeset/new-books-protect.md @@ -0,0 +1,30 @@ +--- +'@backstage/plugin-catalog-backend-module-ldap': minor +--- + +**BREAKING**: Added a `schedule` field to `LdapOrgEntityProvider.fromConfig`, which is required. If you want to retain the old behavior of scheduling the provider manually, you can set it to the string value `'manual'`. But you may want to leverage the ability to instead pass in the recurring task schedule information directly. This will allow you to simplify your backend setup code to not need an intermediate variable and separate scheduling code at the bottom. + +All things said, a typical setup might now look as follows: + +```diff + // packages/backend/src/plugins/catalog.ts ++import { Duration } from 'luxon'; ++import { LdapOrgEntityProvider } from '@backstage/plugin-catalog-backend-module-ldap'; + export default async function createPlugin( + env: PluginEnvironment, + ): Promise { + const builder = await CatalogBuilder.create(env); ++ // The target parameter below needs to match the ldap.providers.target ++ // value specified in your app-config. ++ builder.addEntityProvider( ++ LdapOrgEntityProvider.fromConfig(env.config, { ++ id: 'our-ldap-master', ++ target: 'ldaps://ds.example.net', ++ logger: env.logger, ++ schedule: env.scheduler.createTaskSchedule({ ++ frequency: Duration.fromObject({ minutes: 60 }), ++ timeout: Duration.fromObject({ minutes: 15 }), ++ }), ++ }), ++ ); +``` diff --git a/.changeset/silent-cats-kneel.md b/.changeset/silent-cats-kneel.md index f34fc98afc..9bd9cce0e0 100644 --- a/.changeset/silent-cats-kneel.md +++ b/.changeset/silent-cats-kneel.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend-module-gitlab': minor --- -Added package, moving out gitlab specific functionality from the catalog-backend +Added package, moving out GitLab specific functionality from the catalog-backend diff --git a/.changeset/ten-queens-dance.md b/.changeset/ten-queens-dance.md new file mode 100644 index 0000000000..947675aa46 --- /dev/null +++ b/.changeset/ten-queens-dance.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-tasks': minor +--- + +**BREAKING**: The `TaskDefinition` type has been removed, and replaced by the equal pair `TaskScheduleDefinition` and `TaskInvocationDefinition`. The interface for `PluginTaskScheduler.scheduleTask` stays effectively unchanged, so this only affects you if you use the actual types directly. + +Added the method `PluginTaskScheduler.createTaskSchedule`, which returns a `TaskSchedule` wrapper that is convenient to pass down into classes that want to control their task invocations while the caller wants to retain control of the actual schedule chosen. diff --git a/docs/integrations/ldap/org.md b/docs/integrations/ldap/org.md index 645524493b..b0fa2ca76d 100644 --- a/docs/integrations/ldap/org.md +++ b/docs/integrations/ldap/org.md @@ -32,55 +32,29 @@ yarn add @backstage/plugin-catalog-backend-module-ldap Update the catalog plugin initialization in your backend to add the provider and schedule it: -```ts -// packages/backend/src/plugins/catalog.ts -import { CatalogBuilder } from '@backstage/plugin-catalog-backend'; -import { Router } from 'express'; -import { PluginEnvironment } from '../types'; -import { Duration } from 'luxon'; -import { LdapOrgEntityProvider } from '@backstage/plugin-catalog-backend-module-ldap'; +```diff + // packages/backend/src/plugins/catalog.ts ++import { Duration } from 'luxon'; ++import { LdapOrgEntityProvider } from '@backstage/plugin-catalog-backend-module-ldap'; -export default async function createPlugin( - env: PluginEnvironment, -): Promise { - // The target parameter below needs to match the ldap.providers.target - // value specified in your app-config - const ldapEntityProvider = LdapOrgEntityProvider.fromConfig(env.config, { - id: 'our-ldap-master', - target: 'ldaps://ds.example.net', - logger: env.logger, - }); + export default async function createPlugin( + env: PluginEnvironment, + ): Promise { + const builder = await CatalogBuilder.create(env); - const builder = await CatalogBuilder.create(env); - builder.addEntityProvider(ldapEntityProvider); - - // You can change the refresh interval for the other catalog entries - // independently, or just leave the line below out to use the default - // refresh interval. Note that this interval does NOT at all affect - // the LDAP refresh when using the provider method, which is good! - builder.setRefreshIntervalSeconds(100); - - const { processingEngine, router } = await builder.build(); - await processingEngine.start(); - - // Only perform this scheduling after starting the processing engine - await env.scheduler.scheduleTask({ - id: 'refresh_ldap', - // frequency sets how often you want to ingest users and groups from - // LDAP, in this case every 60 minutes - frequency: Duration.fromObject({ minutes: 60 }), - timeout: Duration.fromObject({ minutes: 15 }), - fn: async () => { - try { - await ldapEntityProvider.read(); - } catch (error) { - env.logger.error(error); - } - }, - }); - - return router; -} ++ // The target parameter below needs to match the ldap.providers.target ++ // value specified in your app-config. ++ builder.addEntityProvider( ++ LdapOrgEntityProvider.fromConfig(env.config, { ++ id: 'our-ldap-master', ++ target: 'ldaps://ds.example.net', ++ logger: env.logger, ++ schedule: env.scheduler.createTaskSchedule({ ++ frequency: Duration.fromObject({ minutes: 60 }), ++ timeout: Duration.fromObject({ minutes: 15 }), ++ }), ++ }), ++ ); ``` After this, you also have to add some configuration in your app-config that diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 82b2ca9b0c..db4dee0c65 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -11,17 +11,10 @@ import { Logger } from 'winston'; // @public export interface PluginTaskScheduler { - scheduleTask(task: TaskDefinition): Promise; -} - -// @public -export interface TaskDefinition { - fn: TaskFunction; - frequency: Duration; - id: string; - initialDelay?: Duration; - signal?: AbortSignal_2; - timeout: Duration; + createTaskSchedule(schedule: TaskScheduleDefinition): TaskSchedule; + scheduleTask( + task: TaskScheduleDefinition & TaskInvocationDefinition, + ): Promise; } // @public @@ -29,6 +22,25 @@ export type TaskFunction = | ((abortSignal: AbortSignal_2) => void | Promise) | (() => void | Promise); +// @public +export interface TaskInvocationDefinition { + fn: TaskFunction; + id: string; + signal?: AbortSignal_2; +} + +// @public +export interface TaskSchedule { + run(task: TaskInvocationDefinition): Promise; +} + +// @public +export interface TaskScheduleDefinition { + frequency: Duration; + initialDelay?: Duration; + timeout: Duration; +} + // @public export class TaskScheduler { constructor(databaseManager: DatabaseManager, logger: Logger); diff --git a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts index e387b85413..0a811a5b88 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts @@ -59,4 +59,31 @@ describe('PluginTaskManagerImpl', () => { 60_000, ); }); + + // This is just to test the wrapper code; most of the actual tests are in + // TaskWorker.test.ts + describe('createTaskSchedule', () => { + it.each(databases.eachSupportedId())( + 'can run the happy path, %p', + async databaseId => { + const { manager } = await init(databaseId); + + const fn = jest.fn(); + await manager + .createTaskSchedule({ + timeout: Duration.fromMillis(5000), + frequency: Duration.fromMillis(5000), + }) + .run({ + id: 'task1', + fn, + }); + + await waitForExpect(() => { + expect(fn).toBeCalled(); + }); + }, + 60_000, + ); + }); }); diff --git a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts index 93975bc327..b410e4d3b5 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts @@ -17,7 +17,12 @@ import { Knex } from 'knex'; import { Logger } from 'winston'; import { TaskWorker } from './TaskWorker'; -import { PluginTaskScheduler, TaskDefinition } from './types'; +import { + PluginTaskScheduler, + TaskInvocationDefinition, + TaskSchedule, + TaskScheduleDefinition, +} from './types'; import { validateId } from './util'; /** @@ -29,7 +34,9 @@ export class PluginTaskSchedulerImpl implements PluginTaskScheduler { private readonly logger: Logger, ) {} - async scheduleTask(task: TaskDefinition): Promise { + async scheduleTask( + task: TaskScheduleDefinition & TaskInvocationDefinition, + ): Promise { validateId(task.id); const knex = await this.databaseFactory(); @@ -47,4 +54,12 @@ export class PluginTaskSchedulerImpl implements PluginTaskScheduler { }, ); } + + createTaskSchedule(schedule: TaskScheduleDefinition): TaskSchedule { + return { + run: async task => { + await this.scheduleTask({ ...task, ...schedule }); + }, + }; + } } diff --git a/packages/backend-tasks/src/tasks/index.ts b/packages/backend-tasks/src/tasks/index.ts index 9e0a06f71c..d925f089e8 100644 --- a/packages/backend-tasks/src/tasks/index.ts +++ b/packages/backend-tasks/src/tasks/index.ts @@ -17,6 +17,8 @@ export { TaskScheduler } from './TaskScheduler'; export type { PluginTaskScheduler, - TaskDefinition, TaskFunction, + TaskInvocationDefinition, + TaskSchedule, + TaskScheduleDefinition, } from './types'; diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index 4693af7ef3..3e46acf55e 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -31,27 +31,11 @@ export type TaskFunction = | (() => void | Promise); /** - * Options that apply to the invocation of a given task. + * Options that control the scheduling of a task. * * @public */ -export interface TaskDefinition { - /** - * A unique ID (within the scope of the plugin) for the task. - */ - id: string; - - /** - * The actual task function to be invoked regularly. - */ - fn: TaskFunction; - - /** - * An abort signal that, when triggered, will stop the recurring execution of - * the task. - */ - signal?: AbortSignal; - +export interface TaskScheduleDefinition { /** * The maximum amount of time that a single task invocation can take, before * it's considered timed out and gets "released" such that a new invocation @@ -91,6 +75,43 @@ export interface TaskDefinition { initialDelay?: Duration; } +/** + * Options that apply to the invocation of a given task. + * + * @public + */ +export interface TaskInvocationDefinition { + /** + * A unique ID (within the scope of the plugin) for the task. + */ + id: string; + + /** + * The actual task function to be invoked regularly. + */ + fn: TaskFunction; + + /** + * An abort signal that, when triggered, will stop the recurring execution of + * the task. + */ + signal?: AbortSignal; +} + +/** + * A previously prepared task schedule, ready to be invoked. + * + * @public + */ +export interface TaskSchedule { + /** + * Takes the schedule and executes an actual task using it. + * + * @param task - The actual runtime properties of the task + */ + run(task: TaskInvocationDefinition): Promise; +} + /** * Deals with the scheduling of distributed tasks, for a given plugin. * @@ -99,15 +120,33 @@ export interface TaskDefinition { export interface PluginTaskScheduler { /** * Schedules a task function for coordinated exclusive invocation across - * workers. + * workers. This convenience method performs both the scheduling and + * invocation in one go. + * + * @remarks * * If the task was already scheduled since before by us or by another party, * its options are just overwritten with the given options, and things * continue from there. * - * @param definition - The task definition + * @param task - The task definition */ - scheduleTask(task: TaskDefinition): Promise; + scheduleTask( + task: TaskScheduleDefinition & TaskInvocationDefinition, + ): Promise; + + /** + * Creates a task schedule, ready to be invoked at a later time. + * + * @remarks + * + * This method is useful for pre-creating a schedule in outer code to be + * passed into an inner implementation, such that the outer code controls + * scheduling while inner code controls implementation. + * + * @param schedule - The task schedule + */ + createTaskSchedule(schedule: TaskScheduleDefinition): TaskSchedule; } function isValidOptionalDurationString(d: string | undefined): boolean { diff --git a/plugins/catalog-backend-module-ldap/api-report.md b/plugins/catalog-backend-module-ldap/api-report.md index 79646aab01..25f479f73c 100644 --- a/plugins/catalog-backend-module-ldap/api-report.md +++ b/plugins/catalog-backend-module-ldap/api-report.md @@ -15,6 +15,7 @@ import { LocationSpec } from '@backstage/plugin-catalog-backend'; import { Logger } from 'winston'; import { SearchEntry } from 'ldapjs'; import { SearchOptions } from 'ldapjs'; +import { TaskSchedule } from '@backstage/backend-tasks'; import { UserEntity } from '@backstage/catalog-model'; // @public @@ -106,17 +107,21 @@ export class LdapOrgEntityProvider implements EntityProvider { // (undocumented) static fromConfig( configRoot: Config, - options: { - id: string; - target: string; - userTransformer?: UserTransformer; - groupTransformer?: GroupTransformer; - logger: Logger; - }, + options: LdapOrgEntityProviderOptions, ): LdapOrgEntityProvider; // (undocumented) getProviderName(): string; - read(): Promise; + read(options?: { logger?: Logger }): Promise; +} + +// @public +export interface LdapOrgEntityProviderOptions { + groupTransformer?: GroupTransformer; + id: string; + logger: Logger; + schedule: 'manual' | TaskSchedule; + target: string; + userTransformer?: UserTransformer; } // @public diff --git a/plugins/catalog-backend-module-ldap/package.json b/plugins/catalog-backend-module-ldap/package.json index 8f05dceb4e..d1c9e764fd 100644 --- a/plugins/catalog-backend-module-ldap/package.json +++ b/plugins/catalog-backend-module-ldap/package.json @@ -33,6 +33,7 @@ "start": "backstage-cli package start" }, "dependencies": { + "@backstage/backend-tasks": "^0.1.10", "@backstage/catalog-model": "^0.12.0", "@backstage/config": "^0.1.15", "@backstage/errors": "^0.2.2", @@ -41,6 +42,7 @@ "@types/ldapjs": "^2.2.0", "ldapjs": "^2.2.0", "lodash": "^4.17.21", + "uuid": "^8.0.0", "winston": "^3.2.1" }, "devDependencies": { diff --git a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts index 4c5bd2e3bd..9c483c7b94 100644 --- a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { TaskSchedule } from '@backstage/backend-tasks'; import { ANNOTATION_LOCATION, ANNOTATION_ORIGIN_LOCATION, @@ -25,6 +26,7 @@ import { EntityProviderConnection, } from '@backstage/plugin-catalog-backend'; import { merge } from 'lodash'; +import * as uuid from 'uuid'; import { Logger } from 'winston'; import { GroupTransformer, @@ -36,6 +38,54 @@ import { UserTransformer, } from '../ldap'; +/** + * Options for {@link LdapOrgEntityProvider}. + * + * @public + */ +export interface LdapOrgEntityProviderOptions { + /** + * A unique, stable identifier for this provider. + * + * @example "production" + */ + id: string; + + /** + * The target that this provider should consume. + * + * Should exactly match the "target" field of one of the "ldap.providers" + * configuration entries. + * + * @example "ldaps://ds-read.example.net" + */ + target: string; + + /** + * The logger to use. + */ + logger: Logger; + + /** + * The refresh schedule to use. + * + * If you pass in 'manual', you are responsible for calling the `read` + * method manually at some interval. If not, it will be automatically + * called regularly with the given schedule using the scheduler. + */ + schedule: 'manual' | TaskSchedule; + + /** + * The function that transforms a user entry in LDAP to an entity. + */ + userTransformer?: UserTransformer; + + /** + * The function that transforms a group entry in LDAP to an entity. + */ + groupTransformer?: GroupTransformer; +} + /** * Reads user and group entries out of an LDAP service, and provides them as * User and Group entities for the catalog. @@ -49,35 +99,11 @@ import { */ export class LdapOrgEntityProvider implements EntityProvider { private connection?: EntityProviderConnection; + private scheduleFn?: () => Promise; static fromConfig( configRoot: Config, - options: { - /** - * A unique, stable identifier for this provider. - * - * @example "production" - */ - id: string; - /** - * The target that this provider should consume. - * - * Should exactly match the "target" field of one of the "ldap.providers" - * configuration entries. - * - * @example "ldaps://ds-read.example.net" - */ - target: string; - /** - * The function that transforms a user entry in LDAP to an entity. - */ - userTransformer?: UserTransformer; - /** - * The function that transforms a group entry in LDAP to an entity. - */ - groupTransformer?: GroupTransformer; - logger: Logger; - }, + options: LdapOrgEntityProviderOptions, ): LdapOrgEntityProvider { // TODO(freben): Deprecate the old catalog.processors.ldapOrg config const config = @@ -101,13 +127,17 @@ export class LdapOrgEntityProvider implements EntityProvider { target: options.target, }); - return new LdapOrgEntityProvider({ + const result = new LdapOrgEntityProvider({ id: options.id, provider, userTransformer: options.userTransformer, groupTransformer: options.groupTransformer, logger, }); + + result.schedule(options.schedule); + + return result; } constructor( @@ -128,18 +158,20 @@ export class LdapOrgEntityProvider implements EntityProvider { /** {@inheritdoc @backstage/plugin-catalog-backend#EntityProvider.connect} */ async connect(connection: EntityProviderConnection) { this.connection = connection; + await this.scheduleFn?.(); } /** - * Runs one complete ingestion loop. Call this method regularly at some - * appropriate cadence. + * Runs one single complete ingestion. This is only necessary if you use + * manual scheduling. */ - async read() { + async read(options?: { logger?: Logger }) { if (!this.connection) { throw new Error('Not initialized'); } - const { markReadComplete } = trackProgress(this.options.logger); + const logger = options?.logger ?? this.options.logger; + const { markReadComplete } = trackProgress(logger); // 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 @@ -157,7 +189,7 @@ export class LdapOrgEntityProvider implements EntityProvider { { groupTransformer: this.options.groupTransformer, userTransformer: this.options.userTransformer, - logger: this.options.logger, + logger, }, ); @@ -173,6 +205,38 @@ export class LdapOrgEntityProvider implements EntityProvider { markCommitComplete(); } + + private schedule(schedule: LdapOrgEntityProviderOptions['schedule']) { + if (schedule === 'manual') { + return; + } + + this.scheduleFn = async () => { + const id = this.getScheduledTaskId(); + await schedule.run({ + id, + fn: async () => { + const logger = this.options.logger.child({ + class: LdapOrgEntityProvider.prototype.constructor.name, + taskId: id, + taskInstanceId: uuid.v4(), + }); + + try { + await this.read({ logger }); + } catch (error) { + logger.error(error); + } + }, + }); + }; + } + + // Gets a suitable scheduler task ID for this provider instance + private getScheduledTaskId(): string { + const rawId = `refresh_${this.getProviderName()}`; + return rawId.toLocaleLowerCase('en-US').replace(/[^a-z0-9]/g, '_'); + } } // Helps wrap the timing and logging behaviors diff --git a/plugins/catalog-backend-module-ldap/src/processors/index.ts b/plugins/catalog-backend-module-ldap/src/processors/index.ts index 96e1a49cbb..5ed0095c3e 100644 --- a/plugins/catalog-backend-module-ldap/src/processors/index.ts +++ b/plugins/catalog-backend-module-ldap/src/processors/index.ts @@ -15,4 +15,5 @@ */ export { LdapOrgEntityProvider } from './LdapOrgEntityProvider'; +export type { LdapOrgEntityProviderOptions } from './LdapOrgEntityProvider'; export { LdapOrgReaderProcessor } from './LdapOrgReaderProcessor';