From a6faeeab4b235ec2a09bdb5fe9284d2bcb3f008c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 18:53:29 +0100 Subject: [PATCH] config-loader: refactor transforms to keep track of base dir and fix file include resolution --- .../src/lib/transform/apply.test.ts | 12 +- .../config-loader/src/lib/transform/apply.ts | 20 ++-- .../src/lib/transform/include.test.ts | 111 +++++++++--------- .../src/lib/transform/include.ts | 23 ++-- .../src/lib/transform/substitution.test.ts | 60 +++++----- .../src/lib/transform/substitution.ts | 6 +- .../config-loader/src/lib/transform/types.ts | 12 +- packages/config-loader/src/loader.test.ts | 9 ++ packages/config-loader/src/loader.ts | 2 +- 9 files changed, 141 insertions(+), 114 deletions(-) diff --git a/packages/config-loader/src/lib/transform/apply.test.ts b/packages/config-loader/src/lib/transform/apply.test.ts index da790978ea..4cdd0e97f5 100644 --- a/packages/config-loader/src/lib/transform/apply.test.ts +++ b/packages/config-loader/src/lib/transform/apply.test.ts @@ -19,6 +19,7 @@ import { applyConfigTransforms } from './apply'; describe('applyConfigTransforms', () => { it('should apply not transforms to input', async () => { const data = applyConfigTransforms( + '', { app: { title: 'Test', @@ -40,13 +41,14 @@ describe('applyConfigTransforms', () => { }); it('should throw if input is not an object', async () => { - const config = applyConfigTransforms('not-config', []); + const config = applyConfigTransforms('', 'not-config', []); await expect(config).rejects.toThrow('expected object at config root'); }); it('should apply transforms', async () => { const config = applyConfigTransforms( + '', { app: { title: 'Test', @@ -58,15 +60,15 @@ describe('applyConfigTransforms', () => { [ async value => { if (typeof value === 'number') { - return [true, value + 1]; + return { applied: true, value: value + 1 }; } - return [false, value]; + return { applied: false }; }, async value => { if (typeof value === 'string' && value.length > 1) { - return [true, value.split('')]; + return { applied: true, value: value.split('') }; } - return [false, value]; + return { applied: false }; }, ], ); diff --git a/packages/config-loader/src/lib/transform/apply.ts b/packages/config-loader/src/lib/transform/apply.ts index 9edb6ae78b..05cfbe3d27 100644 --- a/packages/config-loader/src/lib/transform/apply.ts +++ b/packages/config-loader/src/lib/transform/apply.ts @@ -23,23 +23,27 @@ import { isObject } from './utils'; * The transformation rewrites any special values, like the $secret key. */ export async function applyConfigTransforms( + initialDir: string, input: JsonValue, transforms: TransformFunc[], ): Promise { async function transform( inputObj: JsonValue, path: string, + baseDir: string, ): Promise { let obj = inputObj; + let dir = baseDir; for (const tf of transforms) { try { - const [applied, newObj] = await tf(inputObj); - if (applied) { - if (newObj === undefined) { - return newObj; + const result = await tf(inputObj, baseDir); + if (result.applied) { + if (result.value === undefined) { + return undefined; } - obj = newObj; + obj = result.value; + dir = result.newBaseDir ?? dir; break; } } catch (error) { @@ -55,7 +59,7 @@ export async function applyConfigTransforms( const arr = new Array(); for (const [index, value] of obj.entries()) { - const out = await transform(value, `${path}[${index}]`); + const out = await transform(value, `${path}[${index}]`, dir); if (out !== undefined) { arr.push(out); } @@ -69,7 +73,7 @@ export async function applyConfigTransforms( for (const [key, value] of Object.entries(obj)) { // undefined covers optional fields if (value !== undefined) { - const result = await transform(value, `${path}.${key}`); + const result = await transform(value, `${path}.${key}`, dir); if (result !== undefined) { out[key] = result; } @@ -79,7 +83,7 @@ export async function applyConfigTransforms( return out; } - const finalData = await transform(input, ''); + const finalData = await transform(input, '', initialDir); if (!isObject(finalData)) { throw new TypeError('expected object at config root'); } diff --git a/packages/config-loader/src/lib/transform/include.test.ts b/packages/config-loader/src/lib/transform/include.test.ts index b25b601cc9..4aea445ca9 100644 --- a/packages/config-loader/src/lib/transform/include.test.ts +++ b/packages/config-loader/src/lib/transform/include.test.ts @@ -24,11 +24,11 @@ const env = jest.fn(async (name: string) => { const readFile = jest.fn(async (path: string) => { const content = ({ - 'my-secret': 'secret', - 'my-data.json': '{"a":{"b":{"c":42}}}', - 'my-data.yaml': 'some:\n yaml:\n key: 7', - 'my-data.yml': 'different: { key: hello }', - 'invalid.yaml': 'foo: [}', + '/my-secret': 'secret', + '/my-data.json': '{"a":{"b":{"c":42}}}', + '/my-data.yaml': 'some:\n yaml:\n key: 7', + '/my-data.yml': 'different: { key: hello }', + '/invalid.yaml': 'foo: [}', } as { [key: string]: string })[path]; if (!content) { @@ -41,92 +41,89 @@ const includeTransform = createIncludeTransform(env, readFile); describe('includeTransform', () => { it('should not transform unknown values', async () => { - await expect(includeTransform('foo')).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(includeTransform([1])).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(includeTransform(1)).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(includeTransform({ x: 'y' })).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(includeTransform(null)).resolves.toEqual([false, null]); + await expect(includeTransform('foo', '/')).resolves.toEqual({ + applied: false, + }); + await expect(includeTransform([1], '/')).resolves.toEqual({ + applied: false, + }); + await expect(includeTransform(1, '/')).resolves.toEqual({ applied: false }); + await expect(includeTransform({ x: 'y' }, '/')).resolves.toEqual({ + applied: false, + }); + await expect(includeTransform(null, '/')).resolves.toEqual({ + applied: false, + }); }); it('should include text files', async () => { - await expect(includeTransform({ $file: 'my-secret' })).resolves.toEqual([ - true, - 'secret', - ]); - await expect(includeTransform({ $file: 'no-secret' })).rejects.toThrow( + await expect( + includeTransform({ $file: 'my-secret' }, '/'), + ).resolves.toEqual({ applied: true, value: 'secret' }); + await expect(includeTransform({ $file: 'no-secret' }, '/')).rejects.toThrow( 'File not found!', ); }); it('should include env vars', async () => { - await expect(includeTransform({ $env: 'SECRET' })).resolves.toEqual([ - true, - 'my-secret', - ]); - await expect(includeTransform({ $env: 'NO_SECRET' })).resolves.toEqual([ - true, - undefined, - ]); + await expect(includeTransform({ $env: 'SECRET' }, '/')).resolves.toEqual({ + applied: true, + value: 'my-secret', + }); + await expect(includeTransform({ $env: 'NO_SECRET' }, '/')).resolves.toEqual( + { + applied: true, + value: undefined, + }, + ); }); it('should include config files', async () => { // New format with path in fragment await expect( - includeTransform({ $include: 'my-data.json#a.b.c' }), - ).resolves.toEqual([true, 42]); + includeTransform({ $include: 'my-data.json#a.b.c' }, '/'), + ).resolves.toEqual({ applied: true, value: 42 }); await expect( - includeTransform({ $include: 'my-data.json#a.b' }), - ).resolves.toEqual([true, { c: 42 }]); + includeTransform({ $include: 'my-data.json#a.b' }, '/'), + ).resolves.toEqual({ applied: true, value: { c: 42 } }); await expect( - includeTransform({ $include: 'my-data.yaml#some.yaml.key' }), - ).resolves.toEqual([true, 7]); + includeTransform({ $include: 'my-data.yaml#some.yaml.key' }, '/'), + ).resolves.toEqual({ applied: true, value: 7 }); await expect( - includeTransform({ $include: 'my-data.yaml' }), - ).resolves.toEqual([ - true, - { + includeTransform({ $include: 'my-data.yaml' }, '/'), + ).resolves.toEqual({ + applied: true, + value: { some: { yaml: { key: 7 } }, }, - ]); + }); await expect( - includeTransform({ $include: 'my-data.yaml#' }), - ).resolves.toEqual([ - true, - { + includeTransform({ $include: 'my-data.yaml#' }, '/'), + ).resolves.toEqual({ + applied: true, + value: { some: { yaml: { key: 7 } }, }, - ]); + }); await expect( - includeTransform({ $include: 'my-data.yml#different.key' }), - ).resolves.toEqual([true, 'hello']); + includeTransform({ $include: 'my-data.yml#different.key' }, '/'), + ).resolves.toEqual({ applied: true, value: 'hello' }); }); it('should reject invalid includes', async () => { await expect( - includeTransform({ $include: 'no-parser.js' }), + includeTransform({ $include: 'no-parser.js' }, '/'), ).rejects.toThrow('no configuration parser available for extension .js'); await expect( - includeTransform({ $include: 'no-data.yml#different.key' }), + includeTransform({ $include: 'no-data.yml#different.key' }, '/'), ).rejects.toThrow('File not found!'); await expect( - includeTransform({ $include: 'my-data.yml#missing.key' }), + includeTransform({ $include: 'my-data.yml#missing.key' }, '/'), ).rejects.toThrow( "value at 'missing' in included file my-data.yml is not an object", ); await expect( - includeTransform({ $include: 'invalid.yaml' }), + includeTransform({ $include: 'invalid.yaml' }, '/'), ).rejects.toThrow( 'failed to parse included file invalid.yaml, YAMLSyntaxError: Flow sequence contains an unexpected }', ); diff --git a/packages/config-loader/src/lib/transform/include.ts b/packages/config-loader/src/lib/transform/include.ts index c08da09e7a..af476bc2e1 100644 --- a/packages/config-loader/src/lib/transform/include.ts +++ b/packages/config-loader/src/lib/transform/include.ts @@ -15,7 +15,7 @@ */ import yaml from 'yaml'; -import { extname } from 'path'; +import { extname, dirname, resolve as resolvePath } from 'path'; import { JsonObject, JsonValue } from '@backstage/config'; import { isObject } from './utils'; import { TransformFunc, EnvFunc, ReadFileFunc } from './types'; @@ -36,9 +36,9 @@ export function createIncludeTransform( env: EnvFunc, readFile: ReadFileFunc, ): TransformFunc { - return async (input: JsonValue) => { + return async (input: JsonValue, baseDir: string) => { if (!isObject(input)) { - return [false, input]; + return { applied: false }; } // Check if there's any key that starts with a '$', in that case we treat // this entire object as a secret. @@ -50,7 +50,7 @@ export function createIncludeTransform( ); } } else { - return [false, input]; + return { applied: false }; } const secretValue = input[secretKey]; @@ -61,13 +61,14 @@ export function createIncludeTransform( switch (secretKey) { case '$file': try { - return [true, await readFile(secretValue)]; + const value = await readFile(resolvePath(baseDir, secretValue)); + return { applied: true, value }; } catch (error) { throw new Error(`failed to read file ${secretValue}, ${error}`); } case '$env': try { - return [true, await env(secretValue)]; + return { applied: true, value: await env(secretValue) }; } catch (error) { throw new Error(`failed to read env ${secretValue}, ${error}`); } @@ -83,7 +84,9 @@ export function createIncludeTransform( ); } - const content = await readFile(filePath); + const path = resolvePath(baseDir, filePath); + const content = await readFile(path); + const newBaseDir = dirname(path); const parts = dataPath ? dataPath.split('.') : []; @@ -107,7 +110,11 @@ export function createIncludeTransform( value = value[part]; } - return [true, value]; + return { + applied: true, + value, + newBaseDir: newBaseDir !== baseDir ? newBaseDir : undefined, + }; } default: diff --git a/packages/config-loader/src/lib/transform/substitution.test.ts b/packages/config-loader/src/lib/transform/substitution.test.ts index b6795dbfc4..e2c0200bfd 100644 --- a/packages/config-loader/src/lib/transform/substitution.test.ts +++ b/packages/config-loader/src/lib/transform/substitution.test.ts @@ -27,42 +27,40 @@ const substituteTransform = createSubstitutionTransform(env); describe('substituteTransform', () => { it('should not transform unknown values', async () => { - await expect(substituteTransform(false)).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(substituteTransform([1])).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(substituteTransform(1)).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(substituteTransform({ x: 'y' })).resolves.toEqual([ - false, - expect.anything(), - ]); - await expect(substituteTransform(null)).resolves.toEqual([false, null]); + await expect(substituteTransform(false, '/')).resolves.toEqual({ + applied: false, + }); + await expect(substituteTransform([1], '/')).resolves.toEqual({ + applied: false, + }); + await expect(substituteTransform(1, '/')).resolves.toEqual({ + applied: false, + }); + await expect(substituteTransform({ x: 'y' }, '/')).resolves.toEqual({ + applied: false, + }); + await expect(substituteTransform(null, '/')).resolves.toEqual({ + applied: false, + }); }); it('should substitute env var', async () => { - await expect(substituteTransform('hello ${SECRET}')).resolves.toEqual([ - true, - 'hello my-secret', - ]); + await expect(substituteTransform('hello ${SECRET}', '/')).resolves.toEqual({ + applied: true, + value: 'hello my-secret', + }); await expect( - substituteTransform('${SECRET } $${} ${TOKEN }'), - ).resolves.toEqual([true, 'my-secret $${} my-token']); - await expect(substituteTransform('foo ${MISSING}')).resolves.toEqual([ - true, - undefined, - ]); + substituteTransform('${SECRET } $${} ${TOKEN }', '/'), + ).resolves.toEqual({ applied: true, value: 'my-secret $${} my-token' }); + await expect(substituteTransform('foo ${MISSING}', '/')).resolves.toEqual({ + applied: true, + value: undefined, + }); await expect( - substituteTransform('foo ${MISSING} ${SECRET}'), - ).resolves.toEqual([true, undefined]); + substituteTransform('foo ${MISSING} ${SECRET}', '/'), + ).resolves.toEqual({ applied: true, value: undefined }); await expect( - substituteTransform('foo ${SECRET} ${SECRET}'), - ).resolves.toEqual([true, 'foo my-secret my-secret']); + substituteTransform('foo ${SECRET} ${SECRET}', '/'), + ).resolves.toEqual({ applied: true, value: 'foo my-secret my-secret' }); }); }); diff --git a/packages/config-loader/src/lib/transform/substitution.ts b/packages/config-loader/src/lib/transform/substitution.ts index d1336edd4c..821cc8bc67 100644 --- a/packages/config-loader/src/lib/transform/substitution.ts +++ b/packages/config-loader/src/lib/transform/substitution.ts @@ -25,7 +25,7 @@ import { TransformFunc, EnvFunc } from './types'; export function createSubstitutionTransform(env: EnvFunc): TransformFunc { return async (input: JsonValue) => { if (typeof input !== 'string') { - return [false, input]; + return { applied: false }; } const parts: (string | undefined)[] = input.split(/(? part === undefined)) { - return [true, undefined]; + return { applied: true, value: undefined }; } - return [true, parts.join('')]; + return { applied: true, value: parts.join('') }; }; } diff --git a/packages/config-loader/src/lib/transform/types.ts b/packages/config-loader/src/lib/transform/types.ts index b632547485..c13f01d016 100644 --- a/packages/config-loader/src/lib/transform/types.ts +++ b/packages/config-loader/src/lib/transform/types.ts @@ -22,4 +22,14 @@ export type ReadFileFunc = (path: string) => Promise; export type TransformFunc = ( value: JsonValue, -) => Promise<[boolean, JsonValue | undefined]>; + baseDir: string, +) => Promise< + | { + applied: false; + } + | { + applied: true; + value: JsonValue | undefined; + newBaseDir?: string | undefined; + } +>; diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index bb6262ce13..8c7de557e3 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/packages/config-loader/src/loader.test.ts @@ -33,8 +33,14 @@ describe('loadConfig', () => { sessionKey: development-key backend: $include: ./included.yaml + other: + $include: secrets/included.yaml `, '/root/secrets/session-key.txt': 'abc123', + '/root/secrets/included.yaml': ` + secret: + $file: session-key.txt + `, '/root/included.yaml': ` foo: bar: token \${MY_SECRET} @@ -117,6 +123,9 @@ describe('loadConfig', () => { bar: 'token is-secret', }, }, + other: { + secret: 'abc123', + }, }, }, ]); diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 60a2394ccf..54e4ab5208 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -75,7 +75,7 @@ export async function loadConfig( fs.readFile(resolvePath(dir, path), 'utf8'); const input = yaml.parse(await readFile(configPath)); - const data = await applyConfigTransforms(input, [ + const data = await applyConfigTransforms(dir, input, [ createIncludeTransform(env, readFile), createSubstitutionTransform(env), ]);