Merge pull request #5190 from JP-Dhabolt/feature/pre-substitute-transforms

Feature/pre substitute transforms

.changeset/fair-geese-yell.md

@@ -0,0 +1,15 @@
+---
+'@backstage/config-loader': minor
+---
+
+Fix bug where `$${...}` was not being escaped to `${...}`
+
+Add support for environment variable substitution in `$include`, `$file` and
+`$env` transform values.
+
+- This change allows for including dynamic paths, such as environment specific
+  secrets by using the same environment variable substitution (`${..}`) already
+  supported outside of the various include transforms.
+- If you are currently using the syntax `${...}` in your include transform values,
+  you will need to escape the substitution by using `$${...}` instead to maintain
+  the same behavior.

docs/conf/writing.md

@@ -177,3 +177,31 @@ configuration value will evaluate to `undefined`.
 
 The substitution syntax can be escaped using `$${...}`, which will be resolved
 as `${...}`.
+
+## Combining Includes and Environment Variable Substitution
+
+The Includes and Environment Variable Substitutions can be combined to do
+something like read a secrets configuration for a specific environment. For
+example:
+
+```yaml
+integrations:
+  github:
+    - host: github.com
+      apps:
+        - $include: secrets.${BACKSTAGE_ENVIRONMENT}.yaml
+```
+
+Example `secrets.prod.yaml`:
+
+```yaml
+appId: 1
+webhookUrl: https://smee.io/foo
+clientId: someGithubAppClientId
+clientSecret: someGithubAppClientSecret
+webhookSecret: someWebhookSecret
+privateKey: |
+  -----BEGIN RSA PRIVATE KEY-----
+  SomeRsaPrivateKey
+  -----END RSA PRIVATE KEY-----
+```

packages/config-loader/src/lib/transform/include.test.ts

@@ -17,8 +17,11 @@
 import * as os from 'os';
 import { resolve as resolvePath } from 'path';
 import { createIncludeTransform } from './include';
+import { TransformFunc } from './types';
 
 const root = os.platform() === 'win32' ? 'C:\\' : '/';
