From b95575173dba33912906ab836e8fc41bef9abd5e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 31 Mar 2023 17:57:27 +0200 Subject: [PATCH] config-loader: FileConfigSource tests + fixes Signed-off-by: Patrik Oldsberg --- .../src/sources/FileConfigSource.test.ts | 212 ++++++++++++++++++ .../src/sources/FileConfigSource.ts | 43 +++- .../src/sources/__fixtures__/a.yaml | 1 + .../src/sources/__testUtils__/testUtils.ts | 28 ++- .../src/sources/transform/include.ts | 7 + 5 files changed, 279 insertions(+), 12 deletions(-) create mode 100644 packages/config-loader/src/sources/FileConfigSource.test.ts create mode 100644 packages/config-loader/src/sources/__fixtures__/a.yaml diff --git a/packages/config-loader/src/sources/FileConfigSource.test.ts b/packages/config-loader/src/sources/FileConfigSource.test.ts new file mode 100644 index 0000000000..c6980006c0 --- /dev/null +++ b/packages/config-loader/src/sources/FileConfigSource.test.ts @@ -0,0 +1,212 @@ +/* + * Copyright 2023 The Backstage Authors + * + * 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 os from 'os'; +import fs from 'fs-extra'; +import { resolve as resolvePath } from 'path'; +import { FileConfigSource } from './FileConfigSource'; +import { readN } from './__testUtils__/testUtils'; + +const tmpDirs = new Array(); + +async function tmpFiles(files: Record) { + const tmpDir = await fs.mkdtemp( + resolvePath(os.tmpdir(), 'backstage-unit-test-fixture-'), + ); + tmpDirs.push(tmpDir); + + for (const [name, content] of Object.entries(files)) { + await fs.writeFile(resolvePath(tmpDir, name), content, 'utf8'); + } + + return { + resolve(...paths: string[]) { + return resolvePath(tmpDir, ...paths); + }, + write: async (name: string, content: string) => { + await fs.writeFile(resolvePath(tmpDir, name), content, 'utf8'); + }, + }; +} + +describe('FileConfigSource', () => { + afterEach(async () => { + for (const tmpDir of tmpDirs) { + await fs.remove(tmpDir); + } + }); + + it('should read a config file', async () => { + const tmp = await tmpFiles({ 'a.yaml': 'a: 1' }); + + const source = FileConfigSource.create({ path: tmp.resolve('a.yaml') }); + + await expect(readN(source, 1)).resolves.toEqual([ + [{ data: { a: 1 }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should watch config files', async () => { + const tmp = await tmpFiles({ 'a.yaml': 'a: 1' }); + + const source = FileConfigSource.create({ path: tmp.resolve('a.yaml') }); + + setTimeout(() => { + tmp.write('a.yaml', 'a: 2'); + }, 10); + + await expect(readN(source, 2)).resolves.toEqual([ + [{ data: { a: 1 }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + [{ data: { a: 2 }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should include files', async () => { + const tmp = await tmpFiles({ + 'a.yaml': 'a: { $include: x.yaml }', + 'x.yaml': '3', + }); + + const source = FileConfigSource.create({ path: tmp.resolve('a.yaml') }); + + await expect(readN(source, 1)).resolves.toEqual([ + [{ data: { a: 3 }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should include with substitution', async () => { + const tmp = await tmpFiles({ + 'a.yaml': 'a: { $include: "${MY_FILE}.yaml" } ', + 'x.yaml': '4', + }); + + const source = FileConfigSource.create({ + path: tmp.resolve('a.yaml'), + substitutionFunc: async name => (name === 'MY_FILE' ? 'x' : undefined), + }); + + await expect(readN(source, 1)).resolves.toEqual([ + [{ data: { a: 4 }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should substitute in include', async () => { + const tmp = await tmpFiles({ + 'a.yaml': 'a: { $include: x.yaml }', + 'x.yaml': '${MY_VALUE}', + }); + + const source = FileConfigSource.create({ + path: tmp.resolve('a.yaml'), + substitutionFunc: async name => (name === 'MY_VALUE' ? '5' : undefined), + }); + + await expect(readN(source, 1)).resolves.toEqual([ + [{ data: { a: '5' }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should watch included files', async () => { + const tmp = await tmpFiles({ + 'a.yaml': 'a: { $include: x.yaml }', + 'x.yaml': '${MY_VALUE}', + }); + + const source = FileConfigSource.create({ + path: tmp.resolve('a.yaml'), + substitutionFunc: async name => (name === 'MY_VALUE' ? '6' : '7'), + }); + + setTimeout(() => { + tmp.write('x.yaml', '${MY_OTHER_VALUE}'); + }, 10); + + await expect(readN(source, 2)).resolves.toEqual([ + [{ data: { a: '6' }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + [{ data: { a: '7' }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should watch referenced files', async () => { + const tmp = await tmpFiles({ + 'a.yaml': 'a: { $file: x.txt }', + 'x.txt': '8', + }); + + const source = FileConfigSource.create({ + path: tmp.resolve('a.yaml'), + }); + + setTimeout(() => { + tmp.write('x.txt', '9'); + }, 10); + + await expect(readN(source, 2)).resolves.toEqual([ + [{ data: { a: '8' }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + [{ data: { a: '9' }, context: 'a.yaml', path: tmp.resolve('a.yaml') }], + ]); + }); + + it('should ignore empty files', async () => { + const tmp = await tmpFiles({ + 'a.yaml': '', + }); + + const source = FileConfigSource.create({ + path: tmp.resolve('a.yaml'), + }); + + await expect(readN(source, 1)).resolves.toEqual([[]]); + }); + + it('should error on file', async () => { + const tmp = await tmpFiles({}); + + const source = FileConfigSource.create({ + path: tmp.resolve('not-found.yaml'), + }); + + await expect(readN(source, 1)).rejects.toThrow( + `Config file "${tmp.resolve('not-found.yaml')}" does not exist`, + ); + }); + + it('should error on missing include', async () => { + const tmp = await tmpFiles({ + 'a.yaml': 'a: { $include: not-found.yaml } ', + }); + + const source = FileConfigSource.create({ + path: tmp.resolve('a.yaml'), + }); + + await expect(readN(source, 1)).rejects.toThrow( + `Failed to read config file at "${tmp.resolve( + 'a.yaml', + )}", error at .a, failed to include "${tmp.resolve( + 'not-found.yaml', + )}", file does not exist`, + ); + }); + + it('should refuse relative paths', async () => { + expect(() => + FileConfigSource.create({ + path: 'a.yaml', + }), + ).toThrow('Config load path is not absolute: "a.yaml"'); + }); +}); diff --git a/packages/config-loader/src/sources/FileConfigSource.ts b/packages/config-loader/src/sources/FileConfigSource.ts index 7d2e14700d..947cdfabf7 100644 --- a/packages/config-loader/src/sources/FileConfigSource.ts +++ b/packages/config-loader/src/sources/FileConfigSource.ts @@ -26,6 +26,7 @@ import { ReadConfigDataOptions, } from './types'; import { createConfigTransformer } from './transform'; +import { NotFoundError } from '@backstage/errors'; /** * Options for {@link FileConfigSource.create}. @@ -44,6 +45,17 @@ export interface FileConfigSourceOptions { substitutionFunc?: SubstitutionFunc; } +async function readFile(path: string): Promise { + try { + return await fs.readFile(path, 'utf8'); + } catch (error) { + if (error.code === 'ENOENT') { + return undefined; + } + throw error; + } +} + /** * A config source that loads configuration from a local file. * @@ -62,7 +74,7 @@ export class FileConfigSource implements ConfigSource { */ static create(options: FileConfigSourceOptions): ConfigSource { if (!isAbsolute(options.path)) { - throw new Error(`Config load path is not absolute: '${options.path}'`); + throw new Error(`Config load path is not absolute: "${options.path}"`); } return new FileConfigSource(options); } @@ -93,13 +105,19 @@ export class FileConfigSource implements ConfigSource { const dir = dirname(this.#path); const transformer = createConfigTransformer({ substitutionFunc: this.#substitutionFunc, - async readFile(path) { + readFile: async path => { const fullPath = resolvePath(dir, path); // Any files discovered while reading this config should be watched too watcher.add(fullPath); watchedPaths.push(fullPath); - return fs.readFile(fullPath, 'utf8'); + const data = await readFile(fullPath); + if (data === undefined) { + throw new NotFoundError( + `failed to include "${fullPath}", file does not exist`, + ); + } + return data; }, }); @@ -111,13 +129,22 @@ export class FileConfigSource implements ConfigSource { watcher.add(this.#path); watchedPaths.push(this.#path); - const content = await fs.readFile(this.#path, 'utf8'); + const content = await readFile(this.#path); + if (content === undefined) { + throw new NotFoundError(`Config file "${this.#path}" does not exist`); + } const parsed = yaml.parse(content); if (parsed === null) { return []; } - const data = await transformer(parsed, { dir }); - return [{ data, context: configFileName, path: this.#path }]; + try { + const data = await transformer(parsed, { dir }); + return [{ data, context: configFileName, path: this.#path }]; + } catch (error) { + throw new Error( + `Failed to read config file at "${this.#path}", ${error.message}`, + ); + } }; signal?.addEventListener('abort', () => { @@ -135,10 +162,6 @@ export class FileConfigSource implements ConfigSource { } } - toString() { - return `FileConfigSource{path="${this.#path}"}`; - } - #waitForEvent( watcher: FSWatcher, signal?: AbortSignal, diff --git a/packages/config-loader/src/sources/__fixtures__/a.yaml b/packages/config-loader/src/sources/__fixtures__/a.yaml new file mode 100644 index 0000000000..a8926a52d8 --- /dev/null +++ b/packages/config-loader/src/sources/__fixtures__/a.yaml @@ -0,0 +1 @@ +a: 1 diff --git a/packages/config-loader/src/sources/__testUtils__/testUtils.ts b/packages/config-loader/src/sources/__testUtils__/testUtils.ts index 81746b32c8..79561ffb12 100644 --- a/packages/config-loader/src/sources/__testUtils__/testUtils.ts +++ b/packages/config-loader/src/sources/__testUtils__/testUtils.ts @@ -41,8 +41,8 @@ export async function readAll( const results: ConfigSourceData[][] = []; try { - for await (const { configs: data } of source.readConfigData({ signal })) { - results.push(data); + for await (const { configs } of source.readConfigData({ signal })) { + results.push(configs); } } catch (error) { throw error; @@ -51,6 +51,30 @@ export async function readAll( return results; } +export async function readN( + source: ConfigSource, + n: number, +): Promise { + const results: ConfigSourceData[][] = []; + + try { + const controller = new AbortController(); + for await (const { configs } of source.readConfigData({ + signal: controller.signal, + })) { + results.push(configs); + if (results.length >= n) { + break; + } + } + controller.abort(); + } catch (error) { + throw error; + } + + return results; +} + export function simpleSource( data: JsonObject[], context: string = 'mock-source', diff --git a/packages/config-loader/src/sources/transform/include.ts b/packages/config-loader/src/sources/transform/include.ts index 4a4122aefc..fb2b22ef46 100644 --- a/packages/config-loader/src/sources/transform/include.ts +++ b/packages/config-loader/src/sources/transform/include.ts @@ -126,6 +126,13 @@ export function createIncludeTransform( value = value[part]; } + if (typeof value === 'string') { + const substituted = await substitute(value, { dir: newDir }); + if (substituted.applied) { + value = substituted.value; + } + } + return { applied: true, value,