Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/api/apiUtils/rateLimit/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions lib/api/apiUtils/rateLimit/tokenBucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
};
6 changes: 6 additions & 0 deletions lib/api/bucketDeleteRateLimit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
Expand Down
4 changes: 4 additions & 0 deletions lib/api/bucketPutRateLimit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/api/apiUtils/rateLimit/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
getCachedConfig,
setCachedConfig,
expireCachedConfigs,
invalidateCachedConfig,
} = require('../../../../../lib/api/apiUtils/rateLimit/cache');

describe('test limit config cache storage', () => {
Expand Down Expand Up @@ -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);
});
});
40 changes: 40 additions & 0 deletions tests/unit/api/apiUtils/rateLimit/tokenBucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading