From 01edf6e9169bb56f58f8e96e68addcb4daa7e894 Mon Sep 17 00:00:00 2001 From: Mark Nachazel Date: Tue, 11 Mar 2025 15:17:58 +0000 Subject: [PATCH 1/3] feat(backend-defaults): implement more comprehensive redis option passthrough Signed-off-by: Mark Nachazel --- .changeset/clever-tomatoes-jump.md | 5 ++ .../entrypoints/cache/CacheManager.test.ts | 73 +++++++++++++++- .../src/entrypoints/cache/CacheManager.ts | 86 +++++++++++++++++-- .../src/entrypoints/cache/types.ts | 18 ++++ 4 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 .changeset/clever-tomatoes-jump.md diff --git a/.changeset/clever-tomatoes-jump.md b/.changeset/clever-tomatoes-jump.md new file mode 100644 index 0000000000..b18ca53189 --- /dev/null +++ b/.changeset/clever-tomatoes-jump.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-defaults': minor +--- + +Allow pass through of redis client and cluster options to Cache core service diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts index d15671d5d2..82e4b03d56 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts @@ -15,7 +15,7 @@ */ import { mockServices, TestCaches } from '@backstage/backend-test-utils'; -import KeyvRedis from '@keyv/redis'; +import KeyvRedis, { createCluster } from '@keyv/redis'; import KeyvMemcache from '@keyv/memcache'; import { CacheManager } from './CacheManager'; @@ -30,6 +30,7 @@ jest.mock('@keyv/redis', () => { ...Actual, __esModule: true, default: jest.fn((...args: any[]) => new DefaultConstructor(...args)), + createCluster: jest.fn(), }; }); jest.mock('@keyv/memcache', () => { @@ -194,3 +195,73 @@ describe('CacheManager integration', () => { ).toThrow(/Invalid duration 'hello' in config/); }); }); + +describe('CacheManager store options', () => { + it('uses default options when no store-specific config exists', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'redis', + connection: 'redis://localhost:6379', + }, + }, + }, + }), + ); + + manager.forPlugin('p1'); + + expect(KeyvRedis).toHaveBeenCalledWith('redis://localhost:6379', { + keyPrefixSeparator: ':', + }); + }); + + it('defaults to non-clustered mode when cluster config is missing root nodes', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'redis', + connection: 'redis://localhost:6379', + cluster: {}, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(KeyvRedis).toHaveBeenCalledWith('redis://localhost:6379', { + keyPrefixSeparator: ':', + }); + }); + + it('uses cluster config when present', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'redis', + connection: 'redis://localhost:6379', + redis: { + cluster: { + rootNodes: [{ url: 'redis://localhost:6379' }], + }, + }, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(createCluster).toHaveBeenCalledWith({ + rootNodes: [{ url: 'redis://localhost:6379' }], + defaults: undefined, + }); + }); +}); diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts index 57c326f00d..8c0afae826 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts @@ -22,7 +22,12 @@ import { } from '@backstage/backend-plugin-api'; import Keyv from 'keyv'; import { DefaultCacheClient } from './CacheClient'; -import { CacheManagerOptions, ttlToMilliseconds } from './types'; +import { + CacheManagerOptions, + ttlToMilliseconds, + CacheStoreOptions, + RedisCacheStoreOptions, +} from './types'; import { durationToMilliseconds } from '@backstage/types'; import { readDurationFromConfig } from '@backstage/config'; @@ -51,6 +56,7 @@ export class CacheManager { private readonly connection: string; private readonly errorHandler: CacheManagerOptions['onError']; private readonly defaultTtl?: number; + private readonly storeOptions?: CacheStoreOptions; /** * Creates a new {@link CacheManager} instance by reading from the `backend` @@ -89,15 +95,73 @@ export class CacheManager { } } + // Read store-specific options from config + const storeOptions = CacheManager.parseStoreOptions(store, config, logger); + return new CacheManager( store, connectionString, options.onError, logger, defaultTtl, + storeOptions, ); } + /** + * Parse store-specific options from configuration. + * + * @param store - The cache store type ('redis', 'memcache', or 'memory') + * @param config - The configuration service + * @param logger - Optional logger for warnings + * @returns The parsed store options + */ + private static parseStoreOptions( + store: string, + config: RootConfigService, + logger?: LoggerService, + ): CacheStoreOptions | undefined { + const storeConfigPath = `backend.cache.${store}`; + + if (store === 'redis' && config.has(storeConfigPath)) { + return CacheManager.parseRedisOptions(storeConfigPath, config, logger); + } + + return undefined; + } + + /** + * Parse Redis-specific options from configuration. + */ + private static parseRedisOptions( + storeConfigPath: string, + config: RootConfigService, + logger?: LoggerService, + ): RedisCacheStoreOptions { + const redisOptions: RedisCacheStoreOptions = {}; + const redisConfig = config.getConfig(storeConfigPath); + + redisOptions.client = redisConfig.getOptional('client'); + + if (redisConfig.has('cluster')) { + const clusterConfig = redisConfig.getConfig('cluster'); + + if (!clusterConfig.has('rootNodes')) { + logger?.warn( + `Redis cluster config has no 'rootNodes' key, defaulting to non-clustered mode`, + ); + return redisOptions; + } + + redisOptions.cluster = { + rootNodes: clusterConfig.get('rootNodes'), + defaults: clusterConfig.getOptional('defaults'), + }; + } + + return redisOptions; + } + /** @internal */ constructor( store: string, @@ -105,6 +169,7 @@ export class CacheManager { errorHandler: CacheManagerOptions['onError'], logger?: LoggerService, defaultTtl?: number, + storeOptions?: CacheStoreOptions, ) { if (!this.storeFactories.hasOwnProperty(store)) { throw new Error(`Unknown cache store: ${store}`); @@ -114,6 +179,7 @@ export class CacheManager { this.connection = connectionString; this.errorHandler = errorHandler; this.defaultTtl = defaultTtl; + this.storeOptions = storeOptions; } /** @@ -140,13 +206,23 @@ export class CacheManager { private createRedisStoreFactory(): StoreFactory { const KeyvRedis = require('@keyv/redis').default; - const stores: Record = {}; + const { createCluster } = require('@keyv/redis'); + const stores: Record = {}; return (pluginId, defaultTtl) => { if (!stores[pluginId]) { - stores[pluginId] = new KeyvRedis(this.connection, { - keyPrefixSeparator: ':', - }); + if (this.storeOptions?.cluster) { + // Create a Redis cluster + const cluster = createCluster(this.storeOptions?.cluster); + stores[pluginId] = new KeyvRedis(cluster); + } else { + // Create a regular Redis connection + const redisOptions = this.storeOptions?.client || { + keyPrefixSeparator: ':', + }; + stores[pluginId] = new KeyvRedis(this.connection, redisOptions); + } + // Always provide an error handler to avoid stopping the process stores[pluginId].on('error', (err: Error) => { this.logger?.error('Failed to create redis cache client', err); diff --git a/packages/backend-defaults/src/entrypoints/cache/types.ts b/packages/backend-defaults/src/entrypoints/cache/types.ts index 7260099a0a..4d32f007b0 100644 --- a/packages/backend-defaults/src/entrypoints/cache/types.ts +++ b/packages/backend-defaults/src/entrypoints/cache/types.ts @@ -16,6 +16,24 @@ import { LoggerService } from '@backstage/backend-plugin-api'; import { HumanDuration, durationToMilliseconds } from '@backstage/types'; +import { RedisClusterOptions, RedisClientOptions } from '@keyv/redis'; + +/** + * Options for Redis cache store. + * + * @public + */ +export type RedisCacheStoreOptions = { + client?: RedisClientOptions; + cluster?: RedisClusterOptions; +}; + +/** + * Union type of all cache store options. + * + * @public + */ +export type CacheStoreOptions = RedisCacheStoreOptions; /** * Options given when constructing a {@link CacheManager}. From fc1c605b9eba028f3e5382e16fb5a8610d5b8be5 Mon Sep 17 00:00:00 2001 From: Mark Nachazel Date: Wed, 12 Mar 2025 16:47:52 -0400 Subject: [PATCH 2/3] accept client config for clustered redis Signed-off-by: Mark Nachazel --- .../entrypoints/cache/CacheManager.test.ts | 53 +++++++++++++++++++ .../src/entrypoints/cache/CacheManager.ts | 8 +-- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts index 82e4b03d56..fc7ed34fc4 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts @@ -264,4 +264,57 @@ describe('CacheManager store options', () => { defaults: undefined, }); }); + + it('respects client config for non-clustered mode', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'redis', + connection: 'redis://localhost:6379', + redis: { + client: { + keyPrefixSeparator: '!', + }, + }, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(KeyvRedis).toHaveBeenCalledWith('redis://localhost:6379', { + keyPrefixSeparator: '!', + }); + }); + + it('accepts client config for clustered mode', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'redis', + connection: 'redis://localhost:6379', + redis: { + client: { + keyPrefixSeparator: '!', + }, + cluster: { + rootNodes: [{ url: 'redis://localhost:6379' }], + }, + }, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(KeyvRedis).toHaveBeenCalledWith(expect.anything(), { + keyPrefixSeparator: '!', + }); + }); }); diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts index 8c0afae826..e4f0f5f408 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts @@ -211,15 +211,15 @@ export class CacheManager { return (pluginId, defaultTtl) => { if (!stores[pluginId]) { + const redisOptions = this.storeOptions?.client || { + keyPrefixSeparator: ':', + }; if (this.storeOptions?.cluster) { // Create a Redis cluster const cluster = createCluster(this.storeOptions?.cluster); - stores[pluginId] = new KeyvRedis(cluster); + stores[pluginId] = new KeyvRedis(cluster, redisOptions); } else { // Create a regular Redis connection - const redisOptions = this.storeOptions?.client || { - keyPrefixSeparator: ':', - }; stores[pluginId] = new KeyvRedis(this.connection, redisOptions); } From 893e8730b774d67ad05d15138c035305a4dac144 Mon Sep 17 00:00:00 2001 From: Mark Nachazel Date: Sun, 16 Mar 2025 19:52:54 -0400 Subject: [PATCH 3/3] update types, lock down client options Signed-off-by: Mark Nachazel --- packages/backend-defaults/config.d.ts | 61 +++++++++++++++++++ .../entrypoints/cache/CacheManager.test.ts | 4 +- .../src/entrypoints/cache/CacheManager.ts | 20 +++++- .../src/entrypoints/cache/types.ts | 4 +- 4 files changed, 84 insertions(+), 5 deletions(-) diff --git a/packages/backend-defaults/config.d.ts b/packages/backend-defaults/config.d.ts index 438790a177..0729bad456 100644 --- a/packages/backend-defaults/config.d.ts +++ b/packages/backend-defaults/config.d.ts @@ -559,6 +559,67 @@ export interface Config { connection: string; /** An optional default TTL (in milliseconds, if given as a number). */ defaultTtl?: number | HumanDuration | string; + redis?: { + /** + * An optional Redis client configuration. These options are passed to the `@keyv/redis` client. + */ + client?: { + /** + * Namespace for the current instance. + */ + namespace?: string; + /** + * Separator to use between namespace and key. + */ + keyPrefixSeparator?: string; + /** + * Number of keys to delete in a single batch. + */ + clearBatchSize?: number; + /** + * Enable Unlink instead of using Del for clearing keys. This is more performant but may not be supported by all Redis versions. + */ + useUnlink?: boolean; + /** + * Whether to allow clearing all keys when no namespace is set. + * If set to true and no namespace is set, iterate() will return all keys. + * Defaults to `false`. + */ + noNamespaceAffectsAll?: boolean; + }; + /** + * An optional Redis cluster configuration. + */ + cluster?: { + /** + * Cluster configuration options to be passed to the `@keyv/redis` client (and node-redis under the hood) + * https://github.com/redis/node-redis/blob/master/docs/clustering.md + * + * @visibility secret + */ + rootNodes: Array; + /** + * Cluster node default configuration options to be passed to the `@keyv/redis` client (and node-redis under the hood) + * https://github.com/redis/node-redis/blob/master/docs/clustering.md + * + * @visibility secret + */ + defaults?: Partial; + /** + * When `true`, `.connect()` will only discover the cluster topology, without actually connecting to all the nodes. + * Useful for short-term or PubSub-only connections. + */ + minimizeConnections?: boolean; + /** + * When `true`, distribute load by executing readonly commands (such as `GET`, `GEOSEARCH`, etc.) across all cluster nodes. When `false`, only use master nodes. + */ + useReplicas?: boolean; + /** + * The maximum number of times a command will be redirected due to `MOVED` or `ASK` errors. + */ + maxCommandRedirections?: number; + }; + }; } | { store: 'memcache'; diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts index fc7ed34fc4..3a7886098f 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts @@ -226,7 +226,9 @@ describe('CacheManager store options', () => { cache: { store: 'redis', connection: 'redis://localhost:6379', - cluster: {}, + redis: { + cluster: {}, + }, }, }, }, diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts index e4f0f5f408..ab8b03eb3b 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts @@ -141,7 +141,16 @@ export class CacheManager { const redisOptions: RedisCacheStoreOptions = {}; const redisConfig = config.getConfig(storeConfigPath); - redisOptions.client = redisConfig.getOptional('client'); + redisOptions.client = { + namespace: redisConfig.getOptionalString('client.namespace'), + keyPrefixSeparator: + redisConfig.getOptionalString('client.keyPrefixSeparator') || ':', + clearBatchSize: redisConfig.getOptionalNumber('client.clearBatchSize'), + useUnlink: redisConfig.getOptionalBoolean('client.useUnlink'), + noNamespaceAffectsAll: redisConfig.getOptionalBoolean( + 'client.noNamespaceAffectsAll', + ), + }; if (redisConfig.has('cluster')) { const clusterConfig = redisConfig.getConfig('cluster'); @@ -156,6 +165,13 @@ export class CacheManager { redisOptions.cluster = { rootNodes: clusterConfig.get('rootNodes'), defaults: clusterConfig.getOptional('defaults'), + minimizeConnections: clusterConfig.getOptionalBoolean( + 'minimizeConnections', + ), + useReplicas: clusterConfig.getOptionalBoolean('useReplicas'), + maxCommandRedirections: clusterConfig.getOptionalNumber( + 'maxCommandRedirections', + ), }; } @@ -207,7 +223,7 @@ export class CacheManager { private createRedisStoreFactory(): StoreFactory { const KeyvRedis = require('@keyv/redis').default; const { createCluster } = require('@keyv/redis'); - const stores: Record = {}; + const stores: Record = {}; return (pluginId, defaultTtl) => { if (!stores[pluginId]) { diff --git a/packages/backend-defaults/src/entrypoints/cache/types.ts b/packages/backend-defaults/src/entrypoints/cache/types.ts index 4d32f007b0..d21fb5d44b 100644 --- a/packages/backend-defaults/src/entrypoints/cache/types.ts +++ b/packages/backend-defaults/src/entrypoints/cache/types.ts @@ -16,7 +16,7 @@ import { LoggerService } from '@backstage/backend-plugin-api'; import { HumanDuration, durationToMilliseconds } from '@backstage/types'; -import { RedisClusterOptions, RedisClientOptions } from '@keyv/redis'; +import { RedisClusterOptions, KeyvRedisOptions } from '@keyv/redis'; /** * Options for Redis cache store. @@ -24,7 +24,7 @@ import { RedisClusterOptions, RedisClientOptions } from '@keyv/redis'; * @public */ export type RedisCacheStoreOptions = { - client?: RedisClientOptions; + client?: KeyvRedisOptions; cluster?: RedisClusterOptions; };