From 2a0e3260c5c4d9718f687c69e1d9f6596a90552f Mon Sep 17 00:00:00 2001
From: Kerkesni <walid.elansari@scality.com>
Date: Mon, 4 Nov 2024 15:17:36 +0100
Subject: [PATCH] Fix the dataMoverTopic config param always being required

Extension configs are validated independently of each other, so
validating the dataMoverTopic based on the supportedLifecycleRules
param is not currently possible.

Issue: BB-619
---
 .../replication/ReplicationConfigValidator.js |  6 +---
 lib/Config.js                                 |  7 ++++
 tests/unit/lib/config/Config.spec.js          | 35 ++++++++++++++++---
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/extensions/replication/ReplicationConfigValidator.js b/extensions/replication/ReplicationConfigValidator.js
index 3bd9946f3..9a4d905d9 100644
--- a/extensions/replication/ReplicationConfigValidator.js
+++ b/extensions/replication/ReplicationConfigValidator.js
@@ -53,11 +53,7 @@ const joiSchema = joi.object({
         bootstrapList: bootstrapListJoi,
     },
     topic: joi.string().required(),
-    dataMoverTopic: joi.string().when('..lifecycle.supportedLifecycleRules', {
-        is: joi.array().has(joi.string().valid('transition')),
-        then: joi.required(),
-        otherwise: joi.forbidden(),
-    }),
+    dataMoverTopic: joi.string().optional(),
     replicationStatusTopic: joi.string().required(),
     monitorReplicationFailures: joi.boolean().default(true),
     replicationFailedTopic: joi.string().required(),
diff --git a/lib/Config.js b/lib/Config.js
index 290560576..40c817841 100644
--- a/lib/Config.js
+++ b/lib/Config.js
@@ -82,6 +82,13 @@ class Config extends EventEmitter {
             });
         }
 
+        const lifecycleConfig = parsedConfig.extensions?.lifecycle;
+        const backbeatSupportsTransition = lifecycleConfig?.supportedLifecycleRules?.includes('transitions');
+        const replicationConfig = parsedConfig.extensions?.replication;
+        if (backbeatSupportsTransition && !replicationConfig.dataMoverTopic) {
+            throw new Error('dataMoverTopic is required when lifecycle transitions is supported');
+        }
+
         if (parsedConfig.extensions && parsedConfig.extensions.replication
             && parsedConfig.extensions.replication.destination
             && parsedConfig.extensions.replication.destination.bootstrapList) {
diff --git a/tests/unit/lib/config/Config.spec.js b/tests/unit/lib/config/Config.spec.js
index f3160d20a..0cb36fa21 100644
--- a/tests/unit/lib/config/Config.spec.js
+++ b/tests/unit/lib/config/Config.spec.js
@@ -6,20 +6,47 @@ const { Config } = require('../../../../lib/Config');
 const backbeatConfig = require('./config.json');
 
 describe('Config', () => {
+    let config;
+    let testConfig;
+
+    beforeEach(() => {
+        config = new Config();
+        // deep copy the config to avoid modifying the original
+        testConfig = JSON.parse(JSON.stringify(backbeatConfig));
+    });
+
     it('should make the probeserver config in the queuePoulator' +
         'required when multiple extensions are configured', () => {
-        const config = new Config();
-        const testConfig = { ...backbeatConfig };
         delete testConfig.queuePopulator.probeServer;
         assert.throws(() => config._parseConfig(testConfig));
     });
 
     it('should make the probeserver config in the queuePoulator' +
         'optional when only notification config is specified', () => {
-        const config = new Config();
-        const testConfig = { ...backbeatConfig };
         delete testConfig.queuePopulator.probeServer;
         testConfig.extensions = { notification: testConfig.extensions.notification };
         assert.doesNotThrow(() => config._parseConfig(testConfig));
     });
+
+    it('should throw an error when dataMoverTopic is not provided and transition is supported', () => {
+        delete testConfig.extensions.replication.dataMoverTopic;
+        testConfig.extensions.lifecycle.supportedLifecycleRules = [
+            'transitions',
+            'noncurrentVersionTransition',
+            'expiration',
+            'noncurrentVersionExpiration',
+            'abortIncompleteMultipartUpload',
+        ];
+        assert.throws(() => config._parseConfig(testConfig));
+    });
+
+    it('should make dataMoverTopic optional when transitions are not supported', () => {
+        delete testConfig.extensions.replication.dataMoverTopic;
+        testConfig.extensions.lifecycle.supportedLifecycleRules = [
+            'expiration',
+            'noncurrentVersionExpiration',
+            'abortIncompleteMultipartUpload',
+        ];
+        assert.doesNotThrow(() => config._parseConfig(testConfig));
+    });
 });