diff --git a/.changeset/many-tools-buy.md b/.changeset/many-tools-buy.md new file mode 100644 index 0000000000..c2a617edfd --- /dev/null +++ b/.changeset/many-tools-buy.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-tasks': patch +--- + +Relaxed the task ID requirement to now support any non-empty string diff --git a/.changeset/new-books-protect.md b/.changeset/new-books-protect.md new file mode 100644 index 0000000000..568710ccd0 --- /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.createScheduledTaskRunner({ ++ 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..f7f4e25b1a 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.createScheduledTaskRunner({ ++ 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..98199a55b0 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; + createScheduledTaskRunner(schedule: TaskScheduleDefinition): TaskRunner; + 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 TaskRunner { + 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..993041bed4 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('createScheduledTaskRunner', () => { + it.each(databases.eachSupportedId())( + 'can run the happy path, %p', + async databaseId => { + const { manager } = await init(databaseId); + + const fn = jest.fn(); + await manager + .createScheduledTaskRunner({ + 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..8d50298ffd 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, + TaskRunner, + 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 { }, ); } + + createScheduledTaskRunner(schedule: TaskScheduleDefinition): TaskRunner { + 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..f6695a8d73 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, + TaskRunner, + TaskScheduleDefinition, } from './types'; diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index 4693af7ef3..35b5598700 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 TaskRunner { + /** + * 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,34 @@ 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 scheduled but dormant recurring task, ready to be launched 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 + */ + createScheduledTaskRunner(schedule: TaskScheduleDefinition): TaskRunner; } function isValidOptionalDurationString(d: string | undefined): boolean { diff --git a/packages/backend-tasks/src/tasks/util.test.ts b/packages/backend-tasks/src/tasks/util.test.ts index f74669a7f4..614de8c6c8 100644 --- a/packages/backend-tasks/src/tasks/util.test.ts +++ b/packages/backend-tasks/src/tasks/util.test.ts @@ -20,14 +20,14 @@ import { delegateAbortController, sleep, validateId } from './util'; describe('util', () => { describe('validateId', () => { - it.each(['a', 'a_b', 'ab123c_2'])( + it.each(['a', 'a_b', 'ab123c_2', 'a!', 'A', 'a-b', 'a.b', '_a', 'a_'])( 'accepts valid inputs, %p', async input => { expect(validateId(input)).toBeUndefined(); }, ); - it.each(['', 'a!', 'A', 'a-b', 'a.b', '_a', 'a_', null, Symbol('a')])( + it.each(['', null, Symbol('a')])( 'rejects invalid inputs, %p', async input => { expect(() => validateId(input as any)).toThrow(); diff --git a/packages/backend-tasks/src/tasks/util.ts b/packages/backend-tasks/src/tasks/util.ts index 8b247cdeb8..34280cc2d4 100644 --- a/packages/backend-tasks/src/tasks/util.ts +++ b/packages/backend-tasks/src/tasks/util.ts @@ -19,12 +19,10 @@ import { Knex } from 'knex'; import { DateTime, Duration } from 'luxon'; import { AbortController, AbortSignal } from 'node-abort-controller'; -// Keep the IDs compatible with e.g. Prometheus +// Keep the IDs compatible with e.g. Prometheus labels export function validateId(id: string) { - if (typeof id !== 'string' || !/^[a-z0-9]+(?:_[a-z0-9]+)*$/.test(id)) { - throw new InputError( - `${id} is not a valid ID, expected string of lowercase characters and digits separated by underscores`, - ); + if (typeof id !== 'string' || !id.trim()) { + throw new InputError(`${id} is not a valid ID, expected non-empty string`); } } diff --git a/plugins/catalog-backend-module-ldap/api-report.md b/plugins/catalog-backend-module-ldap/api-report.md index 79646aab01..edeef4cb82 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 { TaskRunner } 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' | TaskRunner; + 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..bad3941767 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 { TaskRunner } 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,59 @@ 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. + * + * @remarks + * + * If you pass in 'manual', you are responsible for calling the `read` method + * manually at some interval. + * + * But more commonly you will pass in the result of + * {@link @backstage/backend-tasks#PluginTaskScheduler.createScheduledTaskRunner} + * to enable automatic scheduling of tasks. + */ + schedule: 'manual' | TaskRunner; + + /** + * 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 +104,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 +132,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 +163,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 +194,7 @@ export class LdapOrgEntityProvider implements EntityProvider { { groupTransformer: this.options.groupTransformer, userTransformer: this.options.userTransformer, - logger: this.options.logger, + logger, }, ); @@ -173,6 +210,32 @@ export class LdapOrgEntityProvider implements EntityProvider { markCommitComplete(); } + + private schedule(schedule: LdapOrgEntityProviderOptions['schedule']) { + if (schedule === 'manual') { + return; + } + + this.scheduleFn = async () => { + const id = `${this.getProviderName()}:refresh`; + 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); + } + }, + }); + }; + } } // 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';