+const substituteMe = '${MY_SUBSTITUTION}';
+const mySubstitution = 'fooSubstitution';
 
 const env = jest.fn(async (name: string) => {
   return ({
@@ -26,6 +29,20 @@ const env = jest.fn(async (name: string) => {
   } as { [name: string]: string })[name];
 });
 
+const substitute: TransformFunc = async value => {
+  if (typeof value !== 'string') {
+    return { applied: false };
+  }
+  if (value.includes(substituteMe)) {
+    return {
+      applied: true,
+      value: value.replace(substituteMe, mySubstitution),
+    };
+  }
+
+  return { applied: false };
+};
+
 const readFile = jest.fn(async (path: string) => {
   const content = ({
     [resolvePath(root, 'my-secret')]: 'secret',
@@ -33,6 +50,7 @@ const readFile = jest.fn(async (path: string) => {
     [resolvePath(root, 'my-data.yaml')]: 'some:\n yaml:\n  key: 7',
     [resolvePath(root, 'my-data.yml')]: 'different: { key: hello }',
     [resolvePath(root, 'invalid.yaml')]: 'foo: [}',
+    [resolvePath(root, `${mySubstitution}/my-data.json`)]: '{"foo":"bar"}',
   } as { [key: string]: string })[path];
 
   if (!content) {
@@ -41,7 +59,7 @@ const readFile = jest.fn(async (path: string) => {
   return content;
 });
 
-const includeTransform = createIncludeTransform(env, readFile);
+const includeTransform = createIncludeTransform(env, readFile, substitute);
 
 describe('includeTransform', () => {
   it('should not transform unknown values', async () => {
@@ -136,4 +154,14 @@ describe('includeTransform', () => {
       'failed to parse included file invalid.yaml, YAMLSyntaxError: Flow sequence contains an unexpected }',
     );
   });
+
+  it('should call substitute prior to handling includes directive', async () => {
+    await expect(
+      includeTransform({ $include: `${substituteMe}/my-data.json` }, root),
+    ).resolves.toEqual({
+      applied: true,
+      value: { foo: 'bar' },
+      newBaseDir: `/${mySubstitution}`,
+    });
+  });
 });

packages/config-loader/src/lib/transform/include.ts

@@ -35,6 +35,7 @@ const includeFileParser: {
 export function createIncludeTransform(
   env: EnvFunc,
   readFile: ReadFileFunc,
+  substitute: TransformFunc,
 ): TransformFunc {
   return async (input: JsonValue, baseDir: string) => {
     if (!isObject(input)) {
@@ -53,11 +54,21 @@ export function createIncludeTransform(
       return { applied: false };
     }
 
-    const includeValue = input[includeKey];
-    if (typeof includeValue !== 'string') {
+    const rawIncludedValue = input[includeKey];
+    if (typeof rawIncludedValue !== 'string') {
       throw new Error(`${includeKey} include value is not a string`);
     }
 
+    const substituteResults = await substitute(rawIncludedValue, baseDir);
+    const includeValue = substituteResults.applied
+      ? substituteResults.value
+      : rawIncludedValue;
+
+    // The second string check is needed for Typescript to know this is a string.
+    if (includeValue === undefined || typeof includeValue !== 'string') {
+      throw new Error(`${includeKey} substitution value was undefined`);
+    }
+
     switch (includeKey) {
       case '$file':
         try {

packages/config-loader/src/lib/transform/substitution.test.ts

@@ -51,16 +51,31 @@ describe('substituteTransform', () => {
     });
     await expect(
       substituteTransform('${SECRET      } $${} ${TOKEN }', '/'),
-    ).resolves.toEqual({ applied: true, value: 'my-secret $${} my-token' });
+    ).resolves.toEqual({ applied: true, value: 'my-secret ${} my-token' });
     await expect(substituteTransform('foo ${MISSING}', '/')).resolves.toEqual({
       applied: true,
       value: undefined,
     });
+    await expect(
+      substituteTransform('empty substitute ${}', '/'),
+    ).resolves.toEqual({
+      applied: true,
+      value: undefined,
+    });
     await expect(
       substituteTransform('foo ${MISSING} ${SECRET}', '/'),
     ).resolves.toEqual({ applied: true, value: undefined });
     await expect(
       substituteTransform('foo ${SECRET} ${SECRET}', '/'),
     ).resolves.toEqual({ applied: true, value: 'foo my-secret my-secret' });
+    await expect(
+      substituteTransform('foo ${SECRET} $$${ESCAPE_ME}', '/'),
+    ).resolves.toEqual({ applied: true, value: 'foo my-secret $${ESCAPE_ME}' });
+    await expect(
+      substituteTransform('foo $${ESCAPE_ME} $$${ESCAPE_ME_TOO} $${}', '/'),
+    ).resolves.toEqual({
+      applied: true,
+      value: 'foo ${ESCAPE_ME} $${ESCAPE_ME_TOO} ${}',
+    });
   });
 });

packages/config-loader/src/lib/transform/substitution.ts

@@ -28,10 +28,16 @@ export function createSubstitutionTransform(env: EnvFunc): TransformFunc {
       return { applied: false };
     }
 
-    const parts: (string | undefined)[] = input.split(/(?<!\$)\$\{([^{}]+)\}/);
+    const parts: (string | undefined)[] = input.split(/(\$?\$\{[^{}]*\})/);
     for (let i = 1; i < parts.length; i += 2) {
-      parts[i] = await env(parts[i]!.trim());
+      const part = parts[i]!;
+      if (part.startsWith('$$')) {
+        parts[i] = part.slice(1);
+      } else {
+        parts[i] = await env(part.slice(2, -1).trim());
+      }
     }
+
     if (parts.some(part => part === undefined)) {
       return { applied: true, value: undefined };
     }

packages/config-loader/src/loader.test.ts

@@ -20,6 +20,7 @@ import mockFs from 'mock-fs';
 describe('loadConfig', () => {
   beforeAll(() => {
     process.env.MY_SECRET = 'is-secret';
+    process.env.SUBSTITUTE_ME = 'substituted';
 
     mockFs({
       '/root/app-config.yaml': `
@@ -27,6 +28,7 @@ describe('loadConfig', () => {
           title: Example App
           sessionKey:
             $file: secrets/session-key.txt
+          escaped: \$\${Escaped}
       `,
       '/root/app-config.development.yaml': `
         app:
@@ -45,6 +47,19 @@ describe('loadConfig', () => {
         foo:
           bar: token \${MY_SECRET}
       `,
+      '/root/app-config.substitute.yaml': `
+        app:
+          someConfig:
+            $include: \${SUBSTITUTE_ME}.yaml
+          noSubstitute:
+            $file: \$\${ESCAPE_ME}.txt
+      `,
+      '/root/substituted.yaml': `
+        secret:
+          $file: secrets/\${SUBSTITUTE_ME}.txt
+      `,
+      '/root/secrets/substituted.txt': '123abc',
+      '/root/${ESCAPE_ME}.txt': 'notSubstituted',
     });
   });
 
@@ -66,6 +81,7 @@ describe('loadConfig', () => {
           app: {
             title: 'Example App',
             sessionKey: 'abc123',
+            escaped: '${Escaped}',
           },
         },
       },
@@ -86,6 +102,7 @@ describe('loadConfig', () => {
           app: {
             title: 'Example App',
             sessionKey: 'abc123',
+            escaped: '${Escaped}',
           },
         },
       },
@@ -109,6 +126,7 @@ describe('loadConfig', () => {
           app: {
             title: 'Example App',
             sessionKey: 'abc123',
+            escaped: '${Escaped}',
           },
         },
       },
@@ -130,4 +148,26 @@ describe('loadConfig', () => {
       },
     ]);
   });
+
+  it('loads deep substituted config', async () => {
+    await expect(
+      loadConfig({
+        configRoot: '/root',
+        configPaths: ['/root/app-config.substitute.yaml'],
+        env: 'development',
+      }),
+    ).resolves.toEqual([
+      {
+        context: 'app-config.substitute.yaml',
+        data: {
+          app: {
+            someConfig: {
+              secret: '123abc',
+            },
+            noSubstitute: 'notSubstituted',
+          },
+        },
+      },
+    ]);
+  });
 });

packages/config-loader/src/loader.ts

@@ -75,9 +75,10 @@ export async function loadConfig(
         fs.readFile(resolvePath(dir, path), 'utf8');
 
       const input = yaml.parse(await readFile(configPath));
+      const substitutionTransform = createSubstitutionTransform(env);
       const data = await applyConfigTransforms(dir, input, [
-        createIncludeTransform(env, readFile),
-        createSubstitutionTransform(env),
+        createIncludeTransform(env, readFile, substitutionTransform),
+        substitutionTransform,
       ]);
 
       configs.push({ data, context: basename(configPath) });