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).

Allow GET actions like getMetadata and headLocation, which are used in workflows that don’t affect replication destination buckets.

This change will ensure that replication only targets versioned buckets and allows lifecycle operations on non-versioned buckets.

lib/routes/routeBackbeat.js

@@ -1289,7 +1289,10 @@ function routeBackbeat(clientIP, request, response, log) {
                     [request.query.operation](request, response, log, next);
             }
             const versioningConfig = bucketInfo.getVersioningConfiguration();
-            if (!versioningConfig || versioningConfig.Status !== 'Enabled') {
+            // The following makes sure that only replication destination-related operations (non-GET requests)
+            // target buckets with versioning enabled.
+            // This allows lifecycle operations (which use GET requests) to work on non-versioned buckets.
+            if (request.method !== 'GET' && (!versioningConfig || versioningConfig.Status !== 'Enabled')) {
                 log.debug('bucket versioning is not enabled', {
                     method: request.method,
                     bucketName: request.bucketName,

tests/unit/routes/routeBackbeat.js

@@ -0,0 +1,142 @@
+const assert = require('assert');
+const sinon = require('sinon');
+const metadataUtils = require('../../../lib/metadata/metadataUtils');
+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');
+
+        // 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();
+    });
+
+    it('should reject non-GET requests when versioning is disabled', async () => {
+        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 GET 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 non-GET requests when versioning is enabled', async () => {
+        mockRequest.method = 'PUT';
+        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 reject non-GET requests when versioning is disabled', async () => {
+        metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => {
+            const bucketInfo = {
+                getVersioningConfiguration: () => null,
+            };
+            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');
+    });
+});