Merge branch 'bugfix/CLDSRV-828-fix-limit-override-bug' into q/9.2

Author: bert-e

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

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,
 };

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);
         });

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);
     });

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);
+    });
 });

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', () => {