diff --git a/.changeset/fix-valkey-cluster-client.md b/.changeset/fix-valkey-cluster-client.md new file mode 100644 index 0000000000..07bd60e302 --- /dev/null +++ b/.changeset/fix-valkey-cluster-client.md @@ -0,0 +1,9 @@ +--- +'@backstage/backend-defaults': patch +--- + +Fixed Valkey cluster mode to use `iovalkey`'s `Cluster` class instead of +`createCluster` from `@keyv/redis`. The previous implementation passed a +`@redis/client` `RedisCluster` instance to `@keyv/valkey`, which expects an +`iovalkey` `Cluster` instance. This caused the cluster client to not be +recognized correctly, as the two libraries have incompatible object models. diff --git a/packages/backend-defaults/package.json b/packages/backend-defaults/package.json index 33a262aadd..4f3aed7a53 100644 --- a/packages/backend-defaults/package.json +++ b/packages/backend-defaults/package.json @@ -170,6 +170,7 @@ "git-url-parse": "^15.0.0", "helmet": "^6.0.0", "infinispan": "^0.12.0", + "iovalkey": "^0.3.3", "is-glob": "^4.0.3", "jose": "^5.0.0", "keyv": "^5.2.1", diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts index 323ca881c5..9a29901239 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.test.ts @@ -18,6 +18,7 @@ import { mockServices, TestCaches } from '@backstage/backend-test-utils'; import KeyvRedis, { createCluster } from '@keyv/redis'; import KeyvValkey from '@keyv/valkey'; import KeyvMemcache from '@keyv/memcache'; +import { Cluster as ValkeyCluster } from 'iovalkey'; import { CacheManager } from './CacheManager'; // This test is in a separate file because the main test file uses other mocking @@ -41,7 +42,13 @@ jest.mock('@keyv/valkey', () => { ...Actual, __esModule: true, default: jest.fn((...args: any[]) => new DefaultConstructor(...args)), - createCluster: jest.fn(), + }; +}); +jest.mock('iovalkey', () => { + const Actual = jest.requireActual('iovalkey'); + return { + ...Actual, + Cluster: jest.fn(), }; }); jest.mock('@keyv/memcache', () => { @@ -211,6 +218,8 @@ describe('CacheManager integration', () => { }); describe('CacheManager store options', () => { + afterEach(jest.clearAllMocks); + it('uses default options when no store-specific config exists', () => { const manager = CacheManager.fromConfig( mockServices.rootConfig({ @@ -307,6 +316,9 @@ describe('CacheManager store options', () => { }); it('accepts client config for clustered mode', () => { + const clusterInstance = { fake: 'cluster' }; + (createCluster as jest.Mock).mockReturnValue(clusterInstance); + const manager = CacheManager.fromConfig( mockServices.rootConfig({ data: { @@ -329,11 +341,112 @@ describe('CacheManager store options', () => { ); manager.forPlugin('p1'); - expect(KeyvRedis).toHaveBeenCalledWith(expect.anything(), { + expect(KeyvRedis).toHaveBeenCalledWith(clusterInstance, { keyPrefixSeparator: '!', }); }); + it('uses iovalkey Cluster for valkey cluster mode', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'valkey', + connection: 'redis://localhost:6379', + valkey: { + cluster: { + rootNodes: [ + { host: 'localhost', port: 6379 }, + { host: 'localhost', port: 6380 }, + ], + }, + }, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(ValkeyCluster).toHaveBeenCalledWith( + [ + { host: 'localhost', port: 6379 }, + { host: 'localhost', port: 6380 }, + ], + { + redisOptions: undefined, + scaleReads: undefined, + maxRedirections: undefined, + lazyConnect: undefined, + }, + ); + expect(KeyvValkey).toHaveBeenCalledWith(expect.any(Object), { + keyPrefix: undefined, + }); + }); + + it('passes valkey cluster options from config', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'valkey', + connection: 'redis://localhost:6379', + valkey: { + client: { keyPrefix: 'my-app:' }, + cluster: { + rootNodes: [{ host: 'localhost', port: 6379 }], + useReplicas: true, + maxCommandRedirections: 5, + }, + }, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(ValkeyCluster).toHaveBeenCalledWith( + [{ host: 'localhost', port: 6379 }], + { + redisOptions: undefined, + scaleReads: 'slave', + maxRedirections: 5, + lazyConnect: undefined, + }, + ); + expect(KeyvValkey).toHaveBeenCalledWith(expect.any(Object), { + keyPrefix: 'my-app:', + }); + }); + + it('defaults to non-clustered valkey when cluster config is missing root nodes', () => { + const manager = CacheManager.fromConfig( + mockServices.rootConfig({ + data: { + backend: { + cache: { + store: 'valkey', + connection: 'redis://localhost:6379', + valkey: { + cluster: {}, + }, + }, + }, + }, + }), + ); + manager.forPlugin('p1'); + + expect(ValkeyCluster).not.toHaveBeenCalled(); + expect(KeyvValkey).toHaveBeenCalledWith('redis://localhost:6379', { + keyPrefix: undefined, + }); + }); + it('correctly applies namespace configuration to redis and valkey stores', () => { const testCases = [ { diff --git a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts index 38c0c761ed..4403d4d76a 100644 --- a/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts +++ b/packages/backend-defaults/src/entrypoints/cache/CacheManager.ts @@ -240,14 +240,16 @@ export class CacheManager { valkeyOptions.cluster = { rootNodes: clusterConfig.get('rootNodes'), - defaults: clusterConfig.getOptional('defaults'), - minimizeConnections: clusterConfig.getOptionalBoolean( - 'minimizeConnections', - ), - useReplicas: clusterConfig.getOptionalBoolean('useReplicas'), - maxCommandRedirections: clusterConfig.getOptionalNumber( - 'maxCommandRedirections', - ), + options: { + redisOptions: clusterConfig.getOptional('defaults'), + scaleReads: clusterConfig.getOptionalBoolean('useReplicas') + ? 'slave' + : undefined, + maxRedirections: clusterConfig.getOptionalNumber( + 'maxCommandRedirections', + ), + lazyConnect: clusterConfig.getOptionalBoolean('minimizeConnections'), + }, }; } @@ -368,9 +370,7 @@ export class CacheManager { private createValkeyStoreFactory(): StoreFactory { const KeyvValkey = require('@keyv/valkey').default; - // `@keyv/valkey` doesn't export a `createCluster` function, but is compatible with the one from `@keyv/redis` - // See https://keyv.org/docs/storage-adapters/valkey - const { createCluster } = require('@keyv/redis'); + const { Cluster } = require('iovalkey'); const stores: Record = {}; return (pluginId, defaultTtl) => { @@ -382,8 +382,11 @@ export class CacheManager { if (!stores[pluginId]) { const valkeyOptions = this.storeOptions?.client; if (this.storeOptions?.cluster) { - // Create a Valkey cluster (Redis cluster under the hood) - const cluster = createCluster(this.storeOptions?.cluster); + // Create an iovalkey Cluster instance, which is the type that @keyv/valkey expects + const cluster = new Cluster( + this.storeOptions.cluster.rootNodes, + this.storeOptions.cluster.options, + ); stores[pluginId] = new KeyvValkey(cluster, valkeyOptions); } else { // Create a regular Valkey connection diff --git a/packages/backend-defaults/src/entrypoints/cache/types.ts b/packages/backend-defaults/src/entrypoints/cache/types.ts index 60e32e8618..7f35d516f7 100644 --- a/packages/backend-defaults/src/entrypoints/cache/types.ts +++ b/packages/backend-defaults/src/entrypoints/cache/types.ts @@ -18,6 +18,7 @@ import { LoggerService } from '@backstage/backend-plugin-api'; import { HumanDuration, durationToMilliseconds } from '@backstage/types'; import { RedisClusterOptions, KeyvRedisOptions } from '@keyv/redis'; import { KeyvValkeyOptions } from '@keyv/valkey'; +import { ClusterNode, ClusterOptions } from 'iovalkey'; /** * Options for Redis cache store. @@ -38,7 +39,10 @@ export type RedisCacheStoreOptions = { export type ValkeyCacheStoreOptions = { type: 'valkey'; client?: KeyvValkeyOptions; - cluster?: RedisClusterOptions; + cluster?: { + rootNodes: Array; + options?: ClusterOptions; + }; }; /** diff --git a/yarn.lock b/yarn.lock index 13b51ed309..992da2bdc8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2566,6 +2566,7 @@ __metadata: helmet: "npm:^6.0.0" http-errors: "npm:^2.0.0" infinispan: "npm:^0.12.0" + iovalkey: "npm:^0.3.3" is-glob: "npm:^4.0.3" jose: "npm:^5.0.0" keyv: "npm:^5.2.1"