Merge pull request #1860 from spotify/rugvip/config

config,config-loader: add support for suffixed config files and multiple roots

.gitignore

@@ -119,3 +119,6 @@ dist
 
 # MkDocs build output
 site
+
+# Local configuration files
+*.local.yaml

packages/backend-common/src/config.ts

@@ -23,7 +23,8 @@ import { loadConfig } from '@backstage/config-loader';
 export async function loadBackendConfig() {
   const paths = findPaths(__dirname);
   const configs = await loadConfig({
-    rootPath: paths.targetRoot,
+    env: process.env.NODE_ENV,
+    rootPaths: [paths.targetRoot, paths.targetDir],
     shouldReadSecrets: true,
   });
   return configs;

packages/cli/src/commands/app/build.ts

@@ -21,7 +21,10 @@ import { paths } from '../../lib/paths';
 import { buildBundle } from '../../lib/bundler';
 
 export default async (cmd: Command) => {
-  const appConfigs = await loadConfig({ rootPath: paths.targetRoot });
+  const appConfigs = await loadConfig({
+    env: 'production',
+    rootPaths: [paths.targetRoot, paths.targetDir],
+  });
   await buildBundle({
     entry: 'src/index',
     statsJsonEnabled: cmd.stats,

packages/cli/src/commands/app/serve.ts

@@ -21,7 +21,10 @@ import { paths } from '../../lib/paths';
 import { serveBundle } from '../../lib/bundler';
 
 export default async (cmd: Command) => {
-  const appConfigs = await loadConfig({ rootPath: paths.targetRoot });
+  const appConfigs = await loadConfig({
+    env: 'development',
+    rootPaths: [paths.targetRoot, paths.targetDir],
+  });
   const waitForExit = await serveBundle({
     entry: 'src/index',
     checksEnabled: cmd.check,

packages/cli/src/commands/backend/dev.ts

@@ -21,7 +21,10 @@ import { paths } from '../../lib/paths';
 import { serveBackend } from '../../lib/bundler/backend';
 
 export default async (cmd: Command) => {
-  const appConfigs = await loadConfig({ rootPath: paths.targetRoot });
+  const appConfigs = await loadConfig({
+    env: 'development',
+    rootPaths: [paths.targetRoot, paths.targetDir],
+  });
   const waitForExit = await serveBackend({
     entry: 'src/index',
     checksEnabled: cmd.check,

packages/cli/src/commands/plugin/export.ts

@@ -21,7 +21,10 @@ import { paths } from '../../lib/paths';
 import { buildBundle } from '../../lib/bundler';
 
 export default async (cmd: Command) => {
-  const appConfigs = await loadConfig({ rootPath: paths.targetRoot });
+  const appConfigs = await loadConfig({
+    env: 'production',
+    rootPaths: [paths.targetRoot, paths.targetDir],
+  });
   await buildBundle({
     entry: 'dev/index',
     statsJsonEnabled: cmd.stats,

packages/cli/src/commands/plugin/serve.ts

@@ -21,7 +21,10 @@ import { paths } from '../../lib/paths';
 import { serveBundle } from '../../lib/bundler';
 
 export default async (cmd: Command) => {
-  const appConfigs = await loadConfig({ rootPath: paths.targetRoot });
+  const appConfigs = await loadConfig({
+    env: 'development',
+    rootPaths: [paths.targetRoot, paths.targetDir],
+  });
   const waitForExit = await serveBundle({
     entry: 'dev/index',
     checksEnabled: cmd.check,

packages/config-loader/src/lib/resolver.test.ts

@@ -0,0 +1,116 @@
+/*
+ * 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.
+ */
+
+const pathExists = jest.fn();
+
+jest.mock('fs-extra', () => ({ pathExists }));
+
+import { resolveStaticConfig } from './resolver';
+
+describe('resolveStaticConfig', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('should resolve no files for empty roots', async () => {
+    const resolved = await resolveStaticConfig({
+      env: 'development',
+      rootPaths: [],
+    });
+
+    expect(resolved).toEqual([]);
+    expect(pathExists).not.toHaveBeenCalled();
+  });
+
+  it('should resolve a single app-config', async () => {
+    pathExists.mockImplementation(async (path: string) =>
+      ['/repo/app-config.yaml'].includes(path),
+    );
+    const resolved = await resolveStaticConfig({
+      env: 'development',
+      rootPaths: ['/repo'],
+    });
+
+    expect(resolved).toEqual(['/repo/app-config.yaml']);
+    expect(pathExists).toHaveBeenCalledTimes(4);
+  });
+
+  it('should resolve a app-configs in different directories', async () => {
+    pathExists.mockImplementation(async (path: string) =>
+      ['/repo/app-config.yaml', '/repo/packages/a/app-config.yaml'].includes(
+        path,
+      ),
+    );
+    const resolved = await resolveStaticConfig({
+      env: 'development',
+      rootPaths: [
+        '/repo',
+        '/other-repo',
+        '/repo/packages/a',
+        '/repo/packages/b',
+      ],
+    });
+
+    expect(resolved).toEqual([
+      '/repo/app-config.yaml',
+      '/repo/packages/a/app-config.yaml',
+    ]);
+    expect(pathExists).toHaveBeenCalledTimes(16);
+  });
+
+  it('should resolve env and local configs', async () => {
+    pathExists.mockImplementation(async (path: string) =>
+      [
+        '/repo/app-config.yaml',
+        '/repo/app-config.local.yaml',
+        '/repo/app-config.production.yaml',
+        '/repo/app-config.production.local.yaml',
+        '/repo/app-config.development.local.yaml',
+        '/repo/packages/a/app-config.development.yaml',
+        '/repo/packages/a/app-config.local.yaml',
+      ].includes(path),
+    );
+    const resolved = await resolveStaticConfig({
+      env: 'development',
+      rootPaths: ['/repo', '/repo/packages/a'],
+    });
+
+    expect(resolved).toEqual([
+      '/repo/app-config.yaml',
+      '/repo/app-config.local.yaml',
+      '/repo/app-config.development.local.yaml',
+      '/repo/packages/a/app-config.local.yaml',
+      '/repo/packages/a/app-config.development.yaml',
+    ]);
+    expect(pathExists).toHaveBeenCalledTimes(8);
+  });
+
+  it('resolves suffixed configs in the correct order', async () => {
+    pathExists.mockImplementation(async () => true);
+    const resolved = await resolveStaticConfig({
+      env: 'production',
+      rootPaths: ['/repo'],
+    });
+
+    expect(resolved).toEqual([
+      '/repo/app-config.yaml',
+      '/repo/app-config.local.yaml',
+      '/repo/app-config.production.yaml',
+      '/repo/app-config.production.local.yaml',
+    ]);
+    expect(pathExists).toHaveBeenCalledTimes(4);
+  });
+});

packages/config-loader/src/lib/resolver.ts

@@ -15,21 +15,45 @@
  */
 
 import { resolve as resolvePath } from 'path';
+import { pathExists } from 'fs-extra';
 
 type ResolveOptions = {
-  // Root path for search for app-config.yaml
-  rootPath: string;
+  // Root paths to search for config files. Config from earlier paths has lower priority.
+  rootPaths: string[];
+  // The environment that we're loading config for, e.g. 'development', 'production'.
+  env: string;
 };
 
 /**
  * Resolves all configuration files that should be loaded in the given environment.
+ *
+ * For each root directory, search for the default app-config.yaml, along with suffixed
+ * NODE_ENV and local variants, e.g. app-config.production.yaml or app-config.development.local.yaml
+ *
+ * The priority order of config loaded through suffixes is `env > local > none`, meaning that
+ * for example app-config.development.yaml has higher priority than `app-config.local.yaml`.
+ *
  */
 export async function resolveStaticConfig(
   options: ResolveOptions,
 ): Promise<string[]> {
-  // TODO: We'll want this to be a bit more elaborate, probably adding configs for
-  //       specific env, and maybe local config for plugins.
-  const configPath = resolvePath(options.rootPath, 'app-config.yaml');
+  const filePaths = [
+    `app-config.yaml`,
+    `app-config.local.yaml`,
+    `app-config.${options.env}.yaml`,
+    `app-config.${options.env}.local.yaml`,
+  ];
 
-  return [configPath];
+  const resolvedPaths = [];
+
+  for (const rootPath of options.rootPaths) {
+    for (const filePath of filePaths) {
+      const path = resolvePath(rootPath, filePath);
+      if (await pathExists(path)) {
+        resolvedPaths.push(path);
+      }
+    }
+  }
+
+  return resolvedPaths;
 }

packages/config-loader/src/loader.ts

@@ -25,8 +25,11 @@ import {
 } from './lib';
 
 export type LoadConfigOptions = {
-  // Root path for search for app-config.yaml
-  rootPath: string;
+  // Root paths to search for config files. Config from earlier paths has lower priority.
+  rootPaths: string[];
+
+  // The environment that we're loading config for, e.g. 'development', 'production'.
+  env: string;
 
   // Whether to read secrets or omit them, defaults to false.
   shouldReadSecrets?: boolean;
@@ -63,8 +66,6 @@ export async function loadConfig(
 ): Promise<AppConfig[]> {
   const configs = [];
 
-  configs.push(...readEnv(process.env));
-
   const configPaths = await resolveStaticConfig(options);
 
   try {
@@ -86,5 +87,7 @@ export async function loadConfig(
     );
   }
 
+  configs.push(...readEnv(process.env));
+
   return configs;
 }

packages/config/src/reader.test.ts

@@ -214,9 +214,19 @@ describe('ConfigReader with fallback', () => {
     const config = ConfigReader.fromConfigs([
       {
         data: {
+          a: true,
+          b: true,
           c: true,
+          nested1: {
+            a: true,
+            b: true,
+          },
+          badBefore: {
+            a: true,
+          },
+          badAfter: true,
         },
-        context: 'x',
+        context: 'z',
       },
       {
         data: {
@@ -234,19 +244,9 @@ describe('ConfigReader with fallback', () => {
       },
       {
         data: {
-          a: true,
-          b: true,
           c: true,
-          nested1: {
-            a: true,
-            b: true,
-          },
-          badBefore: {
-            a: true,
-          },
-          badAfter: true,
         },
-        context: 'z',
+        context: 'x',
       },
     ]);
 
@@ -427,42 +427,42 @@ describe('ConfigReader.get()', () => {
   });
 
   it('should merge in fallback configs', () => {
-    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('a')).toEqual(
+    expect(ConfigReader.fromConfigs([configs[1], configs[0]]).get('a')).toEqual(
       {
         x: 'x1',
         y: ['y11', 'y12', 'y13'],
         z: false,
       },
     );
-    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('b')).toEqual(
+    expect(ConfigReader.fromConfigs([configs[1], configs[0]]).get('b')).toEqual(
       {
         x: 'x1',
         y: ['y11'],
         z: 'z2',
       },
     );
-    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('c')).toEqual(
+    expect(ConfigReader.fromConfigs([configs[1], configs[0]]).get('c')).toEqual(
       {
         c1: {
           c2: 'c2',
         },
       },
     );
-    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('a')).toEqual(
+    expect(ConfigReader.fromConfigs([configs[1], configs[0]]).get('a')).toEqual(
       {
         x: 'x1',
         y: ['y11', 'y12', 'y13'],
         z: false,
       },
     );
-    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('b')).toEqual(
+    expect(ConfigReader.fromConfigs([configs[1], configs[0]]).get('b')).toEqual(
       {
         x: 'x1',
         y: ['y11'],
         z: 'z2',
       },
     );
-    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('c')).toEqual(
+    expect(ConfigReader.fromConfigs([configs[1], configs[0]]).get('c')).toEqual(
       {
         c1: {
           c2: 'c2',
@@ -471,14 +471,14 @@ describe('ConfigReader.get()', () => {
     );
 
     expect(
-      ConfigReader.fromConfigs([configs[2], configs[1]]).getOptional('b'),
+      ConfigReader.fromConfigs([configs[1], configs[2]]).getOptional('b'),
     ).toEqual({
       x: 'x2',
       y: ['y21', 'y22'],
       z: 'z2',
     });
     expect(
-      ConfigReader.fromConfigs([configs[2], configs[1]]).getOptional('c'),
+      ConfigReader.fromConfigs([configs[1], configs[2]]).getOptional('c'),
     ).toEqual({
       c1: 'c1',
     });
@@ -486,23 +486,6 @@ describe('ConfigReader.get()', () => {
 
   it('should not merge non-objects', () => {
     const config = ConfigReader.fromConfigs([
-      {
-        data: {
-          a: ['1', '2'],
-          c: [],
-          d: {
-            x: 'x',
-          },
-          e: ['3'],
-          f: 'foo',
-          g: { z: 'z' },
-          h: {
-            a: 'a1',
-            c: 'c1',
-          },
-        },
-        context: '1',
-      },
       {
         data: {
           a: ['x', 'y', 'z'],
@@ -521,6 +504,23 @@ describe('ConfigReader.get()', () => {
         },
         context: '2',
       },
+      {
+        data: {
+          a: ['1', '2'],
+          c: [],
+          d: {
+            x: 'x',
+          },
+          e: ['3'],
+          f: 'foo',
+          g: { z: 'z' },
+          h: {
+            a: 'a1',
+            c: 'c1',
+          },
+        },
+        context: '1',
+      },
     ]);
     expect(config.get('a')).toEqual(['1', '2']);
     expect(config.get('b')).toEqual(['1']);

packages/config/src/reader.ts

@@ -57,14 +57,11 @@ export class ConfigReader implements Config {
       return new ConfigReader(undefined);
     }
 
-    // Merge together all configs info a single config with recursive fallback
-    // readers, giving the first config object in the array the highest priority.
-    return configs.reduceRight<ConfigReader>(
-      (previousReader, { data, context }) => {
-        return new ConfigReader(data, context, previousReader);
-      },
-      undefined!,
-    );
+    // Merge together all configs into a single config with recursive fallback
+    // readers, giving the first config object in the array the lowest priority.
+    return configs.reduce<ConfigReader>((previousReader, { data, context }) => {
+      return new ConfigReader(data, context, previousReader);
+    }, undefined!);
   }
 
   constructor(

packages/core/src/api-wrappers/createApp.test.tsx

@@ -49,9 +49,9 @@ describe('defaultConfigLoader', () => {
       '{"my":"runtime-config"}',
     );
     expect(configs).toEqual([
-      { data: { my: 'runtime-config' }, context: 'env' },
       { data: { my: 'override-config' }, context: 'a' },
       { data: { my: 'config' }, context: 'b' },
+      { data: { my: 'runtime-config' }, context: 'env' },
     ]);
   });
 

packages/core/src/api-wrappers/createApp.tsx

@@ -60,7 +60,7 @@ export const defaultConfigLoader: AppConfigLoader = async (
   if (runtimeConfigJson !== '__app_injected_runtime_config__'.toUpperCase()) {
     try {
       const data = JSON.parse(runtimeConfigJson) as JsonObject;
-      configs.unshift({ data, context: 'env' });
+      configs.push({ data, context: 'env' });
     } catch (error) {
       throw new Error(`Failed to load runtime configuration, ${error}`);
     }