diff --git a/.changeset/fair-geese-yell.md b/.changeset/fair-geese-yell.md new file mode 100644 index 0000000000..00c5b16854 --- /dev/null +++ b/.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. diff --git a/docs/conf/writing.md b/docs/conf/writing.md index 057675c98e..ac8509db0a 100644 --- a/docs/conf/writing.md +++ b/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----- +``` diff --git a/packages/config-loader/src/lib/transform/include.test.ts b/packages/config-loader/src/lib/transform/include.test.ts index 1724159e56..200bbe3815 100644 --- a/packages/config-loader/src/lib/transform/include.test.ts +++ b/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}`, + }); + }); }); diff --git a/packages/config-loader/src/lib/transform/include.ts b/packages/config-loader/src/lib/transform/include.ts index 1a6963672a..2b5ccf23b6 100644 --- a/packages/config-loader/src/lib/transform/include.ts +++ b/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 { diff --git a/packages/config-loader/src/lib/transform/substitution.test.ts b/packages/config-loader/src/lib/transform/substitution.test.ts index e2c0200bfd..d07e88cfdd 100644 --- a/packages/config-loader/src/lib/transform/substitution.test.ts +++ b/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} ${}', + }); }); }); diff --git a/packages/config-loader/src/lib/transform/substitution.ts b/packages/config-loader/src/lib/transform/substitution.ts index 821cc8bc67..56695fa431 100644 --- a/packages/config-loader/src/lib/transform/substitution.ts +++ b/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(/(? part === undefined)) { return { applied: true, value: undefined }; } diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index 8c7de557e3..27b1e20789 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/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', + }, + }, + }, + ]); + }); }); diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 54e4ab5208..95e5b570ca 100644 --- a/packages/config-loader/src/loader.ts +++ b/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) });