Merge pull request #8819 from backstage/rugvip/clonfig

config: make ConfigReader#get always return a clone

.changeset/clean-wolves-jog.md

@@ -0,0 +1,5 @@
+---
+'@backstage/config': patch
+---
+
+The `ConfigReader#get` method now always returns a deep clone of the configuration data.

packages/config/src/reader.test.ts

@@ -265,6 +265,19 @@ describe('ConfigReader', () => {
       withLogCollector(() => config.getOptionalConfigArray('b')),
     ).toMatchObject({ warn: [] });
   });
+
+  it('should coerce number strings to numbers', () => {
+    const config = ConfigReader.fromConfigs([
+      {
+        data: {
+          port: '123',
+        },
+        context: '1',
+      },
+    ]);
+
+    expect(config.getNumber('port')).toEqual(123);
+  });
 });
 
 describe('ConfigReader with fallback', () => {
@@ -661,16 +674,50 @@ describe('ConfigReader.get()', () => {
     });
   });
 
-  it('coerces number strings to numbers', () => {
-    const config = ConfigReader.fromConfigs([
-      {
-        data: {
-          port: '123',
-        },
-        context: '1',
+  it('should return deep clones of the backing data', () => {
+    const data1 = {
+      foo: {
+        bar: [],
+        baz: {},
       },
+    };
+    const data2 = {
+      x: {
+        y: {
+          z: {},
+        },
+      },
+    };
+
+    const reader = ConfigReader.fromConfigs([
+      { data: data1, context: '1' },
+      { data: data2, context: '2' },
     ]);
 
-    expect(config.getNumber('port')).toEqual(123);
+    reader.get<any>().foo.bar.push(1);
+    reader.get<any>('foo').bar.push(1);
+    reader.get<any>('foo.bar').push(1);
+    reader.get<any>().foo.baz.x = 1;
+    reader.get<any>('foo').baz.x = 1;
+    reader.get<any>('foo.baz').x = 1;
+    reader.get<any>().x.y.z.w = 1;
+    reader.get<any>('x').y.z.w = 1;
+    reader.get<any>('x.y').z.w = 1;
+    reader.get<any>('x.y.z').w = 1;
+
+    const readerSingle = ConfigReader.fromConfigs([
+      { data: data1, context: '1' },
+    ]);
+
+    readerSingle.get<any>().foo.bar.push(1);
+    readerSingle.get<any>('foo').bar.push(1);
+    readerSingle.get<any>('foo.bar').push(1);
+    readerSingle.get<any>().foo.baz.x = 1;
+    readerSingle.get<any>('foo').baz.x = 1;
+    readerSingle.get<any>('foo.baz').x = 1;
+
+    expect(data1.foo.bar).toEqual([]);
+    expect(data1.foo.baz).toEqual({});
+    expect(data2.x.y.z).toEqual({});
   });
 });

packages/config/src/reader.ts

@@ -126,7 +126,7 @@ export class ConfigReader implements Config {
 
   /** {@inheritdoc Config.getOptional} */
   getOptional<T = JsonValue>(key?: string): T | undefined {
-    const value = this.readValue(key);
+    const value = cloneDeep(this.readValue(key));
     const fallbackValue = this.fallback?.getOptional<T>(key);
 
     if (value === undefined) {
@@ -153,11 +153,8 @@ export class ConfigReader implements Config {
 
     // Avoid merging arrays and primitive values, since that's how merging works for other
     // methods for reading config.
-    return mergeWith(
-      {},
-      { value: cloneDeep(fallbackValue) },
-      { value },
-      (into, from) => (!isObject(from) || !isObject(into) ? from : undefined),
+    return mergeWith({}, { value: fallbackValue }, { value }, (into, from) =>
+      !isObject(from) || !isObject(into) ? from : undefined,
     ).value as T;
   }