Skip to content

Commit

Permalink
Support evaluating quotas as bigint
Browse files Browse the repository at this point in the history
- Value we get from vault is number, but converted into bigint
  internally
- Value from the BucketInfo class is already bigint
- Comparison will use the number received from Scuba as a bigint
- Values may be imprecise but comparing with bigint ensures
  the comparison is correct as per the received values

Issue: CLDSRV-606
  • Loading branch information
williamlardier committed Jan 24, 2025
1 parent a7378cd commit d907318
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 53 deletions.
2 changes: 1 addition & 1 deletion lib/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ const api = {
if (err) {
return callback(err);
}
request.accountQuotas = BigInt(infos?.accountQuota || 0);
request.accountQuotas = infos?.accountQuotas;
if (authorizationResults) {
const checkedResults = checkAuthResults(authorizationResults);
if (checkedResults instanceof Error) {
Expand Down
14 changes: 8 additions & 6 deletions lib/api/apiUtils/quotas/quotaUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function _evaluateQuotas(
return parallelDone(err);
}
if (!isMetricStale(bucketMetrics, 'bucket', bucket.getName(), action, inflight, log) &&
bucketMetrics.bytesTotal + inflightForCheck > bucketQuota) {
BigInt(bucketMetrics.bytesTotal || 0) + BigInt(inflightForCheck || 0) > bucketQuota) {
log.debug('Bucket quota exceeded', {
bucket: bucket.getName(),
action,
Expand All @@ -147,8 +147,9 @@ function _evaluateQuotas(
if (err || inflight < 0) {
return parallelDone(err);
}
// Metrics are served as BigInt strings
if (!isMetricStale(accountMetrics, 'account', account.account, action, inflight, log) &&
accountMetrics.bytesTotal + inflightForCheck > accountQuota) {
BigInt(accountMetrics.bytesTotal || 0) + BigInt(inflightForCheck || 0) > accountQuota) {
log.debug('Account quota exceeded', {
accountId: account.account,
action,
Expand Down Expand Up @@ -215,8 +216,9 @@ function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight,
let accountQuotaExceeded = false;
let quotaEvaluationDuration;
const requestStartTime = process.hrtime.bigint();
const bucketQuota = bucket.getQuota();
const accountQuota = account?.quota || 0;
const bucketQuota = bucket.getQuota() || 0n;
// Quota is sent as a number by the authorization service
const accountQuota = BigInt(account?.quota || 0);
const shouldSendInflights = config.isQuotaInflightEnabled();

if (bucketQuota && accountQuota) {
Expand All @@ -231,8 +233,8 @@ function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight,
type = 'delete';
}

if ((bucketQuota <= 0 && accountQuota <= 0) || !QuotaService?.enabled) {
if (bucketQuota > 0 || accountQuota > 0) {
if ((bucketQuota <= 0n && accountQuota <= 0n) || !QuotaService?.enabled) {
if (bucketQuota > 0n || accountQuota > 0n) {
log.warn('quota is set for a bucket, but the quota service is disabled', {
bucketName: bucket.getName(),
});
Expand Down
2 changes: 1 addition & 1 deletion lib/routes/routeBackbeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,7 @@ function routeBackbeat(clientIP, request, response, log) {
return responseJSONBody(err, null, response, log);
}
// eslint-disable-next-line no-param-reassign
request.accountQuotas = BigInt(infos?.accountQuota || 0);
request.accountQuotas = infos?.accountQuotas;
// FIXME for now, any authenticated user can access API
// routes. We should introduce admin accounts or accounts
// with admin privileges, and restrict access to those
Expand Down
Loading

0 comments on commit d907318

Please sign in to comment.