Skip to content

Commit 185e209

Browse files
tmacrotmacro
committed
impr(CLDSRV-855): Refactor helpers to extract bucket and account configs
1 parent 4f3352e commit 185e209

5 files changed

Lines changed: 574 additions & 192 deletions

File tree

‎lib/api/apiUtils/rateLimit/helpers.js‎

Lines changed: 131 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -5,102 +5,159 @@ const { policies: { actionMaps: { actionMapBucketRateLimit } } } = require('arse
55

66
const rateLimitApiActions = Object.keys(actionMapBucketRateLimit);
77

8+
function requestNeedsRateCheck(request) {
9+
// Is the feature enabled?
10+
if (!config.rateLimiting.enabled) {
11+
return false;
12+
}
13+
14+
// Is the request an internal or rate limit admin operation?
15+
if (rateLimitApiActions.includes(request.apiMethod) || request.isInternalServiceRequest) {
16+
return false;
17+
}
18+
19+
// Have we already checked both bucket and account configs?
20+
return !(request.rateLimitAccountAlreadyChecked && request.rateLimitBucketAlreadyChecked);
21+
}
22+
823
/**
9-
* Extract rate limit configuration from bucket metadata or global rate limit configuration.
24+
* Extract bucket rate limit configuration from bucket metadata or global rate limit configuration.
1025
*
1126
* Resolves in priority order:
1227
* 1. Per-bucket configuration (from bucket metadata)
1328
* 2. Global default configuration
14-
* 3. No rate limiting (null)
1529
*
1630
* @param {object} bucket - Bucket metadata object
17-
* @param {string} bucketName - Bucket name
1831
* @param {object} log - Logger instance
19-
* @returns {object|null} Rate limit config or null if no limit
32+
* @returns {object} Rate limit config
2033
*/
21-
function extractBucketRateLimitConfig(bucket, bucketName, log) {
34+
function extractBucketRateLimitConfig(bucketMD, log) {
2235
// Try per-bucket config first
23-
const bucketConfig = bucket.getRateLimitConfiguration();
36+
const bucketConfig = bucketMD.getRateLimitConfiguration();
2437
if (bucketConfig) {
2538
const data = bucketConfig.getData();
26-
const limitConfig = {
27-
limit: data.RequestsPerSecond.Limit,
28-
burstCapacity: config.rateLimiting.bucket.defaultBurstCapacity * 1000,
29-
source: 'bucket',
39+
40+
const merged = {
41+
RequestsPerSecond: {
42+
...config.rateLimiting.bucket.defaultConfig.RequestsPerSecond,
43+
...(data.RequestsPerSecond || {}),
44+
source: data.RequestsPerSecond !== undefined ? 'resource' : 'global',
45+
},
3046
};
3147

3248
log.debug('Extracted per-bucket rate limit config', {
33-
bucketName,
34-
limit: limitConfig.limit,
35-
burstCapacity: config.rateLimiting.bucket.defaultBurstCapacity * 1000,
49+
bucketName: bucketMD.getName(),
50+
cfg: merged,
3651
});
3752

38-
return limitConfig;
53+
return merged;
3954
}
4055

41-
// Fall back to global default config
42-
const globalLimit = config.rateLimiting.bucket?.defaultConfig?.limit;
43-
if (globalLimit !== undefined && globalLimit > 0) {
44-
const limitConfig = {
45-
limit: globalLimit,
46-
burstCapacity: config.rateLimiting.bucket.defaultBurstCapacity * 1000,
56+
log.debug('Using global default rate limit config', {
57+
bucketName: bucketMD.getName(),
58+
cfg: {
59+
...config.rateLimiting.bucket.defaultConfig,
60+
source: 'global',
61+
}
62+
});
63+
64+
return {
65+
RequestsPerSecond: {
66+
...config.rateLimiting.bucket.defaultConfig.RequestsPerSecond,
4767
source: 'global',
68+
},
69+
};
70+
}
71+
72+
/**
73+
* Extract account rate limit configuration from request or global rate limit configuration.
74+
*
75+
* Resolves in priority order:
76+
* 1. Per-account configuration (from request)
77+
* 2. Global default configuration
78+
*
79+
* @param {object} authInfo - Instance of AuthInfo class with requester's info
80+
* @param {object} request - request object given by router
81+
* @param {object} log - Logger instance
82+
* @returns {object} Rate limit config
83+
*/
84+
function extractAccountRateLimitConfig(authInfo, request, log) {
85+
// Try per-account config first
86+
if (request.accountLimits) {
87+
const merged = {
88+
RequestsPerSecond: {
89+
...config.rateLimiting.account.defaultConfig.RequestsPerSecond,
90+
...(request.accountLimits.RequestsPerSecond || {}),
91+
source: request.accountLimits.RequestsPerSecond !== undefined ? 'resource' : 'global',
92+
},
4893
};
4994

50-
log.debug('Using global default rate limit config', {
51-
bucketName,
52-
limit: limitConfig.limit,
95+
log.debug('Extracted per-account rate limit config', {
96+
accountId: authInfo.getCanonicalID(),
97+
cfg: merged,
5398
});
5499

55-
return limitConfig;
100+
return merged;
56101
}
57102

58-
// No rate limiting configured - cache null to avoid repeated lookups
59-
log.trace('No rate limit configured for bucket', { bucketName });
60-
return null;
61-
}
103+
const cfg = {
104+
RequestsPerSecond: {
105+
...config.rateLimiting.account.defaultConfig.RequestsPerSecond,
106+
source: 'global',
107+
},
108+
};
62109

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
110+
log.debug('Using global default rate limit config', {
111+
accountId: authInfo.getCanonicalID(),
112+
cfg,
113+
});
67114

68-
if (!applyRateLimit) {
69-
return { needsCheck: false };
70-
}
115+
return cfg;
116+
}
71117

72-
const limitConfigs = {};
73-
let needsCheck = false;
118+
function extractRateLimitConfigFromRequest(request, authInfo, bucketMD, log) {
119+
const limitConfig = {
120+
bucket: extractBucketRateLimitConfig(bucketMD, log),
121+
account: extractAccountRateLimitConfig(authInfo, request, log),
122+
};
123+
return limitConfig;
124+
}
74125

75-
if (request.accountLimits) {
76-
limitConfigs.account = {
77-
...request.accountLimits,
78-
source: 'account',
79-
};
80-
needsCheck = true;
126+
function getCachedRateLimitConfig(request) {
127+
const cachedConfig = {};
128+
const cachedBucketConfig = cache.getCachedConfig(cache.namespace.bucket, request.bucketName);
129+
if (cachedBucketConfig !== undefined) {
130+
cachedConfig.bucket = cachedBucketConfig;
81131
}
82132

83-
if (request.bucketName) {
84-
const cachedConfig = cache.getCachedConfig(cache.namespace.bucket, request.bucketName);
85-
if (cachedConfig) {
86-
limitConfigs.bucket = cachedConfig;
87-
needsCheck = true;
133+
const cachedOwner = cache.getCachedBucketOwner(request.bucketName);
134+
if (cachedOwner !== undefined) {
135+
const cachedAccountConfig = cache.getCachedConfig(cache.namespace.account, cachedOwner);
136+
if (cachedAccountConfig !== undefined) {
137+
cachedConfig.account = cachedAccountConfig;
138+
cachedConfig.bucketOwner = cachedOwner;
88139
}
140+
}
89141

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-
}
142+
return cachedConfig;
143+
}
144+
145+
function buildRateChecksFromConfig(resourceClass, resourceId, limitConfig) {
146+
const checks = [];
147+
if (limitConfig?.RequestsPerSecond?.Limit > 0) {
148+
checks.push({
149+
resourceClass,
150+
resourceId,
151+
measure: 'rps',
152+
source: limitConfig.RequestsPerSecond.source,
153+
config: {
154+
limit: limitConfig.RequestsPerSecond.Limit,
155+
burstCapacity: limitConfig.RequestsPerSecond.BurstCapacity * 1000,
156+
},
157+
});
101158
}
102159

103-
return { needsCheck, limitConfigs };
160+
return checks;
104161
}
105162

106163
function checkRateLimitsForRequest(checks, log) {
@@ -109,11 +166,11 @@ function checkRateLimitsForRequest(checks, log) {
109166
const bucket = getTokenBucket(check.resourceClass, check.resourceId, check.measure, check.config, log);
110167
if (!bucket.hasCapacity()) {
111168
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,
169+
resourceClass: check.resourceClass,
170+
resourceId: check.resourceId,
171+
measure: check.measure,
172+
limit: check.config.limit,
173+
source: check.source,
117174
});
118175

119176
return { allowed: false, rateLimitSource: `${check.resourceClass}:${check.source}`};
@@ -122,13 +179,15 @@ function checkRateLimitsForRequest(checks, log) {
122179
buckets.push(bucket);
123180

124181
log.trace('Rate limit check: allowed (token consumed)', {
125-
resourceClass: bucket.resourceClass,
126-
resourceId: bucket.resourceId,
127-
measure: bucket.measure,
128-
source: bucket.limitConfig.source,
182+
resourceClass: check.resourceClass,
183+
resourceId: check.resourceId,
184+
measure: check.measure,
185+
source: check.source,
129186
});
130187
}
131188

189+
// buckets have already been checked for available capacity
190+
// no need to check return values
132191
buckets.forEach(bucket => bucket.tryConsume());
133192
return { allowed: true };
134193
}
@@ -137,5 +196,8 @@ module.exports = {
137196
rateLimitApiActions,
138197
extractBucketRateLimitConfig,
139198
extractRateLimitConfigFromRequest,
199+
buildRateChecksFromConfig,
140200
checkRateLimitsForRequest,
201+
getCachedRateLimitConfig,
202+
requestNeedsRateCheck,
141203
};

‎lib/api/apiUtils/rateLimit/tokenBucket.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ function getTokenBucket(resourceClass, resourceId, measure, limitConfig, log) {
200200
} else {
201201
const { updated, oldConfig } = bucket.updateLimit(limitConfig);
202202
if (updated) {
203-
log.info('Updated token bucket limit config', {
203+
log.debug('Updated token bucket limit config', {
204204
cacheKey,
205205
old: oldConfig,
206206
new: limitConfig,

0 commit comments

Comments
 (0)