Skip to content

Commit b995e4d

Browse files
committed
impr(CLDSRV-852): Refactor rate limit helpers
1 parent 0b485b4 commit b995e4d

2 files changed

Lines changed: 212 additions & 279 deletions

File tree

Lines changed: 71 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,12 @@
11
const { config } = require('../../../Config');
22
const cache = require('./cache');
3-
const constants = require('../../../../constants');
43
const { getTokenBucket } = require('./tokenBucket');
54
const { policies: { actionMaps: { actionMapBucketRateLimit } } } = require('arsenal');
65

76
const rateLimitApiActions = Object.keys(actionMapBucketRateLimit);
87

98
/**
10-
* Get rate limit configuration from cache only (no metadata fetch)
11-
*
12-
* @param {string} bucketName - Bucket name
13-
* @returns {object|null|undefined} Rate limit config, null (no limit), or undefined (not cached)
14-
*/
15-
function getRateLimitFromCache(bucketName) {
16-
const cacheKey = `bucket:${bucketName}`;
17-
return cache.getCachedConfig(cacheKey);
18-
}
19-
20-
/**
21-
* Extract rate limit configuration from bucket metadata and cache it
9+
* Extract rate limit configuration from bucket metadata or global rate limit configuration.
2210
*
2311
* Resolves in priority order:
2412
* 1. Per-bucket configuration (from bucket metadata)
@@ -30,24 +18,21 @@ function getRateLimitFromCache(bucketName) {
3018
* @param {object} log - Logger instance
3119
* @returns {object|null} Rate limit config or null if no limit
3220
*/
33-
function extractAndCacheRateLimitConfig(bucket, bucketName, log) {
34-
const cacheKey = `bucket:${bucketName}`;
35-
const cacheTTL = config.rateLimiting.bucket?.configCacheTTL ||
36-
constants.rateLimitDefaultConfigCacheTTL;
37-
21+
function extractBucketRateLimitConfig(bucket, bucketName, log) {
3822
// Try per-bucket config first
3923
const bucketConfig = bucket.getRateLimitConfiguration();
4024
if (bucketConfig) {
4125
const data = bucketConfig.getData();
4226
const limitConfig = {
4327
limit: data.RequestsPerSecond.Limit,
28+
burstCapacity: config.rateLimiting.bucket.defaultBurstCapacity * 1000,
4429
source: 'bucket',
4530
};
4631

47-
cache.setCachedConfig(cacheKey, limitConfig, cacheTTL);
4832
log.debug('Extracted per-bucket rate limit config', {
4933
bucketName,
5034
limit: limitConfig.limit,
35+
burstCapacity: config.rateLimiting.bucket.defaultBurstCapacity * 1000,
5136
});
5237

5338
return limitConfig;
@@ -58,10 +43,10 @@ function extractAndCacheRateLimitConfig(bucket, bucketName, log) {
5843
if (globalLimit !== undefined && globalLimit > 0) {
5944
const limitConfig = {
6045
limit: globalLimit,
46+
burstCapacity: config.rateLimiting.bucket.defaultBurstCapacity * 1000,
6147
source: 'global',
6248
};
6349

64-
cache.setCachedConfig(cacheKey, limitConfig, cacheTTL);
6550
log.debug('Using global default rate limit config', {
6651
bucketName,
6752
limit: limitConfig.limit,
@@ -71,58 +56,86 @@ function extractAndCacheRateLimitConfig(bucket, bucketName, log) {
7156
}
7257

7358
// No rate limiting configured - cache null to avoid repeated lookups
74-
cache.setCachedConfig(cacheKey, null, cacheTTL);
7559
log.trace('No rate limit configured for bucket', { bucketName });
76-
7760
return null;
7861
}
7962

80-
/**
81-
* Check rate limit with pre-resolved configuration using token reservation
82-
*
83-
* Uses token bucket: Workers maintain local tokens granted by Redis.
84-
* Token consumption is pure in-memory (fast). Refills happen async in background.
85-
*
86-
* @param {string} bucketName - Bucket name
87-
* @param {object|null} limitConfig - Pre-resolved rate limit config
88-
* @param {object} log - Logger instance
89-
* @param {function} callback - Callback(err, rateLimited boolean)
90-
* @returns {undefined}
91-
*/
92-
function checkRateLimitWithConfig(bucketName, limitConfig, log, callback) {
93-
// No rate limiting configured
94-
if (!limitConfig || limitConfig.limit === 0) {
95-
return callback(null, false);
63+
function extractRateLimitConfigFromRequest(request) {
64+
const applyRateLimit = config.rateLimiting?.enabled
65+
&& !rateLimitApiActions.includes(request.apiMethod) // Don't limit any rate limit admin actions
66+
&& !request.isInternalServiceRequest; // Don't limit any calls from internal services
67+
68+
if (!applyRateLimit) {
69+
return { needsCheck: false };
70+
}
71+
72+
const limitConfigs = {};
73+
let needsCheck = false;
74+
75+
if (request.accountLimits) {
76+
limitConfigs.account = {
77+
...request.accountLimits,
78+
source: 'account',
79+
};
80+
needsCheck = true;
81+
}
82+
83+
if (request.bucketName) {
84+
const cachedConfig = cache.getCachedConfig(cache.namespace.bucket, request.bucketName);
85+
if (cachedConfig) {
86+
limitConfigs.bucket = cachedConfig;
87+
needsCheck = true;
88+
}
89+
90+
if (!request.accountLimits) {
91+
const cachedOwner = cache.getCachedBucketOwner(request.bucketName);
92+
if (cachedOwner !== undefined) {
93+
const cachedConfig = cache.getCachedConfig(cache.namespace.account, cachedOwner);
94+
if (cachedConfig) {
95+
limitConfigs.account = cachedConfig;
96+
limitConfigs.bucketOwner = cachedOwner;
97+
needsCheck = true;
98+
}
99+
}
100+
}
96101
}
97102

98-
// Get or create token bucket for this bucket
99-
const tokenBucket = getTokenBucket(bucketName, limitConfig, log);
103+
return { needsCheck, limitConfigs };
104+
}
105+
106+
function checkRateLimitsForRequest(checks, log) {
107+
const buckets = [];
108+
for (const check of checks) {
109+
const bucket = getTokenBucket(check.resourceClass, check.resourceId, check.measure, check.config, log);
110+
if (!bucket.hasCapacity()) {
111+
log.debug('Rate limit check: denied (no tokens available)', {
112+
resourceClass: bucket.resourceClass,
113+
resourceId: bucket.resourceId,
114+
measure: bucket.measure,
115+
limit: bucket.limitConfig.limit,
116+
source: bucket.limitConfig.source,
117+
});
100118

101-
// Try to consume a token (in-memory, no Redis)
102-
const allowed = tokenBucket.tryConsume();
119+
return { allowed: false, rateLimitSource: `${check.resourceClass}:${check.source}`};
120+
}
121+
122+
buckets.push(bucket);
103123

104-
if (allowed) {
105124
log.trace('Rate limit check: allowed (token consumed)', {
106-
bucketName,
107-
tokensRemaining: tokenBucket.tokens,
108-
});
109-
} else {
110-
log.debug('Rate limit check: denied (no tokens available)', {
111-
bucketName,
112-
limit: limitConfig.limit,
113-
source: limitConfig.source,
125+
resourceClass: bucket.resourceClass,
126+
resourceId: bucket.resourceId,
127+
measure: bucket.measure,
128+
source: bucket.limitConfig.source,
114129
});
115130
}
116131

117-
// Return inverse: callback expects "rateLimited" boolean
118-
// allowed=true → rateLimited=false
119-
// allowed=false → rateLimited=true
120-
return callback(null, !allowed);
132+
buckets.forEach(bucket => bucket.tryConsume());
133+
return { allowed: true };
121134
}
122135

123136
module.exports = {
124-
getRateLimitFromCache,
125-
extractAndCacheRateLimitConfig,
126-
checkRateLimitWithConfig,
127137
rateLimitApiActions,
138+
extractBucketRateLimitConfig,
139+
extractRateLimitConfigFromRequest,
140+
checkRateLimitsForRequest,
128141
};

0 commit comments

Comments
 (0)