-
Notifications
You must be signed in to change notification settings - Fork 252
CLDSRV-759: bucketGetRateLimit #5997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tmacro
wants to merge
6
commits into
improvement/CLDSRV-766/bucket_rate_limiting
Choose a base branch
from
improvement/CLDSRV-759/GetBucketRateLimit
base: improvement/CLDSRV-766/bucket_rate_limiting
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c353bab
impr(CLDSRV-759): Bump arsenal to branch
tmacro de2ed07
impr(CLDSRV-759): Adapt handler code to arsenal changes
tmacro 074e87e
impr(CLDSRV-759): Add $RATE_LIMIT_SERVICE_USER_ARN to docker-entrypoi…
tmacro 94e3fae
impr(CLDSRV-759): Setup rate limit config in CI
tmacro 422cd7b
impr(CLDSRV-759): Add rate limit test tooling
tmacro c6dc6d9
impr(CLDSRV-759): Add tests for bucketGetRateLimit
tmacro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
60 changes: 60 additions & 0 deletions
60
tests/functional/aws-node-sdk/test/bucket/getBucketRateLimit.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| const AWS = require('aws-sdk'); | ||
| const S3 = AWS.S3; | ||
| const assert = require('assert'); | ||
| const getConfig = require('../support/config'); | ||
| const { sendRateLimitRequest, skipIfRateLimitDisabled } = require('../rateLimit/tooling'); | ||
|
|
||
| const bucket = 'getratelimitestbucket'; | ||
| const rateLimitConfig = { RequestsPerSecond: 100 }; | ||
|
|
||
| skipIfRateLimitDisabled('Test get bucket rate limit', () => { | ||
| let s3; | ||
|
|
||
| before(() => { | ||
| const config = getConfig('lisa', { signatureVersion: 'v4' }); | ||
| s3 = new S3(config); | ||
| AWS.config.update(config); | ||
| }); | ||
|
|
||
| beforeEach(done => s3.createBucket({ Bucket: bucket }, done)); | ||
|
|
||
| afterEach(done => s3.deleteBucket({ Bucket: bucket }, done)); | ||
|
|
||
| it('should return the rate limit config', async () => { | ||
| try { | ||
| // First set the rate limit config | ||
| await sendRateLimitRequest('PUT', '127.0.0.1:8000', | ||
| `/${bucket}/?rate-limit`, JSON.stringify(rateLimitConfig)); | ||
|
|
||
| // Then get it | ||
| const data = await sendRateLimitRequest('GET', '127.0.0.1:8000', | ||
| `/${bucket}/?rate-limit`); | ||
| assert.strictEqual(data.RequestsPerSecond.Limit, 100); | ||
| } catch (err) { | ||
| assert.ifError(err); | ||
| } | ||
| }); | ||
|
|
||
| it('should return NoSuchRateLimitConfig error when config does not exist', async () => { | ||
| try { | ||
| await sendRateLimitRequest('GET', '127.0.0.1:8000', `/${bucket}/?rate-limit`); | ||
| assert.fail('Expected NoSuchRateLimitConfig error'); | ||
| } catch (err) { | ||
| assert.strictEqual(err.Error.Code[0], 'NoSuchRateLimitConfig'); | ||
| } | ||
| }); | ||
|
|
||
| it('should return NoSuchBucket error when bucket does not exist', async () => { | ||
| try { | ||
| await sendRateLimitRequest('GET', '127.0.0.1:8000', '/nonexistentbucket/?rate-limit'); | ||
| } catch (err) { | ||
| assert.strictEqual(err.Error.Code[0], 'NoSuchBucket'); | ||
| } | ||
| }); | ||
|
|
||
| it('should return AccessDenied error for non-service user', async () => { | ||
| // This test would require making a request with regular user credentials | ||
| // For now, we'll skip this as it requires additional setup | ||
| // In a real scenario, you'd use regular user credentials here | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| const assert = require('assert'); | ||
| const sinon = require('sinon'); | ||
| const { bucketPut } = require('../../../lib/api/bucketPut'); | ||
| const { cleanup, DummyRequestLogger, makeAuthInfo } = require('../helpers'); | ||
| const bucketGetRateLimit = require('../../../lib/api/bucketGetRateLimit'); | ||
| const bucketPutRateLimit = require('../../../lib/api/bucketPutRateLimit'); | ||
| const { config } = require('../../../lib/Config'); | ||
| const AuthInfo = require('arsenal').auth.AuthInfo; | ||
|
|
||
| const log = new DummyRequestLogger(); | ||
| const bucketName = 'bucketname'; | ||
| const serviceUserArn = 'arn:aws:iam::123456789012:user/rate-limit-service'; | ||
|
|
||
| // Create a rate limit service user authInfo | ||
| function makeRateLimitServiceUserAuthInfo() { | ||
| return new AuthInfo({ | ||
| canonicalID: '79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be', | ||
| shortid: '123456789012', | ||
| email: '[email protected]', | ||
| accountDisplayName: 'rateLimitServiceDisplayName', | ||
| arn: serviceUserArn, | ||
| }); | ||
| } | ||
|
|
||
| const rateLimitServiceAuthInfo = makeRateLimitServiceUserAuthInfo(); | ||
| const regularAuthInfo = makeAuthInfo('accessKey1'); | ||
|
|
||
| const bucketPutReq = { | ||
| bucketName, | ||
| headers: { | ||
| host: `${bucketName}.s3.amazonaws.com`, | ||
| }, | ||
| url: '/', | ||
| actionImplicitDenies: false, | ||
| }; | ||
|
|
||
| function getRateLimitConfigRequest(bucketName) { | ||
| return { | ||
| bucketName, | ||
| headers: { | ||
| host: `${bucketName}.s3.amazonaws.com`, | ||
| }, | ||
| url: '/?rate-limit', | ||
| method: 'GET', | ||
| actionImplicitDenies: false, | ||
| }; | ||
| } | ||
|
|
||
| function getRateLimitPutRequest(bucketName, configJson) { | ||
| return { | ||
| bucketName, | ||
| headers: { | ||
| host: `${bucketName}.s3.amazonaws.com`, | ||
| }, | ||
| url: '/?rate-limit', | ||
| method: 'PUT', | ||
| post: JSON.stringify(configJson), | ||
| actionImplicitDenies: false, | ||
| }; | ||
| } | ||
|
|
||
| describe('bucketGetRateLimit API', () => { | ||
| let sandbox; | ||
|
|
||
| beforeEach(() => { | ||
| sandbox = sinon.createSandbox(); | ||
| sandbox.stub(config, 'rateLimiting').value({ | ||
| serviceUserArn, | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| sandbox.restore(); | ||
| cleanup(); | ||
| }); | ||
|
|
||
| it('should return AccessDenied error if user is not a rate limit service user', done => { | ||
| bucketPut(regularAuthInfo, bucketPutReq, log, err => { | ||
| assert.ifError(err); | ||
| const rateLimitRequest = getRateLimitConfigRequest(bucketName); | ||
| bucketGetRateLimit(regularAuthInfo, rateLimitRequest, log, err => { | ||
| assert.strictEqual(err.is.AccessDenied, true); | ||
| done(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| it('should return NoSuchRateLimitConfig error if bucket exists but ' + | ||
| 'rate limit config is not set', done => { | ||
| bucketPut(regularAuthInfo, bucketPutReq, log, err => { | ||
| assert.ifError(err); | ||
| const rateLimitRequest = getRateLimitConfigRequest(bucketName); | ||
| bucketGetRateLimit(rateLimitServiceAuthInfo, rateLimitRequest, log, err => { | ||
| assert.strictEqual(err.is.NoSuchRateLimitConfig, true); | ||
| done(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| it('should return bucket not found error if bucket does not exist', done => { | ||
| const rateLimitRequest = getRateLimitConfigRequest('nonexistent-bucket'); | ||
| bucketGetRateLimit(rateLimitServiceAuthInfo, rateLimitRequest, log, err => { | ||
| assert(err, 'should return an error'); | ||
| assert.strictEqual(err.is.NoSuchBucket, true); | ||
| done(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('bucketGetRateLimit API with rate limit config', () => { | ||
| let sandbox; | ||
|
|
||
| beforeEach(() => { | ||
| sandbox = sinon.createSandbox(); | ||
| sandbox.stub(config, 'rateLimiting').value({ | ||
| serviceUserArn, | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| sandbox.restore(); | ||
| cleanup(); | ||
| }); | ||
|
|
||
| beforeEach(done => { | ||
| bucketPut(regularAuthInfo, bucketPutReq, log, err => { | ||
| assert.ifError(err); | ||
| const rateLimitConfig = { RequestsPerSecond: 100 }; | ||
| const putRequest = getRateLimitPutRequest(bucketName, rateLimitConfig); | ||
| bucketPutRateLimit(rateLimitServiceAuthInfo, putRequest, log, err => { | ||
| assert.ifError(err); | ||
| done(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| it('should return rate limit configuration JSON when config exists', done => { | ||
| const rateLimitRequest = getRateLimitConfigRequest(bucketName); | ||
| bucketGetRateLimit(rateLimitServiceAuthInfo, rateLimitRequest, log, (err, res) => { | ||
| assert.ifError(err); | ||
| const rateLimitConfig = JSON.parse(res); | ||
| assert.strictEqual(rateLimitConfig.RequestsPerSecond.Limit, 100); | ||
| done(); | ||
| }); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add an
assert.failhere too like in the above test