From eb0c0c7e20d4804b847dd720036ade6d797ba2ff Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Jan 2021 17:08:44 +0100 Subject: [PATCH 01/14] config-loader: remove deprecated $secret support --- packages/config-loader/src/lib/reader.test.ts | 43 +------------------ packages/config-loader/src/lib/reader.ts | 16 ------- 2 files changed, 1 insertion(+), 58 deletions(-) diff --git a/packages/config-loader/src/lib/reader.test.ts b/packages/config-loader/src/lib/reader.test.ts index a0a8495714..0a3fe01bf7 100644 --- a/packages/config-loader/src/lib/reader.test.ts +++ b/packages/config-loader/src/lib/reader.test.ts @@ -123,50 +123,9 @@ describe('readConfigFile', () => { expect(readSecret).not.toHaveBeenCalled(); }); - it('should read deprecated secrets', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { $secret: { file: "./my-secret" } }', - }); - const readSecret = jest.fn().mockResolvedValue('secret'); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - readSecret: readSecret as ReadSecretFunc, - }); - - await expect(config).resolves.toEqual({ - data: { - app: 'secret', - }, - context: 'app-config.yaml', - }); - expect(readSecret).toHaveBeenCalledWith('.app', { - file: './my-secret', - }); - }); - - it('should require deprecated secrets to be objects', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { $secret: ["wrong-type"] }', - }); - const readSecret = jest.fn().mockResolvedValue('secret'); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - readSecret: readSecret as ReadSecretFunc, - }); - - expect(readSecret).not.toHaveBeenCalled(); - await expect(config).rejects.toThrow( - 'Expected object at secret .app.$secret', - ); - }); - it('should forward secret reading errors', async () => { const readFile = memoryFiles({ - './app-config.yaml': 'app: { $secret: {} }', + './app-config.yaml': 'app: { $file: {} }', }); const readSecret = jest.fn().mockRejectedValue(new Error('NOPE')); diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts index 9eba58be97..0996de6ce7 100644 --- a/packages/config-loader/src/lib/reader.ts +++ b/packages/config-loader/src/lib/reader.ts @@ -54,22 +54,6 @@ export async function readConfigFile( return arr; } - // TODO(Rugvip): This form of declaring secrets is deprecated, warn and remove in the future - if ('$secret' in obj) { - console.warn( - `Deprecated secret declaration at '${path}' in '${context}', use $env, $file, etc. instead`, - ); - if (!isObject(obj.$secret)) { - throw TypeError(`Expected object at secret ${path}.$secret`); - } - - try { - return await ctx.readSecret(path, obj.$secret); - } catch (error) { - throw new Error(`Invalid secret at ${path}: ${error.message}`); - } - } - // Check if there's any key that starts with a '$', in that case we treat // this entire object as a secret. const [secretKey] = Object.keys(obj).filter(key => key.startsWith('$')); From 05f696ced14d8693dfc2c05449140ce57d72483f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Jan 2021 17:14:56 +0100 Subject: [PATCH 02/14] config-loader: remove deprecated $data secret support --- .../config-loader/src/lib/secrets.test.ts | 43 ++--------------- packages/config-loader/src/lib/secrets.ts | 48 +------------------ 2 files changed, 6 insertions(+), 85 deletions(-) diff --git a/packages/config-loader/src/lib/secrets.test.ts b/packages/config-loader/src/lib/secrets.test.ts index 189834f3aa..5dd06a9680 100644 --- a/packages/config-loader/src/lib/secrets.test.ts +++ b/packages/config-loader/src/lib/secrets.test.ts @@ -55,36 +55,6 @@ describe('readSecret', () => { ); }); - it('should read data secrets', async () => { - // Deprecated object form - await expect( - readSecret({ data: 'my-data.json', path: 'a.b.c' }, ctx), - ).resolves.toBe('42'); - await expect( - readSecret({ data: 'my-data.yaml', path: 'some.yaml.key' }, ctx), - ).resolves.toBe('7'); - await expect( - readSecret({ data: 'my-data.yml', path: 'different.key' }, ctx), - ).resolves.toBe('hello'); - await expect( - readSecret({ data: 'no-data.yml', path: 'different.key' }, ctx), - ).rejects.toThrow('File not found!'); - - // New format with path in fragment - await expect(readSecret({ data: 'my-data.json#a.b.c' }, ctx)).resolves.toBe( - '42', - ); - await expect( - readSecret({ data: 'my-data.yaml#some.yaml.key' }, ctx), - ).resolves.toBe('7'); - await expect( - readSecret({ data: 'my-data.yml#different.key' }, ctx), - ).resolves.toBe('hello'); - await expect( - readSecret({ data: 'no-data.yml#different.key' }, ctx), - ).rejects.toThrow('File not found!'); - }); - it('should include extra files', async () => { // New format with path in fragment await expect( @@ -125,19 +95,16 @@ describe('readSecret', () => { 'secret must be a `object` type, but the final value was: `"hello"`.', ); await expect(readSecret({}, ctx)).rejects.toThrow( - "Secret must contain one of 'file', 'env', 'data'", + "Secret must contain one of 'file', 'env', 'include'", ); await expect(readSecret({ unknown: 'derp' }, ctx)).rejects.toThrow( - "Secret must contain one of 'file', 'env', 'data'", + "Secret must contain one of 'file', 'env', 'include'", ); - await expect(readSecret({ data: 'no-data.yml' }, ctx)).rejects.toThrow( - "Invalid format for data secret value, must be of the form #, got 'no-data.yml'", + await expect(readSecret({ include: 'no-parser.js' }, ctx)).rejects.toThrow( + 'No data secret parser available for extension .js', ); await expect( - readSecret({ data: 'no-parser.js', path: '.' }, ctx), - ).rejects.toThrow('No data secret parser available for extension .js'); - await expect( - readSecret({ data: 'my-data.yaml', path: 'some.wrong.yaml.key' }, ctx), + readSecret({ include: 'my-data.yaml#some.wrong.yaml.key' }, ctx), ).rejects.toThrow('Value is not an object at some.wrong in my-data.yaml'); }); diff --git a/packages/config-loader/src/lib/secrets.ts b/packages/config-loader/src/lib/secrets.ts index b0e0a0dd7b..4ce87c6ace 100644 --- a/packages/config-loader/src/lib/secrets.ts +++ b/packages/config-loader/src/lib/secrets.ts @@ -33,21 +33,12 @@ type EnvSecret = { env: string; }; -// Reads a secret from a json-like file and extracts a value at a path. -// The supported extensions are define in dataSecretParser below. -type DataSecret = { - // Path to the data secret file, relative to the config file. - data: string; - // The path to the value inside the data file, each element separated by '.'. - path?: string; -}; - // TODO(Rugvip): Move this out of secret reading when we remove the deprecated DataSecret and $secret format type IncludeSecret = { include: string; }; -type Secret = FileSecret | EnvSecret | DataSecret | IncludeSecret; +type Secret = FileSecret | EnvSecret | IncludeSecret; // Schema for each type of secret description const secretLoaderSchemas = { @@ -57,9 +48,6 @@ const secretLoaderSchemas = { env: yup.object({ env: yup.string().required(), }), - data: yup.object({ - data: yup.string().required(), - }), include: yup.object({ include: yup.string().required(), }), @@ -111,40 +99,6 @@ export async function readSecret( if ('env' in secret) { return ctx.env[secret.env]; } - if ('data' in secret) { - console.warn( - `Configuration uses deprecated $data key, use $include instead.`, - ); - const url = - 'path' in secret ? `${secret.data}#${secret.path}` : secret.data; - const [filePath, dataPath] = url.split(/#(.*)/); - if (!dataPath) { - throw new Error( - `Invalid format for data secret value, must be of the form #, got '${url}'`, - ); - } - - const ext = extname(filePath); - const parser = dataSecretParser[ext]; - if (!parser) { - throw new Error(`No data secret parser available for extension ${ext}`); - } - - const content = await ctx.readFile(filePath); - - const parts = dataPath.split('.'); - - let value: JsonValue | undefined = await parser(content); - for (const [index, part] of parts.entries()) { - if (!isObject(value)) { - const errPath = parts.slice(0, index).join('.'); - throw new Error(`Value is not an object at ${errPath} in ${filePath}`); - } - value = value[part]; - } - - return String(value); - } if ('include' in secret) { const [filePath, dataPath] = secret.include.split(/#(.*)/); From 26a94cfac6468da1fb6a261401716a34b9ba5b65 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Jan 2021 18:59:45 +0100 Subject: [PATCH 03/14] config-loader: refactor to use transforms as a base building block --- .../config-loader/src/lib/include.test.ts | 134 +++++++++++++++++ packages/config-loader/src/lib/include.ts | 117 +++++++++++++++ packages/config-loader/src/lib/index.ts | 4 +- packages/config-loader/src/lib/reader.test.ts | 140 ------------------ .../config-loader/src/lib/secrets.test.ts | 129 ---------------- packages/config-loader/src/lib/secrets.ts | 134 ----------------- .../config-loader/src/lib/transform.test.ts | 82 ++++++++++ .../src/lib/{reader.ts => transform.ts} | 64 ++++---- packages/config-loader/src/lib/types.ts | 20 +-- packages/config-loader/src/lib/utils.ts | 5 - packages/config-loader/src/loader.ts | 57 +++---- 11 files changed, 390 insertions(+), 496 deletions(-) create mode 100644 packages/config-loader/src/lib/include.test.ts create mode 100644 packages/config-loader/src/lib/include.ts delete mode 100644 packages/config-loader/src/lib/reader.test.ts delete mode 100644 packages/config-loader/src/lib/secrets.test.ts delete mode 100644 packages/config-loader/src/lib/secrets.ts create mode 100644 packages/config-loader/src/lib/transform.test.ts rename packages/config-loader/src/lib/{reader.ts => transform.ts} (59%) diff --git a/packages/config-loader/src/lib/include.test.ts b/packages/config-loader/src/lib/include.test.ts new file mode 100644 index 0000000000..b25b601cc9 --- /dev/null +++ b/packages/config-loader/src/lib/include.test.ts @@ -0,0 +1,134 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createIncludeTransform } from './include'; + +const env = jest.fn(async (name: string) => { + return ({ + SECRET: 'my-secret', + } as { [name: string]: string })[name]; +}); + +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: [}', + } as { [key: string]: string })[path]; + + if (!content) { + throw new Error('File not found!'); + } + return content; +}); + +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]); + }); + + it('should include text files', async () => { + await expect(includeTransform({ $file: 'my-secret' })).resolves.toEqual([ + true, + '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, + ]); + }); + + 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]); + await expect( + includeTransform({ $include: 'my-data.json#a.b' }), + ).resolves.toEqual([true, { c: 42 }]); + await expect( + includeTransform({ $include: 'my-data.yaml#some.yaml.key' }), + ).resolves.toEqual([true, 7]); + await expect( + includeTransform({ $include: 'my-data.yaml' }), + ).resolves.toEqual([ + true, + { + some: { yaml: { key: 7 } }, + }, + ]); + await expect( + includeTransform({ $include: 'my-data.yaml#' }), + ).resolves.toEqual([ + true, + { + some: { yaml: { key: 7 } }, + }, + ]); + await expect( + includeTransform({ $include: 'my-data.yml#different.key' }), + ).resolves.toEqual([true, 'hello']); + }); + + it('should reject invalid includes', async () => { + await expect( + includeTransform({ $include: 'no-parser.js' }), + ).rejects.toThrow('no configuration parser available for extension .js'); + await expect( + includeTransform({ $include: 'no-data.yml#different.key' }), + ).rejects.toThrow('File not found!'); + await expect( + 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' }), + ).rejects.toThrow( + 'failed to parse included file invalid.yaml, YAMLSyntaxError: Flow sequence contains an unexpected }', + ); + }); +}); diff --git a/packages/config-loader/src/lib/include.ts b/packages/config-loader/src/lib/include.ts new file mode 100644 index 0000000000..c08da09e7a --- /dev/null +++ b/packages/config-loader/src/lib/include.ts @@ -0,0 +1,117 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import yaml from 'yaml'; +import { extname } from 'path'; +import { JsonObject, JsonValue } from '@backstage/config'; +import { isObject } from './utils'; +import { TransformFunc, EnvFunc, ReadFileFunc } from './types'; + +// Parsers for each type of included file +const includeFileParser: { + [ext in string]: (content: string) => Promise; +} = { + '.json': async content => JSON.parse(content), + '.yaml': async content => yaml.parse(content), + '.yml': async content => yaml.parse(content), +}; + +/** + * Transforms a secret description into the actual secret value. + */ +export function createIncludeTransform( + env: EnvFunc, + readFile: ReadFileFunc, +): TransformFunc { + return async (input: JsonValue) => { + if (!isObject(input)) { + return [false, input]; + } + // Check if there's any key that starts with a '$', in that case we treat + // this entire object as a secret. + const [secretKey] = Object.keys(input).filter(key => key.startsWith('$')); + if (secretKey) { + if (Object.keys(input).length !== 1) { + throw new Error( + `include key ${secretKey} should not have adjacent keys`, + ); + } + } else { + return [false, input]; + } + + const secretValue = input[secretKey]; + if (typeof secretValue !== 'string') { + throw new Error(`${secretKey} include value is not a string`); + } + + switch (secretKey) { + case '$file': + try { + return [true, await readFile(secretValue)]; + } catch (error) { + throw new Error(`failed to read file ${secretValue}, ${error}`); + } + case '$env': + try { + return [true, await env(secretValue)]; + } catch (error) { + throw new Error(`failed to read env ${secretValue}, ${error}`); + } + + case '$include': { + const [filePath, dataPath] = secretValue.split(/#(.*)/); + + const ext = extname(filePath); + const parser = includeFileParser[ext]; + if (!parser) { + throw new Error( + `no configuration parser available for included file ${filePath}`, + ); + } + + const content = await readFile(filePath); + + const parts = dataPath ? dataPath.split('.') : []; + + let value: JsonValue | undefined; + try { + value = await parser(content); + } catch (error) { + throw new Error( + `failed to parse included file ${filePath}, ${error}`, + ); + } + + // This bit handles selecting a subtree in the included file, if a path was provided after a # + for (const [index, part] of parts.entries()) { + if (!isObject(value)) { + const errPath = parts.slice(0, index).join('.'); + throw new Error( + `value at '${errPath}' in included file ${filePath} is not an object`, + ); + } + value = value[part]; + } + + return [true, value]; + } + + default: + throw new Error(`unknown secret ${secretKey}`); + } + }; +} diff --git a/packages/config-loader/src/lib/index.ts b/packages/config-loader/src/lib/index.ts index ceb7c34222..90e64546f6 100644 --- a/packages/config-loader/src/lib/index.ts +++ b/packages/config-loader/src/lib/index.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -export { readConfigFile } from './reader'; +export { applyConfigTransforms } from './transform'; export { readEnvConfig } from './env'; -export { readSecret } from './secrets'; +export { createIncludeTransform } from './include'; export * from './schema'; diff --git a/packages/config-loader/src/lib/reader.test.ts b/packages/config-loader/src/lib/reader.test.ts deleted file mode 100644 index 0a3fe01bf7..0000000000 --- a/packages/config-loader/src/lib/reader.test.ts +++ /dev/null @@ -1,140 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { readConfigFile } from './reader'; -import { ReaderContext, ReadSecretFunc } from './types'; - -function memoryFiles(files: { [path: string]: string }) { - return async (path: string) => { - if (path in files) { - return files[path]; - } - throw new Error(`File not found, ${path}`); - }; -} - -const mockContext: ReaderContext = { - env: {}, - readFile: jest.fn(), - readSecret: jest.fn(), -}; - -describe('readConfigFile', () => { - it('should read a plain config file', async () => { - const readFile = memoryFiles({ - './app-config.yaml': - 'app: { title: "Test", x: 1, y: [null, true], z: null }', - }); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - }); - - await expect(config).resolves.toEqual({ - data: { - app: { - title: 'Test', - x: 1, - y: [true], - }, - }, - context: 'app-config.yaml', - }); - }); - - it('should error out if the config file has invalid syntax', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { title: ]', - }); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - }); - - await expect(config).rejects.toThrow('Flow map contains an unexpected ]'); - }); - - it('should error out if config is not an object', async () => { - const readFile = memoryFiles({ - './app-config.yaml': '[]', - }); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - }); - - await expect(config).rejects.toThrow('Expected object at config root'); - }); - - it('should read secrets', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { $file: "./my-secret" }', - }); - const readSecret = jest.fn().mockResolvedValue('secret'); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - readSecret: readSecret as ReadSecretFunc, - }); - - await expect(config).resolves.toEqual({ - data: { - app: 'secret', - }, - context: 'app-config.yaml', - }); - expect(readSecret).toHaveBeenCalledWith('.app', { - file: './my-secret', - }); - }); - - it('should not allow keys adjacent to secrets', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { extraKey: 3, $file: "./my-secret" }', - }); - const readSecret = jest.fn().mockResolvedValue('secret'); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - readSecret: readSecret as ReadSecretFunc, - }); - - await expect(config).rejects.toThrow( - "Secret key '$file' has adjacent keys at .app", - ); - expect(readSecret).not.toHaveBeenCalled(); - }); - - it('should forward secret reading errors', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { $file: {} }', - }); - const readSecret = jest.fn().mockRejectedValue(new Error('NOPE')); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - readSecret: readSecret as ReadSecretFunc, - }); - - await expect(config).rejects.toThrow('Invalid secret at .app: NOPE'); - }); -}); diff --git a/packages/config-loader/src/lib/secrets.test.ts b/packages/config-loader/src/lib/secrets.test.ts deleted file mode 100644 index 5dd06a9680..0000000000 --- a/packages/config-loader/src/lib/secrets.test.ts +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { readSecret } from './secrets'; -import { ReaderContext } from './types'; - -const ctx: ReaderContext = { - env: { - SECRET: 'my-secret', - }, - readSecret: jest.fn(), - async readFile(path) { - 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: [}', - } as { [key: string]: string })[path]; - - if (!content) { - throw new Error('File not found!'); - } - return content; - }, -}; - -describe('readSecret', () => { - it('should read file secrets', async () => { - await expect(readSecret({ file: 'my-secret' }, ctx)).resolves.toBe( - 'secret', - ); - await expect(readSecret({ file: 'no-secret' }, ctx)).rejects.toThrow( - 'File not found!', - ); - }); - - it('should read present env secrets', async () => { - await expect(readSecret({ env: 'SECRET' }, ctx)).resolves.toBe('my-secret'); - await expect(readSecret({ env: 'NO_SECRET' }, ctx)).resolves.toBe( - undefined, - ); - }); - - it('should include extra files', async () => { - // New format with path in fragment - await expect( - readSecret({ include: 'my-data.json#a.b.c' }, ctx), - ).resolves.toBe(42); - await expect( - readSecret({ include: 'my-data.json#a.b' }, ctx), - ).resolves.toEqual({ c: 42 }); - await expect( - readSecret({ include: 'my-data.yaml#some.yaml.key' }, ctx), - ).resolves.toBe(7); - await expect(readSecret({ include: 'my-data.yaml' }, ctx)).resolves.toEqual( - { - some: { yaml: { key: 7 } }, - }, - ); - await expect( - readSecret({ include: 'my-data.yaml#' }, ctx), - ).resolves.toEqual({ - some: { yaml: { key: 7 } }, - }); - await expect( - readSecret({ include: 'my-data.yml#different.key' }, ctx), - ).resolves.toBe('hello'); - await expect( - readSecret({ include: 'no-data.yml#different.key' }, ctx), - ).rejects.toThrow('File not found!'); - await expect( - readSecret({ include: 'my-data.yml#missing.key' }, ctx), - ).rejects.toThrow('Value is not an object at missing in my-data.yml'); - await expect(readSecret({ include: 'invalid.yaml' }, ctx)).rejects.toThrow( - 'Failed to parse included file invalid.yaml, YAMLSyntaxError: Flow sequence contains an unexpected }', - ); - }); - - it('should reject invalid secrets', async () => { - await expect(readSecret('hello' as any, ctx)).rejects.toThrow( - 'secret must be a `object` type, but the final value was: `"hello"`.', - ); - await expect(readSecret({}, ctx)).rejects.toThrow( - "Secret must contain one of 'file', 'env', 'include'", - ); - await expect(readSecret({ unknown: 'derp' }, ctx)).rejects.toThrow( - "Secret must contain one of 'file', 'env', 'include'", - ); - await expect(readSecret({ include: 'no-parser.js' }, ctx)).rejects.toThrow( - 'No data secret parser available for extension .js', - ); - await expect( - readSecret({ include: 'my-data.yaml#some.wrong.yaml.key' }, ctx), - ).rejects.toThrow('Value is not an object at some.wrong in my-data.yaml'); - }); - - it('should have 100% test coverage', async () => { - let firstVisit = true; - const secret = {}; - const proto = { - get file() { - if (!firstVisit) { - Object.setPrototypeOf(secret, {}); - } - firstVisit = false; - return 'a-file'; - }, - }; - Object.setPrototypeOf(secret, proto); - - await expect(readSecret(secret, ctx)).rejects.toThrow( - 'Secret was left unhandled', - ); - }); -}); diff --git a/packages/config-loader/src/lib/secrets.ts b/packages/config-loader/src/lib/secrets.ts deleted file mode 100644 index 4ce87c6ace..0000000000 --- a/packages/config-loader/src/lib/secrets.ts +++ /dev/null @@ -1,134 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as yup from 'yup'; -import yaml from 'yaml'; -import { extname } from 'path'; -import { JsonObject, JsonValue } from '@backstage/config'; -import { isObject, isNever } from './utils'; -import { ReaderContext } from './types'; - -// Reads a file and forwards the contents as is, assuming ut8 encoding -type FileSecret = { - // Path to the secret file, relative to the config file. - file: string; -}; - -// Reads the secret from an environment variable. -type EnvSecret = { - // The name of the environment file. - env: string; -}; - -// TODO(Rugvip): Move this out of secret reading when we remove the deprecated DataSecret and $secret format -type IncludeSecret = { - include: string; -}; - -type Secret = FileSecret | EnvSecret | IncludeSecret; - -// Schema for each type of secret description -const secretLoaderSchemas = { - file: yup.object({ - file: yup.string().required(), - }), - env: yup.object({ - env: yup.string().required(), - }), - include: yup.object({ - include: yup.string().required(), - }), -}; - -// The top-level secret schema, which figures out what type of secret it is. -const secretSchema = yup.lazy(value => { - if (typeof value !== 'object' || value === null) { - return yup.object().required().label('secret'); - } - - const loaderTypes = Object.keys( - secretLoaderSchemas, - ) as (keyof typeof secretLoaderSchemas)[]; - - for (const key of loaderTypes) { - if (key in value) { - return secretLoaderSchemas[key]; - } - } - throw new yup.ValidationError( - `Secret must contain one of '${loaderTypes.join("', '")}'`, - value, - '$secret', - ); -}); - -// Parsers for each type of data secret file. -const dataSecretParser: { - [ext in string]: (content: string) => Promise; -} = { - '.json': async content => JSON.parse(content), - '.yaml': async content => yaml.parse(content), - '.yml': async content => yaml.parse(content), -}; - -/** - * Transforms a secret description into the actual secret value. - */ -export async function readSecret( - data: JsonObject, - ctx: ReaderContext, -): Promise { - const secret = secretSchema.validateSync(data, { strict: true }) as Secret; - - if ('file' in secret) { - return ctx.readFile(secret.file); - } - if ('env' in secret) { - return ctx.env[secret.env]; - } - if ('include' in secret) { - const [filePath, dataPath] = secret.include.split(/#(.*)/); - - const ext = extname(filePath); - const parser = dataSecretParser[ext]; - if (!parser) { - throw new Error(`No data secret parser available for extension ${ext}`); - } - - const content = await ctx.readFile(filePath); - - const parts = dataPath ? dataPath.split('.') : []; - - let value: JsonValue | undefined; - try { - value = await parser(content); - } catch (error) { - throw new Error(`Failed to parse included file ${filePath}, ${error}`); - } - for (const [index, part] of parts.entries()) { - if (!isObject(value)) { - const errPath = parts.slice(0, index).join('.'); - throw new Error(`Value is not an object at ${errPath} in ${filePath}`); - } - value = value[part]; - } - - return value; - } - - isNever(); - throw new Error('Secret was left unhandled'); -} diff --git a/packages/config-loader/src/lib/transform.test.ts b/packages/config-loader/src/lib/transform.test.ts new file mode 100644 index 0000000000..633bae0a53 --- /dev/null +++ b/packages/config-loader/src/lib/transform.test.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { applyConfigTransforms } from './transform'; + +describe('applyConfigTransforms', () => { + it('should apply not transforms to input', async () => { + const data = applyConfigTransforms( + { + app: { + title: 'Test', + x: 1, + y: [null, true], + z: null, + }, + }, + [], + ); + + await expect(data).resolves.toEqual({ + app: { + title: 'Test', + x: 1, + y: [true], + }, + }); + }); + + it('should throw if input is not an object', async () => { + 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', + x: 1, + y: [null, true], + z: null, + }, + }, + [ + async value => { + if (typeof value === 'number') { + return [true, value + 1]; + } + return [false, value]; + }, + async value => { + if (typeof value === 'string' && value.length > 1) { + return [true, value.split('')]; + } + return [false, value]; + }, + ], + ); + + await expect(config).resolves.toEqual({ + app: { + title: ['T', 'e', 's', 't'], + x: 2, + y: [true], + }, + }); + }); +}); diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/transform.ts similarity index 59% rename from packages/config-loader/src/lib/reader.ts rename to packages/config-loader/src/lib/transform.ts index 0996de6ce7..9edb6ae78b 100644 --- a/packages/config-loader/src/lib/reader.ts +++ b/packages/config-loader/src/lib/transform.ts @@ -14,29 +14,39 @@ * limitations under the License. */ -import { AppConfig, JsonObject, JsonValue } from '@backstage/config'; -import { basename } from 'path'; -import yaml from 'yaml'; -import { ReaderContext } from './types'; +import { JsonObject, JsonValue } from '@backstage/config'; +import { TransformFunc } from './types'; import { isObject } from './utils'; /** * Reads and parses, and validates, and transforms a single config file. * The transformation rewrites any special values, like the $secret key. */ -export async function readConfigFile( - filePath: string, - ctx: ReaderContext, -): Promise { - const configYaml = await ctx.readFile(filePath); - const config = yaml.parse(configYaml); - - const context = basename(filePath); - +export async function applyConfigTransforms( + input: JsonValue, + transforms: TransformFunc[], +): Promise { async function transform( - obj: JsonValue, + inputObj: JsonValue, path: string, ): Promise { + let obj = inputObj; + + for (const tf of transforms) { + try { + const [applied, newObj] = await tf(inputObj); + if (applied) { + if (newObj === undefined) { + return newObj; + } + obj = newObj; + break; + } + } catch (error) { + throw new Error(`error at ${path}, ${error.message}`); + } + } + if (typeof obj !== 'object') { return obj; } else if (obj === null) { @@ -54,24 +64,6 @@ export async function readConfigFile( return arr; } - // Check if there's any key that starts with a '$', in that case we treat - // this entire object as a secret. - const [secretKey] = Object.keys(obj).filter(key => key.startsWith('$')); - if (secretKey) { - if (Object.keys(obj).length !== 1) { - throw new Error( - `Secret key '${secretKey}' has adjacent keys at ${path}`, - ); - } - try { - return await ctx.readSecret(path, { - [secretKey.slice(1)]: obj[secretKey], - }); - } catch (error) { - throw new Error(`Invalid secret at ${path}: ${error.message}`); - } - } - const out: JsonObject = {}; for (const [key, value] of Object.entries(obj)) { @@ -87,9 +79,9 @@ export async function readConfigFile( return out; } - const finalConfig = await transform(config, ''); - if (!isObject(finalConfig)) { - throw new TypeError('Expected object at config root'); + const finalData = await transform(input, ''); + if (!isObject(finalData)) { + throw new TypeError('expected object at config root'); } - return { data: finalConfig, context }; + return finalData; } diff --git a/packages/config-loader/src/lib/types.ts b/packages/config-loader/src/lib/types.ts index 95e1d6655e..b632547485 100644 --- a/packages/config-loader/src/lib/types.ts +++ b/packages/config-loader/src/lib/types.ts @@ -14,20 +14,12 @@ * limitations under the License. */ -import { JsonObject, JsonValue } from '@backstage/config'; +import { JsonValue } from '@backstage/config'; + +export type EnvFunc = (name: string) => Promise; export type ReadFileFunc = (path: string) => Promise; -export type ReadSecretFunc = ( - path: string, - desc: JsonObject, -) => Promise; -export type SkipFunc = (path: string) => boolean; -/** - * Common context that provides all the necessary hooks for reading configuration files. - */ -export type ReaderContext = { - env: { [name in string]?: string }; - readFile: ReadFileFunc; - readSecret: ReadSecretFunc; -}; +export type TransformFunc = ( + value: JsonValue, +) => Promise<[boolean, JsonValue | undefined]>; diff --git a/packages/config-loader/src/lib/utils.ts b/packages/config-loader/src/lib/utils.ts index 37145ec971..9a72bc3c0e 100644 --- a/packages/config-loader/src/lib/utils.ts +++ b/packages/config-loader/src/lib/utils.ts @@ -24,8 +24,3 @@ export function isObject(obj: JsonValue | undefined): obj is JsonObject { } return obj !== null; } - -// A thing to make sure we've narrowed the type down to never -export function isNever() { - return void 0 as T; -} diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 11cb1dc338..71553a3dd7 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -15,9 +15,14 @@ */ import fs from 'fs-extra'; -import { resolve as resolvePath, dirname, isAbsolute } from 'path'; -import { AppConfig, JsonObject, JsonValue } from '@backstage/config'; -import { readConfigFile, readEnvConfig, readSecret } from './lib'; +import yaml from 'yaml'; +import { resolve as resolvePath, dirname, isAbsolute, basename } from 'path'; +import { AppConfig } from '@backstage/config'; +import { + applyConfigTransforms, + readEnvConfig, + createIncludeTransform, +} from './lib'; export type LoadConfigOptions = { // The root directory of the config loading context. Used to find default configs. @@ -30,30 +35,6 @@ export type LoadConfigOptions = { env: string; }; -class Context { - constructor( - private readonly options: { - env: { [name in string]?: string }; - rootPath: string; - }, - ) {} - - get env() { - return this.options.env; - } - - async readFile(path: string): Promise { - return fs.readFile(resolvePath(this.options.rootPath, path), 'utf8'); - } - - async readSecret( - _path: string, - desc: JsonObject, - ): Promise { - return readSecret(desc, this); - } -} - export async function loadConfig( options: LoadConfigOptions, ): Promise { @@ -82,24 +63,28 @@ export async function loadConfig( } } + const env = async (name: string) => process.env[name]; + try { for (const configPath of configPaths) { if (!isAbsolute(configPath)) { throw new Error(`Config load path is not absolute: '${configPath}'`); } - const config = await readConfigFile( - configPath, - new Context({ - env: process.env, - rootPath: dirname(configPath), - }), - ); - configs.push(config); + const dir = dirname(configPath); + const readFile = (path: string) => + fs.readFile(resolvePath(dir, path), 'utf8'); + + const input = yaml.parse(await readFile(configPath)); + const data = await applyConfigTransforms(input, [ + createIncludeTransform(env, readFile), + ]); + + configs.push({ data, context: basename(configPath) }); } } catch (error) { throw new Error( - `Failed to read static configuration file: ${error.message}`, + `Failed to read static configuration file, ${error.message}`, ); } From 011d736978c464ce237388f388aacec37d567c96 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Jan 2021 22:08:30 +0100 Subject: [PATCH 04/14] config-loader: move transform logic into separate lib folder --- packages/config-loader/src/lib/index.ts | 3 +-- .../apply.test.ts} | 2 +- .../lib/{transform.ts => transform/apply.ts} | 0 .../src/lib/{ => transform}/include.test.ts | 0 .../src/lib/{ => transform}/include.ts | 0 .../config-loader/src/lib/transform/index.ts | 18 ++++++++++++++++++ .../src/lib/{ => transform}/types.ts | 0 .../src/lib/{ => transform}/utils.ts | 0 8 files changed, 20 insertions(+), 3 deletions(-) rename packages/config-loader/src/lib/{transform.test.ts => transform/apply.test.ts} (97%) rename packages/config-loader/src/lib/{transform.ts => transform/apply.ts} (100%) rename packages/config-loader/src/lib/{ => transform}/include.test.ts (100%) rename packages/config-loader/src/lib/{ => transform}/include.ts (100%) create mode 100644 packages/config-loader/src/lib/transform/index.ts rename packages/config-loader/src/lib/{ => transform}/types.ts (100%) rename packages/config-loader/src/lib/{ => transform}/utils.ts (100%) diff --git a/packages/config-loader/src/lib/index.ts b/packages/config-loader/src/lib/index.ts index 90e64546f6..192ac81f5d 100644 --- a/packages/config-loader/src/lib/index.ts +++ b/packages/config-loader/src/lib/index.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -export { applyConfigTransforms } from './transform'; export { readEnvConfig } from './env'; -export { createIncludeTransform } from './include'; +export * from './transform'; export * from './schema'; diff --git a/packages/config-loader/src/lib/transform.test.ts b/packages/config-loader/src/lib/transform/apply.test.ts similarity index 97% rename from packages/config-loader/src/lib/transform.test.ts rename to packages/config-loader/src/lib/transform/apply.test.ts index 633bae0a53..da790978ea 100644 --- a/packages/config-loader/src/lib/transform.test.ts +++ b/packages/config-loader/src/lib/transform/apply.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { applyConfigTransforms } from './transform'; +import { applyConfigTransforms } from './apply'; describe('applyConfigTransforms', () => { it('should apply not transforms to input', async () => { diff --git a/packages/config-loader/src/lib/transform.ts b/packages/config-loader/src/lib/transform/apply.ts similarity index 100% rename from packages/config-loader/src/lib/transform.ts rename to packages/config-loader/src/lib/transform/apply.ts diff --git a/packages/config-loader/src/lib/include.test.ts b/packages/config-loader/src/lib/transform/include.test.ts similarity index 100% rename from packages/config-loader/src/lib/include.test.ts rename to packages/config-loader/src/lib/transform/include.test.ts diff --git a/packages/config-loader/src/lib/include.ts b/packages/config-loader/src/lib/transform/include.ts similarity index 100% rename from packages/config-loader/src/lib/include.ts rename to packages/config-loader/src/lib/transform/include.ts diff --git a/packages/config-loader/src/lib/transform/index.ts b/packages/config-loader/src/lib/transform/index.ts new file mode 100644 index 0000000000..9f1026eea6 --- /dev/null +++ b/packages/config-loader/src/lib/transform/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { applyConfigTransforms } from './apply'; +export { createIncludeTransform } from './include'; diff --git a/packages/config-loader/src/lib/types.ts b/packages/config-loader/src/lib/transform/types.ts similarity index 100% rename from packages/config-loader/src/lib/types.ts rename to packages/config-loader/src/lib/transform/types.ts diff --git a/packages/config-loader/src/lib/utils.ts b/packages/config-loader/src/lib/transform/utils.ts similarity index 100% rename from packages/config-loader/src/lib/utils.ts rename to packages/config-loader/src/lib/transform/utils.ts From f79938abe6ef4c86b696c6772a8ea6c81caef48a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 01:05:52 +0100 Subject: [PATCH 05/14] config-loader: add substitution transform --- .../config-loader/src/lib/transform/index.ts | 1 + .../src/lib/transform/substitution.test.ts | 68 +++++++++++++++++++ .../src/lib/transform/substitution.ts | 40 +++++++++++ packages/config-loader/src/loader.test.ts | 13 ++++ packages/config-loader/src/loader.ts | 2 + 5 files changed, 124 insertions(+) create mode 100644 packages/config-loader/src/lib/transform/substitution.test.ts create mode 100644 packages/config-loader/src/lib/transform/substitution.ts diff --git a/packages/config-loader/src/lib/transform/index.ts b/packages/config-loader/src/lib/transform/index.ts index 9f1026eea6..cb9f077d43 100644 --- a/packages/config-loader/src/lib/transform/index.ts +++ b/packages/config-loader/src/lib/transform/index.ts @@ -16,3 +16,4 @@ export { applyConfigTransforms } from './apply'; export { createIncludeTransform } from './include'; +export { createSubstitutionTransform } from './substitution'; diff --git a/packages/config-loader/src/lib/transform/substitution.test.ts b/packages/config-loader/src/lib/transform/substitution.test.ts new file mode 100644 index 0000000000..b6795dbfc4 --- /dev/null +++ b/packages/config-loader/src/lib/transform/substitution.test.ts @@ -0,0 +1,68 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createSubstitutionTransform } from './substitution'; + +const env = jest.fn(async (name: string) => { + return ({ + SECRET: 'my-secret', + TOKEN: 'my-token', + } as { [name: string]: string })[name]; +}); + +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]); + }); + + it('should substitute env var', async () => { + await expect(substituteTransform('hello ${SECRET}')).resolves.toEqual([ + true, + 'hello my-secret', + ]); + await expect( + substituteTransform('${SECRET } $${} ${TOKEN }'), + ).resolves.toEqual([true, 'my-secret $${} my-token']); + await expect(substituteTransform('foo ${MISSING}')).resolves.toEqual([ + true, + undefined, + ]); + await expect( + substituteTransform('foo ${MISSING} ${SECRET}'), + ).resolves.toEqual([true, undefined]); + await expect( + substituteTransform('foo ${SECRET} ${SECRET}'), + ).resolves.toEqual([true, '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 new file mode 100644 index 0000000000..d1336edd4c --- /dev/null +++ b/packages/config-loader/src/lib/transform/substitution.ts @@ -0,0 +1,40 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonValue } from '@backstage/config'; +import { TransformFunc, EnvFunc } from './types'; + +/** + * A environment variable substitution transform that transforms e.g. 'token ${MY_TOKEN}' + * to 'token abc' if MY_TOKEN is 'abc'. If any of the substituted variables are undefined, + * the entire expression ends up undefined. + */ +export function createSubstitutionTransform(env: EnvFunc): TransformFunc { + return async (input: JsonValue) => { + if (typeof input !== 'string') { + return [false, input]; + } + + const parts: (string | undefined)[] = input.split(/(? part === undefined)) { + return [true, undefined]; + } + return [true, parts.join('')]; + }; +} diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index a9857a784f..bb6262ce13 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/packages/config-loader/src/loader.test.ts @@ -19,6 +19,8 @@ import mockFs from 'mock-fs'; describe('loadConfig', () => { beforeAll(() => { + process.env.MY_SECRET = 'is-secret'; + mockFs({ '/root/app-config.yaml': ` app: @@ -29,8 +31,14 @@ describe('loadConfig', () => { '/root/app-config.development.yaml': ` app: sessionKey: development-key + backend: + $include: ./included.yaml `, '/root/secrets/session-key.txt': 'abc123', + '/root/included.yaml': ` + foo: + bar: token \${MY_SECRET} + `, }); }); @@ -104,6 +112,11 @@ describe('loadConfig', () => { app: { sessionKey: 'development-key', }, + backend: { + foo: { + bar: 'token is-secret', + }, + }, }, }, ]); diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 71553a3dd7..34437dcf75 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -22,6 +22,7 @@ import { applyConfigTransforms, readEnvConfig, createIncludeTransform, + createSubstitutionTransform, } from './lib'; export type LoadConfigOptions = { @@ -78,6 +79,7 @@ export async function loadConfig( const input = yaml.parse(await readFile(configPath)); const data = await applyConfigTransforms(input, [ createIncludeTransform(env, readFile), + createSubstitutionTransform(env), ]); configs.push({ data, context: basename(configPath) }); From c87a6bfeff0fa1c9547567cf546fcdb876012bff Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 01:07:28 +0100 Subject: [PATCH 06/14] config-loader: remove deprecated env check --- packages/config-loader/src/loader.ts | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 34437dcf75..fac004fd82 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -32,8 +32,8 @@ export type LoadConfigOptions = { // Absolute paths to load config files from. Configs from earlier paths have lower priority. configPaths: string[]; - // TODO(Rugvip): This will be removed in the future, but for now we use it to warn about possible mistakes. - env: string; + /** @deprecated This option has been removed */ + env?: string; }; export async function loadConfig( @@ -52,16 +52,6 @@ export async function loadConfig( if (await fs.pathExists(localConfig)) { configPaths.push(localConfig); } - - const envFile = `app-config.${options.env}.yaml`; - if (await fs.pathExists(resolvePath(configRoot, envFile))) { - console.error( - `Env config file '${envFile}' is not loaded as APP_ENV and NODE_ENV-based config loading has been removed`, - ); - console.error( - `To load the config file, use --config , listing every config file that you want to load`, - ); - } } const env = async (name: string) => process.env[name]; From 9f33201f5549ac53ae69f3f046cb239fa5095150 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 17:46:38 +0100 Subject: [PATCH 07/14] config-loader: add experimental env var option --- packages/config-loader/src/loader.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index fac004fd82..60a2394ccf 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -24,6 +24,7 @@ import { createIncludeTransform, createSubstitutionTransform, } from './lib'; +import { EnvFunc } from './lib/transform/types'; export type LoadConfigOptions = { // The root directory of the config loading context. Used to find default configs. @@ -34,13 +35,20 @@ export type LoadConfigOptions = { /** @deprecated This option has been removed */ env?: string; + + /** + * Custom environment variable loading function + * + * @experimental This API is not stable and may change at any point + */ + experimentalEnvFunc?: EnvFunc; }; export async function loadConfig( options: LoadConfigOptions, ): Promise { const configs = []; - const { configRoot } = options; + const { configRoot, experimentalEnvFunc: envFunc } = options; const configPaths = options.configPaths.slice(); // If no paths are provided, we default to reading @@ -54,7 +62,7 @@ export async function loadConfig( } } - const env = async (name: string) => process.env[name]; + const env = envFunc ?? (async (name: string) => process.env[name]); try { for (const configPath of configPaths) { From 651561eda857cb983850131d2a13e80a55a83968 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 17:47:41 +0100 Subject: [PATCH 08/14] cli: add --lax option to config:check which assumes that all env vars are set --- packages/cli/src/commands/config/print.ts | 1 + packages/cli/src/commands/config/validate.ts | 1 + packages/cli/src/commands/index.ts | 2 ++ packages/cli/src/lib/config.ts | 5 ++++- 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/config/print.ts b/packages/cli/src/commands/config/print.ts index 8be88adce7..56dcd3f753 100644 --- a/packages/cli/src/commands/config/print.ts +++ b/packages/cli/src/commands/config/print.ts @@ -24,6 +24,7 @@ export default async (cmd: Command) => { const { schema, appConfigs } = await loadCliConfig({ args: cmd.config, fromPackage: cmd.package, + mockEnv: cmd.lax, }); const visibility = getVisibilityOption(cmd); const data = serializeConfigData(appConfigs, schema, visibility); diff --git a/packages/cli/src/commands/config/validate.ts b/packages/cli/src/commands/config/validate.ts index 581e4bae43..37f41164af 100644 --- a/packages/cli/src/commands/config/validate.ts +++ b/packages/cli/src/commands/config/validate.ts @@ -21,5 +21,6 @@ export default async (cmd: Command) => { await loadCliConfig({ args: cmd.config, fromPackage: cmd.package, + mockEnv: cmd.lax, }); }; diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 1151e434c8..1d267c6889 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -153,6 +153,7 @@ export function registerCommands(program: CommanderStatic) { '--package ', 'Only load config schema that applies to the given package', ) + .option('--lax', 'Do not require environment variables to be set') .option('--frontend', 'Print only the frontend configuration') .option('--with-secrets', 'Include secrets in the printed configuration') .option( @@ -169,6 +170,7 @@ export function registerCommands(program: CommanderStatic) { '--package ', 'Only load config schema that applies to the given package', ) + .option('--lax', 'Do not require environment variables to be set') .option(...configOption) .description( 'Validate that the given configuration loads and matches schema', diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index 88aac1e33c..30469421d3 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -21,6 +21,7 @@ import { paths } from './paths'; type Options = { args: string[]; fromPackage?: string; + mockEnv?: boolean; }; export async function loadCliConfig(options: Options) { @@ -40,7 +41,9 @@ export async function loadCliConfig(options: Options) { }); const appConfigs = await loadConfig({ - env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'production', + experimentalEnvFunc: options.mockEnv + ? async name => process.env[name] || 'x' + : undefined, configRoot: paths.targetRoot, configPaths, }); From ef7957be48b55182e15c4a84ce0318a2d66185b9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 18:01:23 +0100 Subject: [PATCH 09/14] changesets: add changesets for all config changes --- .changeset/five-games-grin.md | 5 +++++ .changeset/happy-crabs-punch.md | 12 ++++++++++++ .changeset/lucky-guests-mate.md | 5 +++++ .changeset/tender-parrots-itch.md | 5 +++++ 4 files changed, 27 insertions(+) create mode 100644 .changeset/five-games-grin.md create mode 100644 .changeset/happy-crabs-punch.md create mode 100644 .changeset/lucky-guests-mate.md create mode 100644 .changeset/tender-parrots-itch.md diff --git a/.changeset/five-games-grin.md b/.changeset/five-games-grin.md new file mode 100644 index 0000000000..492c92880c --- /dev/null +++ b/.changeset/five-games-grin.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Add `--lax` option to `config:print` and `config:check`, which causes all environment variables to be assumed to be set. diff --git a/.changeset/happy-crabs-punch.md b/.changeset/happy-crabs-punch.md new file mode 100644 index 0000000000..74ac9d21f3 --- /dev/null +++ b/.changeset/happy-crabs-punch.md @@ -0,0 +1,12 @@ +--- +'@backstage/config-loader': patch +--- + +Added support for environment variable substitutions in string configuration values using a `${VAR}` placeholder. All environment variables much be available, or the entire expression will be evaluated to `undefined`. To escape a substitution, use `$${...}`, which will end up as `${...}`. + +For example: + +```yaml +app: + baseUrl: https://${BASE_HOST} +``` diff --git a/.changeset/lucky-guests-mate.md b/.changeset/lucky-guests-mate.md new file mode 100644 index 0000000000..fd733aac75 --- /dev/null +++ b/.changeset/lucky-guests-mate.md @@ -0,0 +1,5 @@ +--- +'@backstage/config-loader': minor +--- + +Removed support for the deprecated `$data` placeholder. diff --git a/.changeset/tender-parrots-itch.md b/.changeset/tender-parrots-itch.md new file mode 100644 index 0000000000..00757b9b22 --- /dev/null +++ b/.changeset/tender-parrots-itch.md @@ -0,0 +1,5 @@ +--- +'@backstage/config-loader': minor +--- + +Enable further processing of configuration files included using the `$include` placeholder. Meaning that for example for example `$env` includes will be processed as usual in included files. From 78a81c1be41ebf4530566160c08d8aa92ab20be5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 18:16:39 +0100 Subject: [PATCH 10/14] docs/conf: update config writing docs to talk about includes instead of secrets --- docs/conf/index.md | 4 ++-- docs/conf/writing.md | 44 +++++++++++++++++++++----------------------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/docs/conf/index.md b/docs/conf/index.md index a6f1d1f6f7..ef6faffd8b 100644 --- a/docs/conf/index.md +++ b/docs/conf/index.md @@ -18,8 +18,8 @@ allowing for customization. Configuration is stored in YAML files where the defaults are `app-config.yaml` and `app-config.local.yaml` for local overrides. Other sets of files can by loaded by passing `--config ` flags. The configuration files themselves -contain plain YAML, but with support for loading in secrets from various sources -using for example `$env` and `$file` keys. +contain plain YAML, but with support for loading in data and secrets from +various sources using for example `$env` and `$file` keys. It is also possible to supply configuration through environment variables, for example `APP_CONFIG_app_baseUrl=https://staging.example.com`. However these diff --git a/docs/conf/writing.md b/docs/conf/writing.md index 7e8402b6f6..05babd66d5 100644 --- a/docs/conf/writing.md +++ b/docs/conf/writing.md @@ -97,13 +97,13 @@ order: - If no config flags are provided, `app-config.local.yaml` has higher priority than `app-config.yaml`. -## Secrets and Dynamic Data +## Includes and Dynamic Data -Secrets are supported via special data loading keys that are prefixed with `$`, -which in turn provide a number of different ways to read in secrets. To load a -configuration value as a secret, supply an object with one of the special secret -keys, for example `$env` or `$file`. A full list of supported secret keys can be -found below. For example, the following will read the config key +Includes are supported via special data loading keys that are prefixed with `$`, +which in turn provide a number of different ways to read in data. To load in an +external configuration value, supply an object with one of the special include +keys, for example `$env` or `$file`. A full list of supported include keys can +be found below. For example, the following will read the config key `backend.mySecretKey` from the environment variable `MY_SECRET_KEY`: ```yaml @@ -114,28 +114,26 @@ backend: With the above configuration, calling `config.getString('backend.mySecretKey')` will return the value of the environment variable `MY_SECRET_KEY` when the -backend started up. All secrets are loaded at startup, so changing the contents -of secret files or environment variables will not be reflected at runtime. +backend started up. All includes are loaded at startup, so changing the contents +of files or environment variables will not be reflected at runtime. -As hinted at, secrets can be loaded from a bunch of different sources, and can -be extended with more. Below is a list of the currently supported methods for -loading secrets. +Below is a list of the currently supported methods for loading includes. -### Env Secrets +### Env Includes -This reads a secret from an environment variable. For example, the following -config loads the secret from the `MY_SECRET` env var. +This reads a string value from an environment variable. For example, the +following configuration loads the string value from the `MY_SECRET` environment +variable. ```yaml $env: MY_SECRET ``` -### File Secrets +### File Includes -This reads a secret from the entire contents of a file. The file path is -relative to the `app-config.yaml` the defines the secrets. For example, the -following reads the contents of `my-secret.txt` relative to the config file -itself: +This reads a string value from the entire contents of a text file. The file path +is relative to the source config file. For example, the following reads the +contents of `my-secret.txt` relative to the config file itself: ```yaml $file: ./my-secret.txt @@ -143,10 +141,10 @@ $file: ./my-secret.txt ### Including Files -The `$include` keyword can be used to load in JSON data from an external file. -It's able to load and parse data from `.json`, `.yml`, and `.yaml` files. It's -also possible to include a url fragment (`#`) to point to a value at the given -path in the file. +The `$include` keyword can be used to load configuration values from an external +file. It's able to load and parse data from `.json`, `.yml`, and `.yaml` files. +It's also possible to include a url fragment (`#`) to point to a value at the +given path in the file, using a dot-separated list of keys. For example, the following would read `my-secret-key` from `my-secrets.json`: From 3740a8bcb4fc2e82f47bec73ece8a80be31a667e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 18:23:29 +0100 Subject: [PATCH 11/14] docs/conf: add env var substitution docs --- docs/conf/writing.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/conf/writing.md b/docs/conf/writing.md index 05babd66d5..057675c98e 100644 --- a/docs/conf/writing.md +++ b/docs/conf/writing.md @@ -161,3 +161,19 @@ Example `my-secrets.json` file: } } ``` + +## Environment Variable Substitution + +Configuration files support environment variable substitution via a `${MY_VAR}` +syntax. For example: + +```yaml +app: + baseUrl: https://${HOST} +``` + +Note that all environment variables must be available, or the entire +configuration value will evaluate to `undefined`. + +The substitution syntax can be escaped using `$${...}`, which will be resolved +as `${...}`. From a6faeeab4b235ec2a09bdb5fe9284d2bcb3f008c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 Jan 2021 18:53:29 +0100 Subject: [PATCH 12/14] 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), ]); From b2e5c844f7aea4cf4285218dfb3f00bad880df70 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 25 Jan 2021 00:27:33 +0100 Subject: [PATCH 13/14] config-loader: more talking about includes instead of secrets --- .../config-loader/src/lib/transform/apply.ts | 3 +- .../src/lib/transform/include.test.ts | 4 ++- .../src/lib/transform/include.ts | 30 +++++++++---------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/config-loader/src/lib/transform/apply.ts b/packages/config-loader/src/lib/transform/apply.ts index 05cfbe3d27..72c440a922 100644 --- a/packages/config-loader/src/lib/transform/apply.ts +++ b/packages/config-loader/src/lib/transform/apply.ts @@ -19,8 +19,7 @@ import { TransformFunc } from './types'; import { isObject } from './utils'; /** - * Reads and parses, and validates, and transforms a single config file. - * The transformation rewrites any special values, like the $secret key. + * Applies a set of transforms to raw configuration data. */ export async function applyConfigTransforms( initialDir: string, diff --git a/packages/config-loader/src/lib/transform/include.test.ts b/packages/config-loader/src/lib/transform/include.test.ts index 4aea445ca9..564326aefc 100644 --- a/packages/config-loader/src/lib/transform/include.test.ts +++ b/packages/config-loader/src/lib/transform/include.test.ts @@ -113,7 +113,9 @@ describe('includeTransform', () => { it('should reject invalid includes', async () => { await expect( includeTransform({ $include: 'no-parser.js' }, '/'), - ).rejects.toThrow('no configuration parser available for extension .js'); + ).rejects.toThrow( + 'no configuration parser available for included file no-parser.js', + ); await expect( includeTransform({ $include: 'no-data.yml#different.key' }, '/'), ).rejects.toThrow('File not found!'); diff --git a/packages/config-loader/src/lib/transform/include.ts b/packages/config-loader/src/lib/transform/include.ts index af476bc2e1..1a6963672a 100644 --- a/packages/config-loader/src/lib/transform/include.ts +++ b/packages/config-loader/src/lib/transform/include.ts @@ -30,7 +30,7 @@ const includeFileParser: { }; /** - * Transforms a secret description into the actual secret value. + * Transforms a include description into the actual included value. */ export function createIncludeTransform( env: EnvFunc, @@ -41,40 +41,40 @@ export function createIncludeTransform( return { applied: false }; } // Check if there's any key that starts with a '$', in that case we treat - // this entire object as a secret. - const [secretKey] = Object.keys(input).filter(key => key.startsWith('$')); - if (secretKey) { + // this entire object as an include description. + const [includeKey] = Object.keys(input).filter(key => key.startsWith('$')); + if (includeKey) { if (Object.keys(input).length !== 1) { throw new Error( - `include key ${secretKey} should not have adjacent keys`, + `include key ${includeKey} should not have adjacent keys`, ); } } else { return { applied: false }; } - const secretValue = input[secretKey]; - if (typeof secretValue !== 'string') { - throw new Error(`${secretKey} include value is not a string`); + const includeValue = input[includeKey]; + if (typeof includeValue !== 'string') { + throw new Error(`${includeKey} include value is not a string`); } - switch (secretKey) { + switch (includeKey) { case '$file': try { - const value = await readFile(resolvePath(baseDir, secretValue)); + const value = await readFile(resolvePath(baseDir, includeValue)); return { applied: true, value }; } catch (error) { - throw new Error(`failed to read file ${secretValue}, ${error}`); + throw new Error(`failed to read file ${includeValue}, ${error}`); } case '$env': try { - return { applied: true, value: await env(secretValue) }; + return { applied: true, value: await env(includeValue) }; } catch (error) { - throw new Error(`failed to read env ${secretValue}, ${error}`); + throw new Error(`failed to read env ${includeValue}, ${error}`); } case '$include': { - const [filePath, dataPath] = secretValue.split(/#(.*)/); + const [filePath, dataPath] = includeValue.split(/#(.*)/); const ext = extname(filePath); const parser = includeFileParser[ext]; @@ -118,7 +118,7 @@ export function createIncludeTransform( } default: - throw new Error(`unknown secret ${secretKey}`); + throw new Error(`unknown include ${includeKey}`); } }; } From 26f41ef21d5d33b1ad9265e241d2ecec055dffe0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 25 Jan 2021 09:39:22 +0100 Subject: [PATCH 14/14] Update .changeset/happy-crabs-punch.md Co-authored-by: Adam Harvey --- .changeset/happy-crabs-punch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/happy-crabs-punch.md b/.changeset/happy-crabs-punch.md index 74ac9d21f3..4c68dbea1a 100644 --- a/.changeset/happy-crabs-punch.md +++ b/.changeset/happy-crabs-punch.md @@ -2,7 +2,7 @@ '@backstage/config-loader': patch --- -Added support for environment variable substitutions in string configuration values using a `${VAR}` placeholder. All environment variables much be available, or the entire expression will be evaluated to `undefined`. To escape a substitution, use `$${...}`, which will end up as `${...}`. +Added support for environment variable substitutions in string configuration values using a `${VAR}` placeholder. All environment variables must be available, or the entire expression will be evaluated to `undefined`. To escape a substitution, use `$${...}`, which will end up as `${...}`. For example: