CLDSRV-977: add storeCompositeChecksums feature flag

config.json

@@ -153,7 +153,8 @@
         "bucketPutPolicy": 20480
     },
     "integrityChecks": {
-        "enabled": true
+        "enabled": true,
+        "storeCompositeChecksums": true
     },
     "serverAccessLogs": {
         "mode": "DISABLED",

lib/Config.js

@@ -605,36 +605,35 @@ function parseServerAccessLogs(config) {
  * so CompleteMultipartUpload cannot compose a final checksum for that object:
  * it completes the upload without one and logs an error naming the parts.
  *
+ * `storeCompositeChecksums` if false COMPOSITE checksums are not stored in the final object metadata
+ * and also no per part checksum is stored.
+ *
  * @param {object} config - raw parsed config file contents
- * @return {{enabled: boolean}} the parsed integrityChecks section
+ * @return {{enabled: boolean, storeCompositeChecksums: boolean}} the parsed integrityChecks section
  */
 function parseIntegrityChecks(config) {
-    const res = { enabled: true };
-
-    if (config && config.integrityChecks) {
-        assert(
-            typeof config.integrityChecks === 'object' && !Array.isArray(config.integrityChecks),
-            'bad config: integrityChecks must be an object',
-        );
+    const joiSchema = joi.object({
+        enabled: joi.boolean().strict().default(true),
+        storeCompositeChecksums: joi.boolean().strict().default(true),
+    });
+    const section = (config && config.integrityChecks) || {};
 
-        if ('enabled' in config.integrityChecks) {
-            assert(
-                typeof config.integrityChecks.enabled === 'boolean',
-                'bad config: integrityChecks.enabled must be a boolean',
-            );
-            res.enabled = config.integrityChecks.enabled;
-        }
-    }
+    // stripUnknown: legacy per-API keys (CLDSRV-943) may still be in deployed
+    // configs, and joi rejects unknown keys by default. Ignore, do not reject.
+    const res = joi.attempt(section, joiSchema, 'bad config', { stripUnknown: true });
 
-    if (process.env.S3_INTEGRITY_CHECKS_ENABLED !== undefined) {
-        assert(
-            ['true', 'false'].includes(process.env.S3_INTEGRITY_CHECKS_ENABLED),
-            "bad config: S3_INTEGRITY_CHECKS_ENABLED must be 'true' or 'false'",
-        );
-        res.enabled = process.env.S3_INTEGRITY_CHECKS_ENABLED === 'true';
-    }
+    const envSchema = joi
+        .object({
+            S3_INTEGRITY_CHECKS_ENABLED: joi.boolean().default(res.enabled),
+            S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS: joi.boolean().default(res.storeCompositeChecksums),
+        })
+        .unknown(true);
+    const env = joi.attempt(process.env, envSchema, 'bad config');
 
-    return res;
+    return {
+        enabled: env.S3_INTEGRITY_CHECKS_ENABLED,
+        storeCompositeChecksums: env.S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS,
+    };
 }
 
 /**

tests/unit/Config.js

@@ -21,7 +21,7 @@ describe('Config', () => {
     const defaultConfig = JSON.parse(fs.readFileSync('config.json'), { encoding: 'utf-8' });
 
     const envToRestore = [];
-    const setEnv = (key, value) => {
+    const recordEnv = key => {
         if (key in process.env) {
             const v = process.env[key];
             envToRestore.push(() => {
@@ -32,8 +32,15 @@ describe('Config', () => {
                 delete process.env[key];
             });
         }
+    };
+    const setEnv = (key, value) => {
+        recordEnv(key);
         process.env[key] = value;
     };
+    const deleteEnv = key => {
+        recordEnv(key);
+        delete process.env[key];
+    };
 
     beforeEach(() => {
         envToRestore.length = 0;
@@ -931,45 +938,133 @@ describe('Config', () => {
     });
 
     describe('parse integrity checks', () => {
-        afterEach(() => {
-            delete process.env.S3_INTEGRITY_CHECKS_ENABLED;
+        // CI exports S3_INTEGRITY_CHECKS_ENABLED for some jobs, so clear both
+        // vars rather than assert around whatever the environment inherited.
+        // The suite-level afterEach puts them back.
+        beforeEach(() => {
+            deleteEnv('S3_INTEGRITY_CHECKS_ENABLED');
+            deleteEnv('S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS');
         });
 
         it('should default to enabled when not configured', () => {
-            assert.deepStrictEqual(parseIntegrityChecks(null), { enabled: true });
-            assert.deepStrictEqual(parseIntegrityChecks({}), { enabled: true });
+            assert.deepStrictEqual(parseIntegrityChecks(null), { enabled: true, storeCompositeChecksums: true });
+            assert.deepStrictEqual(parseIntegrityChecks({}), { enabled: true, storeCompositeChecksums: true });
         });
 
         it('should read the configured value', () => {
-            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: false } }), { enabled: false });
-            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: true } }), { enabled: true });
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: false } }), {
+                enabled: false,
+                storeCompositeChecksums: true,
+            });
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: true } }), {
+                enabled: true,
+                storeCompositeChecksums: true,
+            });
         });
 
         it('should throw if integrityChecks is not an object', () => {
-            assert.throws(() => parseIntegrityChecks({ integrityChecks: 'yes' }), /must be an object/);
-            assert.throws(() => parseIntegrityChecks({ integrityChecks: [true] }), /must be an object/);
+            assert.throws(() => parseIntegrityChecks({ integrityChecks: 'yes' }), /must be of type object/);
+            assert.throws(() => parseIntegrityChecks({ integrityChecks: [true] }), /must be of type object/);
         });
 
         it('should throw if enabled is not a boolean', () => {
             assert.throws(() => parseIntegrityChecks({ integrityChecks: { enabled: 'false' } }), /must be a boolean/);
             assert.throws(() => parseIntegrityChecks({ integrityChecks: { enabled: 0 } }), /must be a boolean/);
         });
 
+        it('should ignore unknown keys left over from older config files', () => {
+            assert.deepStrictEqual(
+                parseIntegrityChecks({ integrityChecks: { objectPutRetention: true, bucketPutACL: false } }),
+                { enabled: true, storeCompositeChecksums: true },
+            );
+            assert.deepStrictEqual(
+                parseIntegrityChecks({ integrityChecks: { enabled: false, objectPutRetention: true } }),
+                { enabled: false, storeCompositeChecksums: true },
+            );
+        });
+
         it('should let S3_INTEGRITY_CHECKS_ENABLED override the config file', () => {
-            process.env.S3_INTEGRITY_CHECKS_ENABLED = 'false';
-            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: true } }), { enabled: false });
-            process.env.S3_INTEGRITY_CHECKS_ENABLED = 'true';
-            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: false } }), { enabled: true });
+            setEnv('S3_INTEGRITY_CHECKS_ENABLED', 'false');
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: true } }), {
+                enabled: false,
+                storeCompositeChecksums: true,
+            });
+            setEnv('S3_INTEGRITY_CHECKS_ENABLED', 'true');
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { enabled: false } }), {
+                enabled: true,
+                storeCompositeChecksums: true,
+            });
         });
 
         it('should throw on a non-boolean S3_INTEGRITY_CHECKS_ENABLED', () => {
-            process.env.S3_INTEGRITY_CHECKS_ENABLED = 'nope';
-            assert.throws(() => parseIntegrityChecks(null), /S3_INTEGRITY_CHECKS_ENABLED/);
+            setEnv('S3_INTEGRITY_CHECKS_ENABLED', 'nope');
+            assert.throws(() => parseIntegrityChecks(null), /"S3_INTEGRITY_CHECKS_ENABLED" must be a boolean/);
+            setEnv('S3_INTEGRITY_CHECKS_ENABLED', '1');
+            assert.throws(() => parseIntegrityChecks(null), /"S3_INTEGRITY_CHECKS_ENABLED" must be a boolean/);
+        });
+
+        it('should default storeCompositeChecksums to true when not configured', () => {
+            assert.strictEqual(parseIntegrityChecks(null).storeCompositeChecksums, true);
+            assert.strictEqual(parseIntegrityChecks({ integrityChecks: {} }).storeCompositeChecksums, true);
+        });
+
+        it('should read the configured storeCompositeChecksums value', () => {
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { storeCompositeChecksums: false } }), {
+                enabled: true,
+                storeCompositeChecksums: false,
+            });
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { storeCompositeChecksums: true } }), {
+                enabled: true,
+                storeCompositeChecksums: true,
+            });
+        });
+
+        it('should parse enabled and storeCompositeChecksums independently', () => {
+            assert.deepStrictEqual(
+                parseIntegrityChecks({ integrityChecks: { enabled: false, storeCompositeChecksums: true } }),
+                { enabled: false, storeCompositeChecksums: true },
+            );
+            assert.deepStrictEqual(
+                parseIntegrityChecks({ integrityChecks: { enabled: true, storeCompositeChecksums: false } }),
+                { enabled: true, storeCompositeChecksums: false },
+            );
+        });
+
+        it('should throw if storeCompositeChecksums is not a boolean', () => {
+            assert.throws(
+                () => parseIntegrityChecks({ integrityChecks: { storeCompositeChecksums: 'false' } }),
+                /"storeCompositeChecksums" must be a boolean/,
+            );
+            assert.throws(
+                () => parseIntegrityChecks({ integrityChecks: { storeCompositeChecksums: 0 } }),
+                /"storeCompositeChecksums" must be a boolean/,
+            );
+        });
+
+        it('should let S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS override the config file', () => {
+            setEnv('S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS', 'false');
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { storeCompositeChecksums: true } }), {
+                enabled: true,
+                storeCompositeChecksums: false,
+            });
+            setEnv('S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS', 'true');
+            assert.deepStrictEqual(parseIntegrityChecks({ integrityChecks: { storeCompositeChecksums: false } }), {
+                enabled: true,
+                storeCompositeChecksums: true,
+            });
+        });
+
+        it('should throw on a non-boolean S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS', () => {
+            setEnv('S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS', 'nope');
+            assert.throws(
+                () => parseIntegrityChecks(null),
+                /"S3_INTEGRITY_CHECKS_STORE_COMPOSITE_CHECKSUMS" must be a boolean/,
+            );
         });
 
         it('should expose integrityChecks on the config object', () => {
             const config = new ConfigObject();
-            assert.deepStrictEqual(config.integrityChecks, { enabled: true });
+            assert.deepStrictEqual(config.integrityChecks, { enabled: true, storeCompositeChecksums: true });
         });
     });