From 62f448edb0b56b2e8c659a0b51331e0536379eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 28 Aug 2023 11:40:05 +0200 Subject: [PATCH] allow app-config of the catalog processingInterval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/fifty-adults-shake.md | 5 + .changeset/friendly-pugs-lick.md | 5 + .changeset/popular-coins-tap.md | 5 + ...adTaskScheduleDefinitionFromConfig.test.ts | 6 +- .../readTaskScheduleDefinitionFromConfig.ts | 47 +------ packages/config/api-report.md | 10 ++ packages/config/package.json | 1 + packages/config/src/index.ts | 1 + .../config/src/readDurationFromConfig.test.ts | 128 ++++++++++++++++++ packages/config/src/readDurationFromConfig.ts | 101 ++++++++++++++ plugins/catalog-backend/config.d.ts | 20 +++ .../src/service/CatalogBuilder.ts | 41 +++++- yarn.lock | 1 + 13 files changed, 316 insertions(+), 55 deletions(-) create mode 100644 .changeset/fifty-adults-shake.md create mode 100644 .changeset/friendly-pugs-lick.md create mode 100644 .changeset/popular-coins-tap.md create mode 100644 packages/config/src/readDurationFromConfig.test.ts create mode 100644 packages/config/src/readDurationFromConfig.ts diff --git a/.changeset/fifty-adults-shake.md b/.changeset/fifty-adults-shake.md new file mode 100644 index 0000000000..a21c5805ab --- /dev/null +++ b/.changeset/fifty-adults-shake.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Allow configuring the processing interval in your app-config, under the `catalog.processingInterval` key. diff --git a/.changeset/friendly-pugs-lick.md b/.changeset/friendly-pugs-lick.md new file mode 100644 index 0000000000..3a82497da8 --- /dev/null +++ b/.changeset/friendly-pugs-lick.md @@ -0,0 +1,5 @@ +--- +'@backstage/config': minor +--- + +Added a `readDurationFromConfig` function diff --git a/.changeset/popular-coins-tap.md b/.changeset/popular-coins-tap.md new file mode 100644 index 0000000000..3ece9e20b1 --- /dev/null +++ b/.changeset/popular-coins-tap.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-tasks': patch +--- + +Use `readDurationFromConfig` from the config package diff --git a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts index 21f51954b5..c52d59b016 100644 --- a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts +++ b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts @@ -85,7 +85,7 @@ describe('readTaskScheduleDefinitionFromConfig', () => { }); expect(() => readTaskScheduleDefinitionFromConfig(config)).toThrow( - 'HumanDuration needs at least one of', + "Failed to read duration from config at 'frequency', Error: Needs one or more of 'years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'milliseconds'", ); }); @@ -98,7 +98,7 @@ describe('readTaskScheduleDefinitionFromConfig', () => { }); expect(() => readTaskScheduleDefinitionFromConfig(config)).toThrow( - "Unable to convert config value for key 'frequency.minutes' in 'mock-config' to a number", + "Failed to read duration from config, Error: Unable to convert config value for key 'frequency.minutes' in 'mock-config' to a number", ); }); @@ -112,7 +112,7 @@ describe('readTaskScheduleDefinitionFromConfig', () => { }); expect(() => readTaskScheduleDefinitionFromConfig(config)).toThrow( - 'HumanDuration does not contain properties: invalid', + "Failed to read duration from config at 'frequency', Error: Unknown property 'invalid'; expected one or more of 'years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'milliseconds'", ); }); diff --git a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts index b31cf68f74..8053936263 100644 --- a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts +++ b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts @@ -14,54 +14,11 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; +import { Config, readDurationFromConfig } from '@backstage/config'; import { HumanDuration } from '@backstage/types'; import { TaskScheduleDefinition } from './types'; import { Duration } from 'luxon'; -const propsOfHumanDuration = [ - 'years', - 'months', - 'weeks', - 'days', - 'hours', - 'minutes', - 'seconds', - 'milliseconds', -]; - -function convertToHumanDuration(config: Config, key: string): HumanDuration { - // Ensures that the root is an object - const root = config.getConfig(key); - - const result: Record = {}; - let found = false; - for (const prop of propsOfHumanDuration) { - const value = root.getOptionalNumber(prop); - if (value !== undefined) { - result[prop] = value; - found = true; - } - } - - if (!found) { - throw new Error( - `HumanDuration needs at least one of: ${propsOfHumanDuration}`, - ); - } - - const invalidProps = root - .keys() - .filter(prop => !propsOfHumanDuration.includes(prop)); - if (invalidProps.length > 0) { - throw new Error( - `HumanDuration does not contain properties: ${invalidProps}`, - ); - } - - return result as HumanDuration; -} - function readDuration(config: Config, key: string): Duration | HumanDuration { if (typeof config.get(key) === 'string') { const value = config.getString(key); @@ -72,7 +29,7 @@ function readDuration(config: Config, key: string): Duration | HumanDuration { return duration; } - return convertToHumanDuration(config, key); + return readDurationFromConfig(config, { key }); } function readCronOrDuration( diff --git a/packages/config/api-report.md b/packages/config/api-report.md index df9baeb4cd..ea9617b2a3 100644 --- a/packages/config/api-report.md +++ b/packages/config/api-report.md @@ -3,6 +3,8 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +import { Config as Config_2 } from '@backstage/config'; +import { HumanDuration } from '@backstage/types'; import type { JsonArray as JsonArray_2 } from '@backstage/types'; import { JsonObject as JsonObject_2 } from '@backstage/types'; import type { JsonPrimitive as JsonPrimitive_2 } from '@backstage/types'; @@ -80,4 +82,12 @@ export type JsonPrimitive = JsonPrimitive_2; // @public @deprecated export type JsonValue = JsonValue_2; + +// @public +export function readDurationFromConfig( + config: Config_2, + options?: { + key?: string; + }, +): HumanDuration; ``` diff --git a/packages/config/package.json b/packages/config/package.json index 4470e9719e..f8884cde71 100644 --- a/packages/config/package.json +++ b/packages/config/package.json @@ -32,6 +32,7 @@ "clean": "backstage-cli package clean" }, "dependencies": { + "@backstage/errors": "workspace:^", "@backstage/types": "workspace:^", "lodash": "^4.17.21" }, diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index c8800d0ad2..c6254e97ba 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -26,5 +26,6 @@ export type { JsonPrimitive, JsonValue, } from './deprecatedTypes'; +export { readDurationFromConfig } from './readDurationFromConfig'; export { ConfigReader } from './reader'; export type { AppConfig, Config } from './types'; diff --git a/packages/config/src/readDurationFromConfig.test.ts b/packages/config/src/readDurationFromConfig.test.ts new file mode 100644 index 0000000000..3da790ddcf --- /dev/null +++ b/packages/config/src/readDurationFromConfig.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + readDurationFromConfig, + propsOfHumanDuration, +} from './readDurationFromConfig'; +import { ConfigReader } from './reader'; + +describe('readDurationFromConfig', () => { + it('reads all known keys', () => { + const config = new ConfigReader({ + milliseconds: 1, + seconds: 2, + minutes: 3, + hours: 4, + days: 5, + weeks: 6, + months: 7, + years: 8, + }); + expect(readDurationFromConfig(config)).toEqual({ + milliseconds: 1, + seconds: 2, + minutes: 3, + hours: 4, + days: 5, + weeks: 6, + months: 7, + years: 8, + }); + }); + + it('reads all known keys, for a subkey', () => { + const config = new ConfigReader({ + sub: { + key: { + milliseconds: 1, + seconds: 2, + minutes: 3, + hours: 4, + days: 5, + weeks: 6, + months: 7, + years: 8, + }, + }, + }); + expect(readDurationFromConfig(config, { key: 'sub.key' })).toEqual({ + milliseconds: 1, + seconds: 2, + minutes: 3, + hours: 4, + days: 5, + weeks: 6, + months: 7, + years: 8, + }); + }); + + it('rejects wrong type of target, for a subkey', () => { + const config = new ConfigReader({ + sub: { key: 7 }, + }); + expect(() => readDurationFromConfig(config, { key: 'sub.key' })).toThrow( + "Failed to read duration from config, TypeError: Invalid type in config for key 'sub.key' in 'mock-config', got number, wanted object", + ); + }); + + it('rejects no keys', () => { + const config = new ConfigReader({}); + expect(() => readDurationFromConfig(config)).toThrow( + `Failed to read duration from config, Error: Needs one or more of 'years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'milliseconds'`, + ); + }); + + it('rejects no keys, for a subkey', () => { + const config = new ConfigReader({ sub: { key: {} } }); + expect(() => readDurationFromConfig(config, { key: 'sub.key' })).toThrow( + `Failed to read duration from config at 'sub.key', Error: Needs one or more of 'years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'milliseconds'`, + ); + }); + + it('rejects unknown keys', () => { + const config = new ConfigReader({ + minutes: 3, + invalid: 'value', + }); + expect(() => readDurationFromConfig(config)).toThrow( + `Failed to read duration from config, Error: Unknown property 'invalid'; expected one or more of 'years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'milliseconds'`, + ); + }); + + it.each(propsOfHumanDuration)('rejects non-number %p', prop => { + const config = new ConfigReader({ + [prop]: 'value', + }); + expect(() => readDurationFromConfig(config)).toThrow( + `Failed to read duration from config, Error: Unable to convert config value for key '${prop}' in 'mock-config' to a number`, + ); + }); + + it.each(propsOfHumanDuration)('rejects non-number %p, for a subkey', prop => { + const config = new ConfigReader({ + sub: { + key: { + [prop]: 'value', + }, + }, + }); + expect(() => readDurationFromConfig(config, { key: 'sub.key' })).toThrow( + `Failed to read duration from config, Error: Unable to convert config value for key 'sub.key.${prop}' in 'mock-config' to a number`, + ); + }); +}); diff --git a/packages/config/src/readDurationFromConfig.ts b/packages/config/src/readDurationFromConfig.ts new file mode 100644 index 0000000000..2f59e9a976 --- /dev/null +++ b/packages/config/src/readDurationFromConfig.ts @@ -0,0 +1,101 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Config } from '@backstage/config'; +import { InputError } from '@backstage/errors'; +import { HumanDuration } from '@backstage/types'; + +export const propsOfHumanDuration = [ + 'years', + 'months', + 'weeks', + 'days', + 'hours', + 'minutes', + 'seconds', + 'milliseconds', +]; + +/** + * Reads a duration from a config object. + * + * @public + * @remarks + * + * This does not support optionality; if you want to support optional durations, + * you need to first check the presence of the target with `config.has(...)` and + * then call this function. + * + * @param config - A configuration object + * @param key - If specified, read the duration from the given subkey + * under the config object + * @returns A duration object + */ +export function readDurationFromConfig( + config: Config, + options?: { + key?: string; + }, +): HumanDuration { + let root: Config; + let found = false; + const result: Record = {}; + + try { + root = options?.key ? config.getConfig(options.key) : config; + for (const prop of propsOfHumanDuration) { + const value = root.getOptionalNumber(prop); + if (value !== undefined) { + result[prop] = value; + found = true; + } + } + } catch (error) { + // This needs no contextual key prefix since the config reader adds it to + // its own errors + throw new InputError(`Failed to read duration from config, ${error}`); + } + + try { + if (!found) { + const good = propsOfHumanDuration.map(p => `'${p}'`).join(', '); + throw new Error(`Needs one or more of ${good}`); + } + + const invalidProps = root + .keys() + .filter(prop => !propsOfHumanDuration.includes(prop)); + if (invalidProps.length) { + const what = invalidProps.length === 1 ? 'property' : 'properties'; + const bad = invalidProps.map(p => `'${p}'`).join(', '); + const good = propsOfHumanDuration.map(p => `'${p}'`).join(', '); + throw new Error( + `Unknown ${what} ${bad}; expected one or more of ${good}`, + ); + } + } catch (error) { + // For our own errors, even though we don't know where we are anchored in + // the config hierarchy, we try to be helpful and at least add the subkey if + // we have it + let prefix = 'Failed to read duration from config'; + if (options?.key) { + prefix += ` at '${options.key}'`; + } + throw new InputError(`${prefix}, ${error}`); + } + + return result as HumanDuration; +} diff --git a/plugins/catalog-backend/config.d.ts b/plugins/catalog-backend/config.d.ts index 03e47f8e61..c5f04793e4 100644 --- a/plugins/catalog-backend/config.d.ts +++ b/plugins/catalog-backend/config.d.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { HumanDuration } from '@backstage/types'; + export interface Config { /** * Configuration options for the catalog plugin. @@ -142,5 +144,23 @@ export interface Config { * "keep". */ orphanStrategy?: 'keep' | 'delete'; + + /** + * The interval at which the catalog should process its entities. + * + * @remarks + * + * Note that this is only a suggested minimum, and the actual interval may + * be longer. Internally, the catalog will scale up this number by a small + * factor and choose random numbers in that range to spread out the load. If + * the catalog is overloaded and cannot process all entities during the + * interval, the time taken between processing runs of any given entity may + * also be longer than specified here. + * + * Setting this value too low risks exhausting rate limits on external + * systems that are queried by processors, such as version control systems + * housing catalog-info files. + */ + processingInterval?: HumanDuration; }; } diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 9c351a76f2..ed0d15b6f9 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -74,7 +74,7 @@ import { createRouter } from './createRouter'; import { DefaultRefreshService } from './DefaultRefreshService'; import { AuthorizedRefreshService } from './AuthorizedRefreshService'; import { DefaultCatalogRulesEnforcer } from '../ingestion/CatalogRules'; -import { Config } from '@backstage/config'; +import { Config, readDurationFromConfig } from '@backstage/config'; import { Logger } from 'winston'; import { connectEntityProviders } from '../processing/connectEntityProviders'; import { PermissionRuleParams } from '@backstage/plugin-permission-common'; @@ -100,6 +100,7 @@ import { AuthorizedLocationService } from './AuthorizedLocationService'; import { DefaultProviderDatabase } from '../database/DefaultProviderDatabase'; import { DefaultCatalogDatabase } from '../database/DefaultCatalogDatabase'; import { EventBroker } from '@backstage/plugin-events-node'; +import { durationToMilliseconds } from '@backstage/types'; /** * This is a duplicate of the alpha `CatalogPermissionRule` type, for use in the stable API. @@ -160,11 +161,7 @@ export class CatalogBuilder { unprocessedEntity: Entity; errors: Error[]; }) => Promise | void; - private processingInterval: ProcessingIntervalFunction = - createRandomProcessingInterval({ - minSeconds: 100, - maxSeconds: 150, - }); + private processingInterval: ProcessingIntervalFunction; private locationAnalyzer: LocationAnalyzer | undefined = undefined; private readonly permissionRules: CatalogPermissionRuleInput[]; private allowedLocationType: string[]; @@ -191,6 +188,10 @@ export class CatalogBuilder { this.parser = undefined; this.permissionRules = Object.values(catalogPermissionRules); this.allowedLocationType = ['url']; + + this.processingInterval = CatalogBuilder.getDefaultProcessingInterval( + env.config, + ); } /** @@ -422,7 +423,7 @@ export class CatalogBuilder { } /** - * Enables the publishing of events for cloflicts in the DefaultProcessingDatabase + * Enables the publishing of events for conflicts in the DefaultProcessingDatabase */ setEventBroker(broker: EventBroker): CatalogBuilder { this.eventBroker = broker; @@ -758,4 +759,30 @@ export class CatalogBuilder { 'https://backstage.io/docs/integrations/azure/org', ); } + + private static getDefaultProcessingInterval( + config: Config, + ): ProcessingIntervalFunction { + const processingIntervalKey = 'catalog.processingInterval'; + + if (!config.has(processingIntervalKey)) { + return createRandomProcessingInterval({ + minSeconds: 100, + maxSeconds: 150, + }); + } + + const duration = readDurationFromConfig(config, { + key: processingIntervalKey, + }); + const seconds = Math.min( + 1, + Math.round(durationToMilliseconds(duration) / 1000), + ); + + return createRandomProcessingInterval({ + minSeconds: seconds, + maxSeconds: seconds * 1.5, + }); + } } diff --git a/yarn.lock b/yarn.lock index 82c9012702..7b788eda90 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3784,6 +3784,7 @@ __metadata: resolution: "@backstage/config@workspace:packages/config" dependencies: "@backstage/cli": "workspace:^" + "@backstage/errors": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" "@types/node": ^16.0.0