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, }; 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); }); 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": { 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', () => {