From 20b381ab8072f42f6f5f3072264b118be77ba512 Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:25:54 +0100 Subject: [PATCH 1/4] fix(rateLimit): update token bucket on limit change The token bucket was created once with the initial limitConfig and never updated when the rate limit changed dynamically. This caused the GCRA interval calculation to use stale limits. Changes: - getTokenBucket() now updates limitConfig when limit changes - Add removeTokenBucket() for cleanup on rate limit deletion - Add invalidateCachedConfig() for immediate cache invalidation Issue: CLDSRV-828 --- lib/api/apiUtils/rateLimit/cache.js | 12 ++++++++++++ lib/api/apiUtils/rateLimit/tokenBucket.js | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lib/api/apiUtils/rateLimit/cache.js b/lib/api/apiUtils/rateLimit/cache.js index 708edeb5f8..a9bef0c1cd 100644 --- a/lib/api/apiUtils/rateLimit/cache.js +++ b/lib/api/apiUtils/rateLimit/cache.js @@ -39,10 +39,22 @@ function expireCachedConfigs(now) { return toRemove.length; } +/** + * Invalidate cached config for a specific bucket + * + * @param {string} bucketName - Bucket name + * @returns {boolean} True if entry was found and removed + */ +function invalidateCachedConfig(bucketName) { + const cacheKey = `bucket:${bucketName}`; + return configCache.delete(cacheKey); +} + module.exports = { setCachedConfig, getCachedConfig, expireCachedConfigs, + invalidateCachedConfig, // Do not access directly // Used only for tests diff --git a/lib/api/apiUtils/rateLimit/tokenBucket.js b/lib/api/apiUtils/rateLimit/tokenBucket.js index 6115863e0b..26b7850187 100644 --- a/lib/api/apiUtils/rateLimit/tokenBucket.js +++ b/lib/api/apiUtils/rateLimit/tokenBucket.js @@ -247,6 +247,16 @@ function getTokenBucket(bucketName, limitConfig, log) { bufferSize: bucket.bufferSize, refillThreshold: bucket.refillThreshold, }); + } else if (bucket.limitConfig.limit !== limitConfig.limit) { + // Update limit config when it changes dynamically + const oldLimit = bucket.limitConfig.limit; + bucket.limitConfig = limitConfig; + + log.info('Updated token bucket limit config', { + bucketName, + oldLimit, + newLimit: limitConfig.limit, + }); } return bucket; @@ -286,9 +296,20 @@ function cleanupTokenBuckets(maxIdleMs = 60000) { return toRemove.length; } +/** + * Remove a specific token bucket (used when rate limit config is deleted) + * + * @param {string} bucketName - Bucket name + * @returns {boolean} True if bucket was found and removed + */ +function removeTokenBucket(bucketName) { + return tokenBuckets.delete(bucketName); +} + module.exports = { WorkerTokenBucket, getTokenBucket, getAllTokenBuckets, cleanupTokenBuckets, + removeTokenBucket, }; From d2d05348a1146827058eaa8f6544f5cef3e80367 Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:26:00 +0100 Subject: [PATCH 2/4] fix(rateLimit): invalidate cache on PUT/DELETE Without cache invalidation, rate limit changes took up to 30 seconds (cache TTL) to take effect. This also caused stale rateLimitSource in server access logs used by monitoring. Changes: - PUT: invalidate cache after metadata update - DELETE: invalidate cache and remove token bucket Issue: CLDSRV-828 --- lib/api/bucketDeleteRateLimit.js | 6 ++++++ lib/api/bucketPutRateLimit.js | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/lib/api/bucketDeleteRateLimit.js b/lib/api/bucketDeleteRateLimit.js index fc4d28a54e..81bfde32eb 100644 --- a/lib/api/bucketDeleteRateLimit.js +++ b/lib/api/bucketDeleteRateLimit.js @@ -4,6 +4,8 @@ const metadata = require('../metadata/wrapper'); const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const { isRateLimitServiceUser } = require('./apiUtils/authorization/serviceUser'); +const { invalidateCachedConfig } = require('./apiUtils/rateLimit/cache'); +const { removeTokenBucket } = require('./apiUtils/rateLimit/tokenBucket'); /** * bucketDeleteRateLimit - Delete the bucket rate limit configuration @@ -49,6 +51,10 @@ function bucketDeleteRateLimit(authInfo, request, log, callback) { if (err) { return callback(err, corsHeaders); } + // Invalidate cache and remove token bucket + invalidateCachedConfig(bucketName); + removeTokenBucket(bucketName); + log.debug('invalidated rate limit cache and token bucket for bucket', { bucketName }); // TODO: implement Utapi metric support return callback(null, corsHeaders); }); diff --git a/lib/api/bucketPutRateLimit.js b/lib/api/bucketPutRateLimit.js index 4d3e55ea0a..116c8940d9 100644 --- a/lib/api/bucketPutRateLimit.js +++ b/lib/api/bucketPutRateLimit.js @@ -6,6 +6,7 @@ const { config } = require('../Config'); const metadata = require('../metadata/wrapper'); const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { isRateLimitServiceUser } = require('./apiUtils/authorization/serviceUser'); +const { invalidateCachedConfig } = require('./apiUtils/rateLimit/cache'); function parseRequestBody(requestBody, callback) { try { @@ -92,6 +93,9 @@ function bucketPutRateLimit(authInfo, request, log, callback) { { error: err, method: 'bucketPutRateLimit' }); return callback(err, corsHeaders); } + // Invalidate cache so new limit takes effect immediately + invalidateCachedConfig(bucketName); + log.debug('invalidated rate limit cache for bucket', { bucketName }); // TODO: implement Utapi metric support return callback(null, corsHeaders); }); From c62dd51bb859601d6d5e61b93c23d49e1a1ec68c Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:26:06 +0100 Subject: [PATCH 3/4] test(rateLimit): add tests for dynamic limit updates Add unit tests covering the cache invalidation and token bucket update functionality introduced in the previous commits. Tests added: - getTokenBucket updates limitConfig when limit changes - getTokenBucket does not log when limit unchanged - removeTokenBucket removes existing bucket - removeTokenBucket returns false for non-existent bucket - invalidateCachedConfig removes specific bucket entry - invalidateCachedConfig returns false for non-existent Issue: CLDSRV-828 --- tests/unit/api/apiUtils/rateLimit/cache.js | 18 +++++++++ .../api/apiUtils/rateLimit/tokenBucket.js | 40 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/tests/unit/api/apiUtils/rateLimit/cache.js b/tests/unit/api/apiUtils/rateLimit/cache.js index fd3e1b7a1b..dc8b15552e 100644 --- a/tests/unit/api/apiUtils/rateLimit/cache.js +++ b/tests/unit/api/apiUtils/rateLimit/cache.js @@ -7,6 +7,7 @@ const { getCachedConfig, setCachedConfig, expireCachedConfigs, + invalidateCachedConfig, } = require('../../../../../lib/api/apiUtils/rateLimit/cache'); describe('test limit config cache storage', () => { @@ -66,4 +67,21 @@ describe('test limit config cache storage', () => { config: 10, }); }); + + it('should invalidate cached config for a specific bucket', () => { + setCachedConfig('bucket:my-bucket', { limit: 100 }, constants.rateLimitDefaultConfigCacheTTL); + setCachedConfig('bucket:other-bucket', { limit: 200 }, constants.rateLimitDefaultConfigCacheTTL); + + const result = invalidateCachedConfig('my-bucket'); + + assert.strictEqual(result, true); + assert.strictEqual(getCachedConfig('bucket:my-bucket'), undefined); + assert.deepStrictEqual(getCachedConfig('bucket:other-bucket'), { limit: 200 }); + }); + + it('should return false when invalidating non-existent bucket', () => { + const result = invalidateCachedConfig('non-existent-bucket'); + + assert.strictEqual(result, false); + }); }); diff --git a/tests/unit/api/apiUtils/rateLimit/tokenBucket.js b/tests/unit/api/apiUtils/rateLimit/tokenBucket.js index 4d9743a260..fcb635416b 100644 --- a/tests/unit/api/apiUtils/rateLimit/tokenBucket.js +++ b/tests/unit/api/apiUtils/rateLimit/tokenBucket.js @@ -377,6 +377,46 @@ describe('Token bucket management functions', () => { assert.strictEqual(bucket1.bucketName, 'bucket-1'); assert.strictEqual(bucket2.bucketName, 'bucket-2'); }); + + it('should update limitConfig when limit changes', () => { + const bucket1 = tokenBucket.getTokenBucket('test-bucket', { limit: 100, source: 'bucket' }, mockLog); + assert.strictEqual(bucket1.limitConfig.limit, 100); + + // Simulate rate limit change + const bucket2 = tokenBucket.getTokenBucket('test-bucket', { limit: 200, source: 'bucket' }, mockLog); + + assert.strictEqual(bucket1, bucket2); // Same bucket instance + assert.strictEqual(bucket2.limitConfig.limit, 200); // Limit updated + assert(mockLog.info.calledOnce); + assert(mockLog.info.firstCall.args[0].includes('Updated token bucket limit config')); + }); + + it('should not log update when limit is unchanged', () => { + tokenBucket.getTokenBucket('test-bucket', { limit: 100 }, mockLog); + mockLog.info.resetHistory(); + + tokenBucket.getTokenBucket('test-bucket', { limit: 100 }, mockLog); + + assert.strictEqual(mockLog.info.called, false); + }); + }); + + describe('removeTokenBucket', () => { + it('should remove existing bucket and return true', () => { + tokenBucket.getTokenBucket('test-bucket', { limit: 100 }, mockLog); + assert.strictEqual(tokenBucket.getAllTokenBuckets().size, 1); + + const result = tokenBucket.removeTokenBucket('test-bucket'); + + assert.strictEqual(result, true); + assert.strictEqual(tokenBucket.getAllTokenBuckets().size, 0); + }); + + it('should return false when bucket does not exist', () => { + const result = tokenBucket.removeTokenBucket('non-existent-bucket'); + + assert.strictEqual(result, false); + }); }); describe('getAllTokenBuckets', () => { From 456bd7c8c06a64453ae4a338c6d7988505ac1cba Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Tue, 20 Jan 2026 07:30:34 +0100 Subject: [PATCH 4/4] CLDSRV-828: bump package.json from 9.2.15 to 9.2.16 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e6fe2b0bdb..8f9a7c4511 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "9.2.15", + "version": "9.2.16", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": {