Skip to content

Commit

Permalink
[config] stop merging arrays from multiple configs (#161884)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
pgayvallet authored Jul 14, 2023
1 parent 37b48d3 commit a9f6e03
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 2 deletions.
10 changes: 10 additions & 0 deletions packages/kbn-config/src/__fixtures__/merge_1.yml
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions packages/kbn-config/src/__fixtures__/merge_2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
foo: 2
arr1: [4, 5]
arr2: []
nested:
str1: "bar"
str3: "dolly"
obj_array:
- id: 3
22 changes: 22 additions & 0 deletions packages/kbn-config/src/raw/__snapshots__/read_config.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions packages/kbn-config/src/raw/read_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 14 additions & 2 deletions packages/kbn-config/src/raw/read_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down

0 comments on commit a9f6e03

Please sign in to comment.