config-loader: FileConfigSource tests + fixes

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

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<string>();
+
+async function tmpFiles(files: Record<string, string>) {
+  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"');
+  });
+});

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<string | undefined> {
+  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,

packages/config-loader/src/sources/__fixtures__/a.yaml

@@ -0,0 +1 @@
+a: 1

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<ConfigSourceData[][]> {
+  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',

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,