Do not mutate config in place during deprecations

packages/kbn-config/src/deprecation/apply_deprecations.test.ts

@@ -36,19 +36,16 @@ describe('applyDeprecations', () => {
     const addDeprecation = jest.fn();
     const createAddDeprecation = jest.fn().mockReturnValue(addDeprecation);
     const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
-    const alteredConfig = { foo: 'bar' };
 
-    const handlerA = jest.fn().mockReturnValue(alteredConfig);
-    const handlerB = jest.fn().mockImplementation((conf) => conf);
+    const handlerA = jest.fn().mockReturnValue({ unset: [{ path: 'deprecated' }] });
+    const handlerB = jest.fn().mockReturnValue(undefined);
 
     applyDeprecations(
       initialConfig,
       [wrapHandler(handlerA, 'pathA'), wrapHandler(handlerB, 'pathB')],
       createAddDeprecation
     );
 
-    expect(handlerA).toHaveBeenCalledWith(initialConfig, 'pathA', addDeprecation);
-    expect(handlerB).toHaveBeenCalledWith(alteredConfig, 'pathB', addDeprecation);
     expect(createAddDeprecation).toBeCalledTimes(2);
     expect(createAddDeprecation).toHaveBeenNthCalledWith(1, 'pathA');
     expect(createAddDeprecation).toHaveBeenNthCalledWith(2, 'pathB');
@@ -60,17 +57,26 @@ describe('applyDeprecations', () => {
     const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
     const alteredConfig = { foo: 'bar' };
 
-    const handlerA = jest.fn().mockReturnValue(alteredConfig);
-    const handlerB = jest.fn().mockImplementation((conf) => conf);
+    const configs: Array<{ fn: string; config: Record<string, any> }> = [];
+    const handlerA = jest.fn().mockImplementation((config) => {
+      // the first argument is mutated between calls, we store a copy of it
+      configs.push({ fn: 'handlerA', config: { ...config } });
+      return { unset: [{ path: 'deprecated' }] };
+    });
+    const handlerB = jest.fn().mockImplementation((config) => {
+      configs.push({ fn: 'handlerB', config: { ...config } });
+    });
 
     applyDeprecations(
       initialConfig,
       [wrapHandler(handlerA, 'pathA'), wrapHandler(handlerB, 'pathB')],
       createAddDeprecation
     );
 
-    expect(handlerA).toHaveBeenCalledWith(initialConfig, 'pathA', addDeprecation);
-    expect(handlerB).toHaveBeenCalledWith(alteredConfig, 'pathB', addDeprecation);
+    expect(configs).toEqual([
+      { fn: 'handlerA', config: initialConfig },
+      { fn: 'handlerB', config: alteredConfig },
+    ]);
   });
 
   it('returns the migrated config', () => {
@@ -94,4 +100,40 @@ describe('applyDeprecations', () => {
     expect(initialConfig).toEqual({ foo: 'bar', deprecated: 'deprecated' });
     expect(migrated).toEqual({ foo: 'bar' });
   });
+
+  it('ignores a command for unknown path', () => {
+    const addDeprecation = jest.fn();
+    const createAddDeprecation = jest.fn().mockReturnValue(addDeprecation);
+    const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
+
+    const handler = jest.fn().mockImplementation((config) => {
+      return { unset: [{ path: 'unknown' }] };
+    });
+
+    const migrated = applyDeprecations(
+      initialConfig,
+      [wrapHandler(handler, 'pathA')],
+      createAddDeprecation
+    );
+
+    expect(migrated).toEqual(initialConfig);
+  });
+
+  it('ignores an unknown command', () => {
+    const addDeprecation = jest.fn();
+    const createAddDeprecation = jest.fn().mockReturnValue(addDeprecation);
+    const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
+
+    const handler = jest.fn().mockImplementation((config) => {
+      return { rewrite: [{ path: 'foo' }] };
+    });
+
+    const migrated = applyDeprecations(
+      initialConfig,
+      [wrapHandler(handler, 'pathA')],
+      createAddDeprecation
+    );
+
+    expect(migrated).toEqual(initialConfig);
+  });
 });

packages/kbn-config/src/deprecation/apply_deprecations.ts

@@ -6,7 +6,8 @@
  * Side Public License, v 1.
  */
 
-import { cloneDeep } from 'lodash';
+import { cloneDeep, unset } from 'lodash';
+import { set } from '@elastic/safer-lodash-set';
 import { ConfigDeprecationWithContext, AddConfigDeprecation } from './types';
 
 const noopAddDeprecationFactory: () => AddConfigDeprecation = () => () => undefined;
@@ -22,9 +23,21 @@ export const applyDeprecations = (
   deprecations: ConfigDeprecationWithContext[],
   createAddDeprecation: (pluginId: string) => AddConfigDeprecation = noopAddDeprecationFactory
 ) => {
-  let processed = cloneDeep(config);
+  const result = cloneDeep(config);
   deprecations.forEach(({ deprecation, path }) => {
-    processed = deprecation(processed, path, createAddDeprecation(path));
+    const commands = deprecation(result, path, createAddDeprecation(path));
+    if (commands) {
+      if (commands.set) {
+        commands.set.forEach(function ({ path: commandPath, value }) {
+          set(result, commandPath, value);
+        });
+      }
+      if (commands.unset) {
+        commands.unset.forEach(function ({ path: commandPath }) {
+          unset(result, commandPath);
+        });
+      }
+    }
   });
-  return processed;
+  return result;
 };