From a9f6e03da76d9d6e8c71fe2e45fa68505cd4410e Mon Sep 17 00:00:00 2001
From: Pierre Gayvallet <pierre.gayvallet@elastic.co>
Date: Fri, 14 Jul 2023 12:57:58 +0200
Subject: [PATCH] [config] stop merging arrays from multiple configs (#161884)

## Summary

Fix an unwanted behavior of the config merging logic, that was merging
arrays the same way objects are merged.

E.g the output of `getConfigFromFiles(file1, file2)` with

```yaml
### file 1
array: [1, 2, 3]
obj_array:
  - id: 1
  - id: 2

### file 2
array: [4]
obj_array:
  - id: 3
```

was

```yaml
array: [4, 2, 3]
obj_array:
  - id: 3
  - id: 2
```

instead of

```yaml
array: [4]
obj_array:
  - id: 3
```

Which is causing problems when merging the default, serverless,
serverless project and dev file in some scenarios.
---
 .../kbn-config/src/__fixtures__/merge_1.yml   | 10 +++++++++
 .../kbn-config/src/__fixtures__/merge_2.yml   |  8 +++++++
 .../__snapshots__/read_config.test.ts.snap    | 22 +++++++++++++++++++
 .../kbn-config/src/raw/read_config.test.ts    |  5 +++++
 packages/kbn-config/src/raw/read_config.ts    | 16 ++++++++++++--
 5 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 packages/kbn-config/src/__fixtures__/merge_1.yml
 create mode 100644 packages/kbn-config/src/__fixtures__/merge_2.yml

diff --git a/packages/kbn-config/src/__fixtures__/merge_1.yml b/packages/kbn-config/src/__fixtures__/merge_1.yml
new file mode 100644
index 0000000000000..0d540bd61f998
--- /dev/null
+++ b/packages/kbn-config/src/__fixtures__/merge_1.yml
@@ -0,0 +1,10 @@
+foo: 1
+bar: 3
+arr1: [1, 2, 3]
+arr2: [42, 90]
+nested:
+  str1: "foo"
+  str2: "hello"
+obj_array:
+  - id: 1
+  - id: 2
diff --git a/packages/kbn-config/src/__fixtures__/merge_2.yml b/packages/kbn-config/src/__fixtures__/merge_2.yml
new file mode 100644
index 0000000000000..acf49c3856c64
--- /dev/null
+++ b/packages/kbn-config/src/__fixtures__/merge_2.yml
@@ -0,0 +1,8 @@
+foo: 2
+arr1: [4, 5]
+arr2: []
+nested:
+  str1: "bar"
+  str3: "dolly"
+obj_array:
+  - id: 3
diff --git a/packages/kbn-config/src/raw/__snapshots__/read_config.test.ts.snap b/packages/kbn-config/src/raw/__snapshots__/read_config.test.ts.snap
index afdce4e76d3f5..4ad36abc1b481 100644
--- a/packages/kbn-config/src/raw/__snapshots__/read_config.test.ts.snap
+++ b/packages/kbn-config/src/raw/__snapshots__/read_config.test.ts.snap
@@ -22,6 +22,28 @@ Object {
 }
 `;
 
+exports[`merging two configs 1`] = `
+Object {
+  "arr1": Array [
+    4,
+    5,
+  ],
+  "arr2": Array [],
+  "bar": 3,
+  "foo": 2,
+  "nested": Object {
+    "str1": "bar",
+    "str2": "hello",
+    "str3": "dolly",
+  },
+  "obj_array": Array [
+    Object {
+      "id": 3,
+    },
+  ],
+}
+`;
+
 exports[`reads and merges multiple yaml files from file system and parses to json 1`] = `
 Object {
   "abc": Object {
diff --git a/packages/kbn-config/src/raw/read_config.test.ts b/packages/kbn-config/src/raw/read_config.test.ts
index 5562772885690..0fdacd99d46cf 100644
--- a/packages/kbn-config/src/raw/read_config.test.ts
+++ b/packages/kbn-config/src/raw/read_config.test.ts
@@ -59,6 +59,11 @@ test('throws parsing another config with forbidden paths', () => {
   ).toThrowErrorMatchingInlineSnapshot(`"Forbidden path detected: test.hello.__proto__"`);
 });
 
+test('merging two configs', () => {
+  const config = getConfigFromFiles([fixtureFile('merge_1.yml'), fixtureFile('merge_2.yml')]);
+  expect(config).toMatchSnapshot();
+});
+
 describe('different cwd()', () => {
   const originalCwd = process.cwd();
   const tempCwd = resolve(__dirname);
diff --git a/packages/kbn-config/src/raw/read_config.ts b/packages/kbn-config/src/raw/read_config.ts
index 1cf38b52fa3e6..eb8f8df4c8815 100644
--- a/packages/kbn-config/src/raw/read_config.ts
+++ b/packages/kbn-config/src/raw/read_config.ts
@@ -27,17 +27,29 @@ function replaceEnvVarRefs(val: string) {
 }
 
 function merge(target: Record<string, any>, value: any, key?: string) {
-  if ((isPlainObject(value) || Array.isArray(value)) && Object.keys(value).length > 0) {
+  if (isPlainObject(value) && Object.keys(value).length > 0) {
     for (const [subKey, subVal] of Object.entries(value)) {
       merge(target, subVal, key ? `${key}.${subKey}` : subKey);
     }
   } else if (key !== undefined) {
-    set(target, key, typeof value === 'string' ? replaceEnvVarRefs(value) : value);
+    set(target, key, recursiveReplaceEnvVar(value));
   }
 
   return target;
 }
 
+function recursiveReplaceEnvVar(value: any) {
+  if (isPlainObject(value) || Array.isArray(value)) {
+    for (const [subKey, subVal] of Object.entries(value)) {
+      set(value, subKey, recursiveReplaceEnvVar(subVal));
+    }
+  }
+  if (typeof value === 'string') {
+    return replaceEnvVarRefs(value);
+  }
+  return value;
+}
+
 /** @internal */
 export const getConfigFromFiles = (configFiles: readonly string[]) => {
   let mergedYaml = {};