-
Notifications
You must be signed in to change notification settings - Fork 242
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
CLDSRV-584 Limit backbeat API versioning check to replication operations
Context: In Artesca, when creating a location (whether used for replication or not), you specify both the endpoint and the bucket name where data will be stored. At this stage, Zenko checks that the bucket has versioning enabled. In S3C, it is a bit different, a “replication location” consists of a list of S3C endpoints (replicationEndpoints), and the destination bucket is specified later in the API replication rule (with put-bucket-replication API). Currently, S3C does not check if the destination bucket has versioning enabled. This means data could be replicated from a versioned source bucket to a non-versioned or suspended destination bucket, which can cause issues. e.g. https://scality.atlassian.net/browse/S3C-821 Current solution: A validation step was added to the Backbeat API in cloudserver. This check rejects any requests made to buckets that either do not have versioning enabled or have versioning suspended. This check makes sure the Backbeat API only works with buckets that have versioning enabled. This prevents operations on destination buckets where versioning is disabled or suspended, hense maintaining proper version control. On the source, we should be fine. Cloudserver prevents setting up replication on a bucket if its versioning is disabled or suspended, and it prevents changing a bucket’s versioning to suspended while replication is ongoing. Issue: The Backbeat API is also used for lifecycle, which works on both versioned and non-versioned buckets. The current versioning check affects the lifecycle operations, which should be allowed on non-versioned buckets. Proposed solution: Modify the Backbeat API to apply the bucket versioning check only to actions related to the replication destination buckets (such as PUT, POST, and DELETE requests). Implement a new client header x-scal-versioning-required in the Backbeat client to make sure that replication targets only versioned buckets. When this header is set, Cloudserver will make sure that the destination bucket has versioning enabled, preventing replication to non-versioned buckets. This change will ensure that replication only targets versioned buckets and allows lifecycle operations on non-versioned buckets.
- Loading branch information
1 parent
4352e97
commit b762233
Showing
3 changed files
with
288 additions
and
31 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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,185 @@ | ||
| const assert = require('assert'); | ||
| const sinon = require('sinon'); | ||
| const metadataUtils = require('../../../lib/metadata/metadataUtils'); | ||
| const storeObject = require('../../../lib/api/apiUtils/object/storeObject'); | ||
| const metadata = require('../../../lib/metadata/wrapper'); | ||
| const { DummyRequestLogger } = require('../helpers'); | ||
| const DummyRequest = require('../DummyRequest'); | ||
|
|
||
| const log = new DummyRequestLogger(); | ||
|
|
||
| function prepareDummyRequest(headers = {}) { | ||
| const request = new DummyRequest({ | ||
| hostname: 'localhost', | ||
| method: 'PUT', | ||
| url: '/_/backbeat/metadata/bucket0/key0', | ||
| port: 80, | ||
| headers, | ||
| socket: { | ||
| remoteAddress: '0.0.0.0', | ||
| }, | ||
| }, '{"replicationInfo":"{}"}'); | ||
| return request; | ||
| } | ||
|
|
||
| describe('routeBackbeat', () => { | ||
| let mockResponse; | ||
| let mockRequest; | ||
| let sandbox; | ||
| let endPromise; | ||
| let resolveEnd; | ||
| let routeBackbeat; | ||
|
|
||
| beforeEach(() => { | ||
| sandbox = sinon.createSandbox(); | ||
|
|
||
| // create a Promise that resolves when response.end is called | ||
| endPromise = new Promise((resolve) => { resolveEnd = resolve; }); | ||
|
|
||
| mockResponse = { | ||
| statusCode: null, | ||
| body: null, | ||
| setHeader: () => {}, | ||
| writeHead: sandbox.spy(statusCode => { | ||
| mockResponse.statusCode = statusCode; | ||
| }), | ||
| end: sandbox.spy((body, encoding, callback) => { | ||
| mockResponse.body = JSON.parse(body); | ||
| if (callback) callback(); | ||
| resolveEnd(); // Resolve the Promise when end is called | ||
| }), | ||
| }; | ||
|
|
||
| mockRequest = prepareDummyRequest(); | ||
|
|
||
| sandbox.stub(metadataUtils, 'standardMetadataValidateBucketAndObj'); | ||
| sandbox.stub(storeObject, 'dataStore'); | ||
|
|
||
| // Clear require cache for routeBackbeat to make sure fresh module with stubbed dependencies | ||
| delete require.cache[require.resolve('../../../lib/routes/routeBackbeat')]; | ||
| routeBackbeat = require('../../../lib/routes/routeBackbeat'); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| sandbox.restore(); | ||
| }); | ||
|
|
||
| const rejectionTests = [ | ||
| { | ||
| description: 'should reject CRR destination (putData) requests when versioning is disabled', | ||
| method: 'PUT', | ||
| url: '/_/backbeat/data/bucket0/key0', | ||
| }, | ||
| { | ||
| description: 'should reject CRR destination (putMetadata) requests when versioning is disabled', | ||
| method: 'PUT', | ||
| url: '/_/backbeat/metadata/bucket0/key0', | ||
| }, | ||
| ]; | ||
|
|
||
| rejectionTests.forEach(({ description, method, url }) => { | ||
| it(description, async () => { | ||
| mockRequest.method = method; | ||
| mockRequest.url = url; | ||
| mockRequest.headers = { | ||
| 'x-scal-versioning-required': 'true', | ||
| }; | ||
| metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { | ||
| const bucketInfo = { | ||
| getVersioningConfiguration: () => ({ Status: 'Disabled' }), | ||
| }; | ||
| const objMd = {}; | ||
| callback(null, bucketInfo, objMd); | ||
| }); | ||
|
|
||
| routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); | ||
|
|
||
| void await endPromise; | ||
|
|
||
| assert.strictEqual(mockResponse.statusCode, 409); | ||
| assert.strictEqual(mockResponse.body.code, 'InvalidBucketState'); | ||
| }); | ||
| }); | ||
|
|
||
| it('should allow non-CRR destination (getMetadata) requests regardless of versioning', async () => { | ||
| mockRequest.method = 'GET'; | ||
|
|
||
| metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { | ||
| const bucketInfo = { | ||
| getVersioningConfiguration: () => ({ Status: 'Disabled' }), | ||
| }; | ||
| const objMd = {}; | ||
| callback(null, bucketInfo, objMd); | ||
| }); | ||
|
|
||
| routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); | ||
|
|
||
| void await endPromise; | ||
|
|
||
| assert.strictEqual(mockResponse.statusCode, 200); | ||
| assert.deepStrictEqual(mockResponse.body, { Body: '{}' }); | ||
| }); | ||
|
|
||
| it('should allow CRR destination requests (putMetadata) when versioning is enabled', async () => { | ||
| mockRequest.method = 'PUT'; | ||
| mockRequest.url = '/_/backbeat/metadata/bucket0/key0'; | ||
| mockRequest.headers = { | ||
| 'x-scal-versioning-required': 'true', | ||
| }; | ||
| mockRequest.destroy = () => {}; | ||
|
|
||
| sandbox.stub(metadata, 'putObjectMD').callsFake((bucketName, objectKey, omVal, options, logParam, cb) => { | ||
| cb(null, {}); | ||
| }); | ||
|
|
||
| metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { | ||
| const bucketInfo = { | ||
| getVersioningConfiguration: () => ({ Status: 'Enabled' }), | ||
| isVersioningEnabled: () => true, | ||
| }; | ||
| const objMd = {}; | ||
| callback(null, bucketInfo, objMd); | ||
| }); | ||
|
|
||
| routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); | ||
|
|
||
| void await endPromise; | ||
|
|
||
| assert.strictEqual(mockResponse.statusCode, 200); | ||
| assert.deepStrictEqual(mockResponse.body, {}); | ||
| }); | ||
|
|
||
| it('should allow CRR destination requests (putData) when versioning is enabled', async () => { | ||
| const md5 = '1234'; | ||
| mockRequest.method = 'PUT'; | ||
| mockRequest.url = '/_/backbeat/data/bucket0/key0'; | ||
| mockRequest.headers = { | ||
| 'x-scal-canonical-id': 'id', | ||
| 'content-md5': md5, | ||
| 'content-length': '0', | ||
| 'x-scal-versioning-required': 'true', | ||
| }; | ||
| mockRequest.destroy = () => {}; | ||
|
|
||
| metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { | ||
| const bucketInfo = { | ||
| getVersioningConfiguration: () => ({ Status: 'Enabled' }), | ||
| isVersioningEnabled: () => true, | ||
| getLocationConstraint: () => undefined, | ||
| }; | ||
| const objMd = {}; | ||
| callback(null, bucketInfo, objMd); | ||
| }); | ||
| storeObject.dataStore.callsFake((objectContext, cipherBundle, stream, size, | ||
| streamingV4Params, backendInfo, log, callback) => { | ||
| callback(null, {}, md5); | ||
| }); | ||
|
|
||
| routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); | ||
|
|
||
| void await endPromise; | ||
|
|
||
| assert.strictEqual(mockResponse.statusCode, 200); | ||
| assert.deepStrictEqual(mockResponse.body, [{}]); | ||
| }); | ||
| }); |