From ed8436717a0eea5e699765745dc71f81fc9f95bd Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 01:24:18 +0200
Subject: [PATCH 01/10] packages/config-loader: add support for transforming
 config and identifying secrets

---
 packages/config-loader/src/loader.ts | 75 ++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 5 deletions(-)

diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index c8b5bb538d..6bdc954e04 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -17,7 +17,7 @@
 import fs from 'fs-extra';
 import yaml from 'yaml';
 import { resolve as resolvePath } from 'path';
-import { AppConfig, JsonObject } from '@backstage/config';
+import { AppConfig, JsonObject, JsonValue } from '@backstage/config';
 import { findRootPath } from './paths';
 import { LoadConfigOptions } from './types';
 
@@ -77,7 +77,70 @@ export function readEnv(env: {
   return config ? [config] : [];
 }
 
-export async function readStaticConfig(
+type ReadFileFunc = (path: string) => Promise<string>;
+
+function isObject(obj: JsonValue | undefined): obj is JsonObject {
+  if (typeof obj !== 'object') {
+    return false;
+  } else if (Array.isArray(obj)) {
+    return false;
+  }
+  return obj !== null;
+}
+
+export async function readConfigFile(filePath: string, readFile: ReadFileFunc) {
+  const configYaml = await readFile(filePath);
+  const config = yaml.parse(configYaml);
+
+  async function transform(
+    obj: JsonValue,
+    path: string,
+  ): Promise<JsonValue | undefined> {
+    if (typeof obj !== 'object') {
+      return obj;
+    } else if (obj === null) {
+      return obj;
+    } else if (Array.isArray(obj)) {
+      const arr = new Array<JsonValue>();
+
+      for (const [index, value] of obj.entries()) {
+        const out = await transform(value, `${path}[${index}]`);
+        if (out !== undefined) {
+          arr.push(out);
+        }
+      }
+
+      return arr;
+    }
+
+    if ('$secret' in obj) {
+      if (!isObject(obj.$secret)) {
+        throw TypeError(`Secret expected object at secret ${path}.$secret`);
+      }
+
+      return undefined;
+    }
+
+    const out: JsonObject = {};
+
+    for (const [key, value] of Object.entries(obj)) {
+      const result = await transform(value, `${path}.${key}`);
+      if (result !== undefined) {
+        out[key] = result;
+      }
+    }
+
+    return out;
+  }
+
+  const finalConfig = await transform(config, '');
+  if (!isObject(finalConfig)) {
+    throw new TypeError('Expected object at config root');
+  }
+  return finalConfig;
+}
+
+export async function loadStaticConfig(
   options: LoadConfigOptions,
 ): Promise<AppConfig[]> {
   // TODO: We'll want this to be a bit more elaborate, probably adding configs for
@@ -91,8 +154,10 @@ export async function readStaticConfig(
   }
 
   try {
-    const configYaml = await fs.readFile(configPath, 'utf8');
-    const config = yaml.parse(configYaml);
+    const rootPath = configPath;
+    const config = await readConfigFile(configPath, (path: string) => {
+      return fs.readFile(resolvePath(rootPath, path), 'utf8');
+    });
     return [config];
   } catch (error) {
     throw new Error(`Failed to read static configuration file, ${error}`);
@@ -105,7 +170,7 @@ export async function loadConfig(
   const configs = [];
 
   configs.push(...readEnv(process.env));
-  configs.push(...(await readStaticConfig(options)));
+  configs.push(...(await loadStaticConfig(options)));
 
   return configs;
 }

From fcc514e90912922ea4248e24b1266caede12a8a0 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 15:52:27 +0200
Subject: [PATCH 02/10] packages/config-loader: added secret reading option +
 initial validation and file handler

---
 packages/config-loader/package.json  |  6 +-
 packages/config-loader/src/loader.ts | 85 +++++++++++++++++++++++++---
 packages/config-loader/src/types.ts  |  3 +
 3 files changed, 83 insertions(+), 11 deletions(-)

diff --git a/packages/config-loader/package.json b/packages/config-loader/package.json
index 7018dc40a8..de6b26363a 100644
--- a/packages/config-loader/package.json
+++ b/packages/config-loader/package.json
@@ -32,11 +32,13 @@
   "dependencies": {
     "@backstage/config": "^0.1.1-alpha.7",
     "fs-extra": "^9.0.0",
-    "yaml": "^1.9.2"
+    "yaml": "^1.9.2",
+    "yup": "^0.28.5"
   },
   "devDependencies": {
     "@types/jest": "^25.2.2",
-    "@types/node": "^12.0.0"
+    "@types/node": "^12.0.0",
+    "@types/yup": "^0.28.2"
   },
   "files": [
     "dist/**/*.{js,d.ts}"
diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index 6bdc954e04..23bb8799cf 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -15,8 +15,9 @@
  */
 
 import fs from 'fs-extra';
+import * as yup from 'yup';
 import yaml from 'yaml';
-import { resolve as resolvePath } from 'path';
+import { resolve as resolvePath, dirname } from 'path';
 import { AppConfig, JsonObject, JsonValue } from '@backstage/config';
 import { findRootPath } from './paths';
 import { LoadConfigOptions } from './types';
@@ -79,6 +80,11 @@ export function readEnv(env: {
 
 type ReadFileFunc = (path: string) => Promise<string>;
 
+type ReaderContext = {
+  shouldReadSecrets: boolean;
+  readFile: ReadFileFunc;
+};
+
 function isObject(obj: JsonValue | undefined): obj is JsonObject {
   if (typeof obj !== 'object') {
     return false;
@@ -88,8 +94,54 @@ function isObject(obj: JsonValue | undefined): obj is JsonObject {
   return obj !== null;
 }
 
-export async function readConfigFile(filePath: string, readFile: ReadFileFunc) {
-  const configYaml = await readFile(filePath);
+type FileSecret = {
+  file: string;
+};
+
+type Secret = FileSecret;
+
+const secretLoaderSchemas = {
+  file: yup.object({
+    file: yup.string().required(),
+  }),
+};
+
+const secretSchema = yup.lazy<object>(value => {
+  if (typeof value !== 'object' || value === null) {
+    return yup.object().required();
+  }
+
+  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',
+  );
+});
+
+export async function readSecret(
+  data: JsonObject,
+  readFile: ReadFileFunc,
+): Promise<string | undefined> {
+  const secret = secretSchema.validateSync(data) as Secret;
+
+  if ('file' in secret) {
+    return readFile(secret.file);
+  }
+
+  throw new Error('Secret was left unhandled');
+}
+
+export async function readConfigFile(filePath: string, context: ReaderContext) {
+  const configYaml = await context.readFile(filePath);
   const config = yaml.parse(configYaml);
 
   async function transform(
@@ -115,10 +167,18 @@ export async function readConfigFile(filePath: string, readFile: ReadFileFunc) {
 
     if ('$secret' in obj) {
       if (!isObject(obj.$secret)) {
-        throw TypeError(`Secret expected object at secret ${path}.$secret`);
+        throw TypeError(`Expected object at secret ${path}.$secret`);
       }
 
-      return undefined;
+      if (!context.shouldReadSecrets) {
+        return undefined;
+      }
+
+      try {
+        return await readSecret(obj.$secret, context.readFile);
+      } catch (error) {
+        throw new Error(`Invalid secret at ${path}: ${error.message}`);
+      }
     }
 
     const out: JsonObject = {};
@@ -143,6 +203,8 @@ export async function readConfigFile(filePath: string, readFile: ReadFileFunc) {
 export async function loadStaticConfig(
   options: LoadConfigOptions,
 ): Promise<AppConfig[]> {
+  const { shouldReadSecrets = false } = options;
+
   // TODO: We'll want this to be a bit more elaborate, probably adding configs for
   //       specific env, and maybe local config for plugins.
   let { configPath } = options;
@@ -154,13 +216,18 @@ export async function loadStaticConfig(
   }
 
   try {
-    const rootPath = configPath;
-    const config = await readConfigFile(configPath, (path: string) => {
-      return fs.readFile(resolvePath(rootPath, path), 'utf8');
+    const rootPath = dirname(configPath);
+    const config = await readConfigFile(configPath, {
+      shouldReadSecrets,
+      readFile: (path: string) => {
+        return fs.readFile(resolvePath(rootPath, path), 'utf8');
+      },
     });
     return [config];
   } catch (error) {
-    throw new Error(`Failed to read static configuration file, ${error}`);
+    throw new Error(
+      `Failed to read static configuration file: ${error.message}`,
+    );
   }
 }
 
diff --git a/packages/config-loader/src/types.ts b/packages/config-loader/src/types.ts
index d25a2d3e8b..0c27ab1854 100644
--- a/packages/config-loader/src/types.ts
+++ b/packages/config-loader/src/types.ts
@@ -17,4 +17,7 @@
 export type LoadConfigOptions = {
   // Config path, defaults to app-config.yaml in project root
   configPath?: string;
+
+  // Whether to read secrets or omit them, defaults to false.
+  shouldReadSecrets?: boolean;
 };

From 3c958523b522f08afa5c1a0821066a8b0c68cad8 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 16:09:06 +0200
Subject: [PATCH 03/10] packages/config-loader: add support for env secrets

---
 packages/config-loader/src/loader.ts | 38 ++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index 23bb8799cf..26e258a588 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -83,6 +83,7 @@ type ReadFileFunc = (path: string) => Promise<string>;
 type ReaderContext = {
   shouldReadSecrets: boolean;
   readFile: ReadFileFunc;
+  env: { [name in string]?: string };
 };
 
 function isObject(obj: JsonValue | undefined): obj is JsonObject {
@@ -98,12 +99,19 @@ type FileSecret = {
   file: string;
 };
 
-type Secret = FileSecret;
+type EnvSecret = {
+  env: string;
+};
+
+type Secret = FileSecret | EnvSecret;
 
 const secretLoaderSchemas = {
   file: yup.object({
     file: yup.string().required(),
   }),
+  env: yup.object({
+    env: yup.string().required(),
+  }),
 };
 
 const secretSchema = yup.lazy<object>(value => {
@@ -127,21 +135,34 @@ const secretSchema = yup.lazy<object>(value => {
   );
 });
 
+// A thing to make sure we've narrowed the type down to never
+function isNever<T extends never>() {
+  return void 0 as T;
+}
+
 export async function readSecret(
   data: JsonObject,
-  readFile: ReadFileFunc,
+  ctx: ReaderContext,
 ): Promise<string | undefined> {
+  if (!ctx.shouldReadSecrets) {
+    return undefined;
+  }
+
   const secret = secretSchema.validateSync(data) as Secret;
 
   if ('file' in secret) {
-    return readFile(secret.file);
+    return ctx.readFile(secret.file);
+  }
+  if ('env' in secret) {
+    return ctx.env[secret.env];
   }
 
+  isNever<typeof secret>();
   throw new Error('Secret was left unhandled');
 }
 
-export async function readConfigFile(filePath: string, context: ReaderContext) {
-  const configYaml = await context.readFile(filePath);
+export async function readConfigFile(filePath: string, ctx: ReaderContext) {
+  const configYaml = await ctx.readFile(filePath);
   const config = yaml.parse(configYaml);
 
   async function transform(
@@ -170,12 +191,8 @@ export async function readConfigFile(filePath: string, context: ReaderContext) {
         throw TypeError(`Expected object at secret ${path}.$secret`);
       }
 
-      if (!context.shouldReadSecrets) {
-        return undefined;
-      }
-
       try {
-        return await readSecret(obj.$secret, context.readFile);
+        return await readSecret(obj.$secret, ctx);
       } catch (error) {
         throw new Error(`Invalid secret at ${path}: ${error.message}`);
       }
@@ -218,6 +235,7 @@ export async function loadStaticConfig(
   try {
     const rootPath = dirname(configPath);
     const config = await readConfigFile(configPath, {
+      env: process.env,
       shouldReadSecrets,
       readFile: (path: string) => {
         return fs.readFile(resolvePath(rootPath, path), 'utf8');

From c2b59e20cbd7b60686ec2cf671bf1f1a80578676 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 16:26:15 +0200
Subject: [PATCH 04/10] packages/config-loader: added data secret loader

---
 packages/config-loader/src/loader.ts | 50 ++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index 26e258a588..ba577c4637 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -17,7 +17,7 @@
 import fs from 'fs-extra';
 import * as yup from 'yup';
 import yaml from 'yaml';
-import { resolve as resolvePath, dirname } from 'path';
+import { resolve as resolvePath, dirname, extname } from 'path';
 import { AppConfig, JsonObject, JsonValue } from '@backstage/config';
 import { findRootPath } from './paths';
 import { LoadConfigOptions } from './types';
@@ -103,7 +103,12 @@ type EnvSecret = {
   env: string;
 };
 
-type Secret = FileSecret | EnvSecret;
+type DataSecret = {
+  data: string;
+  path: string | string[];
+};
+
+type Secret = FileSecret | EnvSecret | DataSecret;
 
 const secretLoaderSchemas = {
   file: yup.object({
@@ -112,6 +117,15 @@ const secretLoaderSchemas = {
   env: yup.object({
     env: yup.string().required(),
   }),
+  data: yup.object({
+    data: yup.string().required(),
+    path: yup.lazy(value => {
+      if (typeof value === 'string') {
+        return yup.string().required();
+      }
+      return yup.array().of(yup.string().required()).required();
+    }),
+  }),
 };
 
 const secretSchema = yup.lazy<object>(value => {
@@ -135,6 +149,14 @@ const secretSchema = yup.lazy<object>(value => {
   );
 });
 
+const dataSecretParser: {
+  [ext in string]: (content: string) => Promise<JsonObject>;
+} = {
+  '.json': async content => JSON.parse(content),
+  '.yaml': async content => yaml.parse(content),
+  '.yml': async content => yaml.parse(content),
+};
+
 // A thing to make sure we've narrowed the type down to never
 function isNever<T extends never>() {
   return void 0 as T;
@@ -156,6 +178,30 @@ export async function readSecret(
   if ('env' in secret) {
     return ctx.env[secret.env];
   }
+  if ('data' in secret) {
+    const ext = extname(secret.data);
+    const parser = dataSecretParser[ext];
+    if (!parser) {
+      throw new Error(`No data secret parser available for extension ${ext}`);
+    }
+
+    const content = await ctx.readFile(secret.data);
+
+    const { path } = secret;
+    const parts = typeof path === 'string' ? path.split('.') : path;
+
+    let value: JsonValue = 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 ${secret.data}`,
+        );
+      }
+      value = value[part];
+    }
+    return String(value);
+  }
 
   isNever<typeof secret>();
   throw new Error('Secret was left unhandled');

From 3a2c353ad7c953504e50022b77254db8435a7953 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 17:27:53 +0200
Subject: [PATCH 05/10] packages/config-loader: split loader into lib

---
 packages/config-loader/src/index.ts           |   2 +-
 .../src/{loader.test.ts => lib/env.test.ts}   |   2 +-
 packages/config-loader/src/lib/env.ts         |  73 +++++
 packages/config-loader/src/lib/index.ts       |  19 ++
 .../config-loader/src/{ => lib}/paths.test.ts |   0
 packages/config-loader/src/{ => lib}/paths.ts |   0
 packages/config-loader/src/lib/reader.ts      |  77 ++++++
 packages/config-loader/src/lib/secrets.ts     | 129 +++++++++
 packages/config-loader/src/{ => lib}/types.ts |  10 +-
 packages/config-loader/src/lib/utils.ts       |  31 +++
 packages/config-loader/src/loader.ts          | 252 +-----------------
 11 files changed, 344 insertions(+), 251 deletions(-)
 rename packages/config-loader/src/{loader.test.ts => lib/env.test.ts} (98%)
 create mode 100644 packages/config-loader/src/lib/env.ts
 create mode 100644 packages/config-loader/src/lib/index.ts
 rename packages/config-loader/src/{ => lib}/paths.test.ts (100%)
 rename packages/config-loader/src/{ => lib}/paths.ts (100%)
 create mode 100644 packages/config-loader/src/lib/reader.ts
 create mode 100644 packages/config-loader/src/lib/secrets.ts
 rename packages/config-loader/src/{ => lib}/types.ts (73%)
 create mode 100644 packages/config-loader/src/lib/utils.ts

diff --git a/packages/config-loader/src/index.ts b/packages/config-loader/src/index.ts
index 822fb53d04..6738611a17 100644
--- a/packages/config-loader/src/index.ts
+++ b/packages/config-loader/src/index.ts
@@ -15,4 +15,4 @@
  */
 
 export { loadConfig } from './loader';
-export type { LoadConfigOptions } from './types';
+export type { LoadConfigOptions } from './loader';
diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/lib/env.test.ts
similarity index 98%
rename from packages/config-loader/src/loader.test.ts
rename to packages/config-loader/src/lib/env.test.ts
index 4c7a120c63..6a0a115b24 100644
--- a/packages/config-loader/src/loader.test.ts
+++ b/packages/config-loader/src/lib/env.test.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { readEnv } from './loader';
+import { readEnv } from './env';
 
 describe('readEnv', () => {
   it('should return empty config for empty env', () => {
diff --git a/packages/config-loader/src/lib/env.ts b/packages/config-loader/src/lib/env.ts
new file mode 100644
index 0000000000..e9087c4387
--- /dev/null
+++ b/packages/config-loader/src/lib/env.ts
@@ -0,0 +1,73 @@
+/*
+ * 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 { AppConfig, JsonObject } from '@backstage/config';
+
+const ENV_PREFIX = 'APP_CONFIG_';
+
+// Update the same pattern in config package if this is changed
+const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i;
+
+export function readEnv(env: {
+  [name: string]: string | undefined;
+}): AppConfig[] {
+  let config: JsonObject | undefined = undefined;
+
+  for (const [name, value] of Object.entries(env)) {
+    if (!value) {
+      continue;
+    }
+    if (name.startsWith(ENV_PREFIX)) {
+      const key = name.replace(ENV_PREFIX, '');
+      const keyParts = key.split('_');
+
+      let obj = (config = config ?? {});
+      for (const [index, part] of keyParts.entries()) {
+        if (!CONFIG_KEY_PART_PATTERN.test(part)) {
+          throw new TypeError(`Invalid env config key '${key}'`);
+        }
+        if (index < keyParts.length - 1) {
+          obj = (obj[part] = obj[part] ?? {}) as JsonObject;
+          if (typeof obj !== 'object' || Array.isArray(obj)) {
+            const subKey = keyParts.slice(0, index + 1).join('_');
+            throw new TypeError(
+              `Could not nest config for key '${key}' under existing value '${subKey}'`,
+            );
+          }
+        } else {
+          if (part in obj) {
+            throw new TypeError(
+              `Refusing to override existing config at key '${key}'`,
+            );
+          }
+          try {
+            const parsedValue = JSON.parse(value);
+            if (parsedValue === null) {
+              throw new Error('value may not be null');
+            }
+            obj[part] = parsedValue;
+          } catch (error) {
+            throw new TypeError(
+              `Failed to parse JSON-serialized config value for key '${key}', ${error}`,
+            );
+          }
+        }
+      }
+    }
+  }
+
+  return config ? [config] : [];
+}
diff --git a/packages/config-loader/src/lib/index.ts b/packages/config-loader/src/lib/index.ts
new file mode 100644
index 0000000000..9c4a2d9f91
--- /dev/null
+++ b/packages/config-loader/src/lib/index.ts
@@ -0,0 +1,19 @@
+/*
+ * 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 { findRootPath } from './paths';
+export { readConfigFile } from './reader';
+export { readEnv } from './env';
diff --git a/packages/config-loader/src/paths.test.ts b/packages/config-loader/src/lib/paths.test.ts
similarity index 100%
rename from packages/config-loader/src/paths.test.ts
rename to packages/config-loader/src/lib/paths.test.ts
diff --git a/packages/config-loader/src/paths.ts b/packages/config-loader/src/lib/paths.ts
similarity index 100%
rename from packages/config-loader/src/paths.ts
rename to packages/config-loader/src/lib/paths.ts
diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts
new file mode 100644
index 0000000000..160e45417e
--- /dev/null
+++ b/packages/config-loader/src/lib/reader.ts
@@ -0,0 +1,77 @@
+/*
+ * 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 { isObject } from './utils';
+import { readSecret } from './secrets';
+import { JsonValue, JsonObject } from '@backstage/config';
+import { ReaderContext } from './types';
+
+export async function readConfigFile(filePath: string, ctx: ReaderContext) {
+  const configYaml = await ctx.readFile(filePath);
+  const config = yaml.parse(configYaml);
+
+  async function transform(
+    obj: JsonValue,
+    path: string,
+  ): Promise<JsonValue | undefined> {
+    if (typeof obj !== 'object') {
+      return obj;
+    } else if (obj === null) {
+      return obj;
+    } else if (Array.isArray(obj)) {
+      const arr = new Array<JsonValue>();
+
+      for (const [index, value] of obj.entries()) {
+        const out = await transform(value, `${path}[${index}]`);
+        if (out !== undefined) {
+          arr.push(out);
+        }
+      }
+
+      return arr;
+    }
+
+    if ('$secret' in obj) {
+      if (!isObject(obj.$secret)) {
+        throw TypeError(`Expected object at secret ${path}.$secret`);
+      }
+
+      try {
+        return await readSecret(obj.$secret, ctx);
+      } catch (error) {
+        throw new Error(`Invalid secret at ${path}: ${error.message}`);
+      }
+    }
+
+    const out: JsonObject = {};
+
+    for (const [key, value] of Object.entries(obj)) {
+      const result = await transform(value, `${path}.${key}`);
+      if (result !== undefined) {
+        out[key] = result;
+      }
+    }
+
+    return out;
+  }
+
+  const finalConfig = await transform(config, '');
+  if (!isObject(finalConfig)) {
+    throw new TypeError('Expected object at config root');
+  }
+  return finalConfig;
+}
diff --git a/packages/config-loader/src/lib/secrets.ts b/packages/config-loader/src/lib/secrets.ts
new file mode 100644
index 0000000000..797b9d1f44
--- /dev/null
+++ b/packages/config-loader/src/lib/secrets.ts
@@ -0,0 +1,129 @@
+/*
+ * 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';
+
+type FileSecret = {
+  file: string;
+};
+
+type EnvSecret = {
+  env: string;
+};
+
+type DataSecret = {
+  data: string;
+  path: string | string[];
+};
+
+type Secret = FileSecret | EnvSecret | DataSecret;
+
+const secretLoaderSchemas = {
+  file: yup.object({
+    file: yup.string().required(),
+  }),
+  env: yup.object({
+    env: yup.string().required(),
+  }),
+  data: yup.object({
+    data: yup.string().required(),
+    path: yup.lazy(value => {
+      if (typeof value === 'string') {
+        return yup.string().required();
+      }
+      return yup.array().of(yup.string().required()).required();
+    }),
+  }),
+};
+
+const secretSchema = yup.lazy<object>(value => {
+  if (typeof value !== 'object' || value === null) {
+    return yup.object().required();
+  }
+
+  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',
+  );
+});
+
+const dataSecretParser: {
+  [ext in string]: (content: string) => Promise<JsonObject>;
+} = {
+  '.json': async content => JSON.parse(content),
+  '.yaml': async content => yaml.parse(content),
+  '.yml': async content => yaml.parse(content),
+};
+
+export async function readSecret(
+  data: JsonObject,
+  ctx: ReaderContext,
+): Promise<string | undefined> {
+  if (!ctx.shouldReadSecrets) {
+    return undefined;
+  }
+
+  const secret = secretSchema.validateSync(data) as Secret;
+
+  if ('file' in secret) {
+    return ctx.readFile(secret.file);
+  }
+  if ('env' in secret) {
+    return ctx.env[secret.env];
+  }
+  if ('data' in secret) {
+    const ext = extname(secret.data);
+    const parser = dataSecretParser[ext];
+    if (!parser) {
+      throw new Error(`No data secret parser available for extension ${ext}`);
+    }
+
+    const content = await ctx.readFile(secret.data);
+
+    const { path } = secret;
+    const parts = typeof path === 'string' ? path.split('.') : path;
+
+    let value: JsonValue = 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 ${secret.data}`,
+        );
+      }
+      value = value[part];
+    }
+    return String(value);
+  }
+
+  isNever<typeof secret>();
+  throw new Error('Secret was left unhandled');
+}
diff --git a/packages/config-loader/src/types.ts b/packages/config-loader/src/lib/types.ts
similarity index 73%
rename from packages/config-loader/src/types.ts
rename to packages/config-loader/src/lib/types.ts
index 0c27ab1854..bde324a3c1 100644
--- a/packages/config-loader/src/types.ts
+++ b/packages/config-loader/src/lib/types.ts
@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-export type LoadConfigOptions = {
-  // Config path, defaults to app-config.yaml in project root
-  configPath?: string;
+export type ReadFileFunc = (path: string) => Promise<string>;
 
-  // Whether to read secrets or omit them, defaults to false.
-  shouldReadSecrets?: boolean;
+export type ReaderContext = {
+  shouldReadSecrets: boolean;
+  readFile: ReadFileFunc;
+  env: { [name in string]?: string };
 };
diff --git a/packages/config-loader/src/lib/utils.ts b/packages/config-loader/src/lib/utils.ts
new file mode 100644
index 0000000000..37145ec971
--- /dev/null
+++ b/packages/config-loader/src/lib/utils.ts
@@ -0,0 +1,31 @@
+/*
+ * 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, JsonObject } from '@backstage/config';
+
+export function isObject(obj: JsonValue | undefined): obj is JsonObject {
+  if (typeof obj !== 'object') {
+    return false;
+  } else if (Array.isArray(obj)) {
+    return false;
+  }
+  return obj !== null;
+}
+
+// A thing to make sure we've narrowed the type down to never
+export function isNever<T extends never>() {
+  return void 0 as T;
+}
diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index ba577c4637..a8adbde32c 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -15,254 +15,18 @@
  */
 
 import fs from 'fs-extra';
-import * as yup from 'yup';
-import yaml from 'yaml';
-import { resolve as resolvePath, dirname, extname } from 'path';
-import { AppConfig, JsonObject, JsonValue } from '@backstage/config';
-import { findRootPath } from './paths';
-import { LoadConfigOptions } from './types';
+import { resolve as resolvePath, dirname } from 'path';
+import { AppConfig } from '@backstage/config';
+import { findRootPath, readConfigFile, readEnv } from './lib';
 
-const ENV_PREFIX = 'APP_CONFIG_';
+export type LoadConfigOptions = {
+  // Config path, defaults to app-config.yaml in project root
+  configPath?: string;
 
-// Update the same pattern in config package if this is changed
-const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i;
-
-export function readEnv(env: {
-  [name: string]: string | undefined;
-}): AppConfig[] {
-  let config: JsonObject | undefined = undefined;
-
-  for (const [name, value] of Object.entries(env)) {
-    if (!value) {
-      continue;
-    }
-    if (name.startsWith(ENV_PREFIX)) {
-      const key = name.replace(ENV_PREFIX, '');
-      const keyParts = key.split('_');
-
-      let obj = (config = config ?? {});
-      for (const [index, part] of keyParts.entries()) {
-        if (!CONFIG_KEY_PART_PATTERN.test(part)) {
-          throw new TypeError(`Invalid env config key '${key}'`);
-        }
-        if (index < keyParts.length - 1) {
-          obj = (obj[part] = obj[part] ?? {}) as JsonObject;
-          if (typeof obj !== 'object' || Array.isArray(obj)) {
-            const subKey = keyParts.slice(0, index + 1).join('_');
-            throw new TypeError(
-              `Could not nest config for key '${key}' under existing value '${subKey}'`,
-            );
-          }
-        } else {
-          if (part in obj) {
-            throw new TypeError(
-              `Refusing to override existing config at key '${key}'`,
-            );
-          }
-          try {
-            const parsedValue = JSON.parse(value);
-            if (parsedValue === null) {
-              throw new Error('value may not be null');
-            }
-            obj[part] = parsedValue;
-          } catch (error) {
-            throw new TypeError(
-              `Failed to parse JSON-serialized config value for key '${key}', ${error}`,
-            );
-          }
-        }
-      }
-    }
-  }
-
-  return config ? [config] : [];
-}
-
-type ReadFileFunc = (path: string) => Promise<string>;
-
-type ReaderContext = {
-  shouldReadSecrets: boolean;
-  readFile: ReadFileFunc;
-  env: { [name in string]?: string };
+  // Whether to read secrets or omit them, defaults to false.
+  shouldReadSecrets?: boolean;
 };
 
-function isObject(obj: JsonValue | undefined): obj is JsonObject {
-  if (typeof obj !== 'object') {
-    return false;
-  } else if (Array.isArray(obj)) {
-    return false;
-  }
-  return obj !== null;
-}
-
-type FileSecret = {
-  file: string;
-};
-
-type EnvSecret = {
-  env: string;
-};
-
-type DataSecret = {
-  data: string;
-  path: string | string[];
-};
-
-type Secret = FileSecret | EnvSecret | DataSecret;
-
-const secretLoaderSchemas = {
-  file: yup.object({
-    file: yup.string().required(),
-  }),
-  env: yup.object({
-    env: yup.string().required(),
-  }),
-  data: yup.object({
-    data: yup.string().required(),
-    path: yup.lazy(value => {
-      if (typeof value === 'string') {
-        return yup.string().required();
-      }
-      return yup.array().of(yup.string().required()).required();
-    }),
-  }),
-};
-
-const secretSchema = yup.lazy<object>(value => {
-  if (typeof value !== 'object' || value === null) {
-    return yup.object().required();
-  }
-
-  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',
-  );
-});
-
-const dataSecretParser: {
-  [ext in string]: (content: string) => Promise<JsonObject>;
-} = {
-  '.json': async content => JSON.parse(content),
-  '.yaml': async content => yaml.parse(content),
-  '.yml': async content => yaml.parse(content),
-};
-
-// A thing to make sure we've narrowed the type down to never
-function isNever<T extends never>() {
-  return void 0 as T;
-}
-
-export async function readSecret(
-  data: JsonObject,
-  ctx: ReaderContext,
-): Promise<string | undefined> {
-  if (!ctx.shouldReadSecrets) {
-    return undefined;
-  }
-
-  const secret = secretSchema.validateSync(data) as Secret;
-
-  if ('file' in secret) {
-    return ctx.readFile(secret.file);
-  }
-  if ('env' in secret) {
-    return ctx.env[secret.env];
-  }
-  if ('data' in secret) {
-    const ext = extname(secret.data);
-    const parser = dataSecretParser[ext];
-    if (!parser) {
-      throw new Error(`No data secret parser available for extension ${ext}`);
-    }
-
-    const content = await ctx.readFile(secret.data);
-
-    const { path } = secret;
-    const parts = typeof path === 'string' ? path.split('.') : path;
-
-    let value: JsonValue = 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 ${secret.data}`,
-        );
-      }
-      value = value[part];
-    }
-    return String(value);
-  }
-
-  isNever<typeof secret>();
-  throw new Error('Secret was left unhandled');
-}
-
-export async function readConfigFile(filePath: string, ctx: ReaderContext) {
-  const configYaml = await ctx.readFile(filePath);
-  const config = yaml.parse(configYaml);
-
-  async function transform(
-    obj: JsonValue,
-    path: string,
-  ): Promise<JsonValue | undefined> {
-    if (typeof obj !== 'object') {
-      return obj;
-    } else if (obj === null) {
-      return obj;
-    } else if (Array.isArray(obj)) {
-      const arr = new Array<JsonValue>();
-
-      for (const [index, value] of obj.entries()) {
-        const out = await transform(value, `${path}[${index}]`);
-        if (out !== undefined) {
-          arr.push(out);
-        }
-      }
-
-      return arr;
-    }
-
-    if ('$secret' in obj) {
-      if (!isObject(obj.$secret)) {
-        throw TypeError(`Expected object at secret ${path}.$secret`);
-      }
-
-      try {
-        return await readSecret(obj.$secret, ctx);
-      } catch (error) {
-        throw new Error(`Invalid secret at ${path}: ${error.message}`);
-      }
-    }
-
-    const out: JsonObject = {};
-
-    for (const [key, value] of Object.entries(obj)) {
-      const result = await transform(value, `${path}.${key}`);
-      if (result !== undefined) {
-        out[key] = result;
-      }
-    }
-
-    return out;
-  }
-
-  const finalConfig = await transform(config, '');
-  if (!isObject(finalConfig)) {
-    throw new TypeError('Expected object at config root');
-  }
-  return finalConfig;
-}
-
 export async function loadStaticConfig(
   options: LoadConfigOptions,
 ): Promise<AppConfig[]> {

From d223b28419168f0e96d38391102495eba8b99acc Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 17:36:55 +0200
Subject: [PATCH 06/10] packages/config-loader: split config file resolve logic
 into separate module

---
 packages/config-loader/src/lib/index.ts    |  2 +-
 packages/config-loader/src/lib/resolver.ts | 43 ++++++++++++++++
 packages/config-loader/src/loader.ts       | 57 +++++++++-------------
 3 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 packages/config-loader/src/lib/resolver.ts

diff --git a/packages/config-loader/src/lib/index.ts b/packages/config-loader/src/lib/index.ts
index 9c4a2d9f91..c0b6fbc93b 100644
--- a/packages/config-loader/src/lib/index.ts
+++ b/packages/config-loader/src/lib/index.ts
@@ -14,6 +14,6 @@
  * limitations under the License.
  */
 
-export { findRootPath } from './paths';
+export { resolveStaticConfig } from './resolver';
 export { readConfigFile } from './reader';
 export { readEnv } from './env';
diff --git a/packages/config-loader/src/lib/resolver.ts b/packages/config-loader/src/lib/resolver.ts
new file mode 100644
index 0000000000..d7a1a7f764
--- /dev/null
+++ b/packages/config-loader/src/lib/resolver.ts
@@ -0,0 +1,43 @@
+/*
+ * 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 fs from 'fs-extra';
+import { resolve as resolvePath } from 'path';
+import { findRootPath } from './paths';
+
+type ResolveOptions = {
+  // Same as configPath in LoadConfigOptions
+  configPath?: string;
+};
+
+/**
+ * Resolves all configuration files that should be loaded.
+ */
+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.
+  let { configPath } = options;
+  if (!configPath) {
+    configPath = resolvePath(
+      findRootPath(fs.realpathSync(process.cwd())),
+      'app-config.yaml',
+    );
+  }
+
+  return [configPath];
+}
diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index a8adbde32c..8f4ffdcecf 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -17,7 +17,7 @@
 import fs from 'fs-extra';
 import { resolve as resolvePath, dirname } from 'path';
 import { AppConfig } from '@backstage/config';
-import { findRootPath, readConfigFile, readEnv } from './lib';
+import { resolveStaticConfig, readConfigFile, readEnv } from './lib';
 
 export type LoadConfigOptions = {
   // Config path, defaults to app-config.yaml in project root
@@ -27,45 +27,34 @@ export type LoadConfigOptions = {
   shouldReadSecrets?: boolean;
 };
 
-export async function loadStaticConfig(
-  options: LoadConfigOptions,
-): Promise<AppConfig[]> {
-  const { shouldReadSecrets = false } = options;
-
-  // TODO: We'll want this to be a bit more elaborate, probably adding configs for
-  //       specific env, and maybe local config for plugins.
-  let { configPath } = options;
-  if (!configPath) {
-    configPath = resolvePath(
-      findRootPath(fs.realpathSync(process.cwd())),
-      'app-config.yaml',
-    );
-  }
-
-  try {
-    const rootPath = dirname(configPath);
-    const config = await readConfigFile(configPath, {
-      env: process.env,
-      shouldReadSecrets,
-      readFile: (path: string) => {
-        return fs.readFile(resolvePath(rootPath, path), 'utf8');
-      },
-    });
-    return [config];
-  } catch (error) {
-    throw new Error(
-      `Failed to read static configuration file: ${error.message}`,
-    );
-  }
-}
-
 export async function loadConfig(
   options: LoadConfigOptions = {},
 ): Promise<AppConfig[]> {
   const configs = [];
 
   configs.push(...readEnv(process.env));
-  configs.push(...(await loadStaticConfig(options)));
+
+  const configPaths = await resolveStaticConfig(options);
+
+  try {
+    for (const configPath of configPaths) {
+      const rootPath = dirname(configPath);
+
+      const config = await readConfigFile(configPath, {
+        env: process.env,
+        shouldReadSecrets: Boolean(options.shouldReadSecrets),
+        readFile: (path: string) => {
+          return fs.readFile(resolvePath(rootPath, path), 'utf8');
+        },
+      });
+
+      configs.push(config);
+    }
+  } catch (error) {
+    throw new Error(
+      `Failed to read static configuration file: ${error.message}`,
+    );
+  }
 
   return configs;
 }

From 6da3cc8fec2fa9f04c4645e82a35ad0ccd5182a8 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 18:13:56 +0200
Subject: [PATCH 07/10] packages/config-loader: move readSecret and secret
 reading switch to context

---
 packages/config-loader/src/lib/index.ts   |  1 +
 packages/config-loader/src/lib/reader.ts  |  3 +-
 packages/config-loader/src/lib/secrets.ts |  4 --
 packages/config-loader/src/lib/types.ts   |  7 ++-
 packages/config-loader/src/loader.ts      | 52 ++++++++++++++++++-----
 5 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/packages/config-loader/src/lib/index.ts b/packages/config-loader/src/lib/index.ts
index c0b6fbc93b..226932784b 100644
--- a/packages/config-loader/src/lib/index.ts
+++ b/packages/config-loader/src/lib/index.ts
@@ -17,3 +17,4 @@
 export { resolveStaticConfig } from './resolver';
 export { readConfigFile } from './reader';
 export { readEnv } from './env';
+export { readSecret } from './secrets';
diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts
index 160e45417e..7ff514b0e2 100644
--- a/packages/config-loader/src/lib/reader.ts
+++ b/packages/config-loader/src/lib/reader.ts
@@ -16,7 +16,6 @@
 
 import yaml from 'yaml';
 import { isObject } from './utils';
-import { readSecret } from './secrets';
 import { JsonValue, JsonObject } from '@backstage/config';
 import { ReaderContext } from './types';
 
@@ -51,7 +50,7 @@ export async function readConfigFile(filePath: string, ctx: ReaderContext) {
       }
 
       try {
-        return await readSecret(obj.$secret, ctx);
+        return await ctx.readSecret(obj.$secret);
       } catch (error) {
         throw new Error(`Invalid secret at ${path}: ${error.message}`);
       }
diff --git a/packages/config-loader/src/lib/secrets.ts b/packages/config-loader/src/lib/secrets.ts
index 797b9d1f44..44c07d9822 100644
--- a/packages/config-loader/src/lib/secrets.ts
+++ b/packages/config-loader/src/lib/secrets.ts
@@ -87,10 +87,6 @@ export async function readSecret(
   data: JsonObject,
   ctx: ReaderContext,
 ): Promise<string | undefined> {
-  if (!ctx.shouldReadSecrets) {
-    return undefined;
-  }
-
   const secret = secretSchema.validateSync(data) as Secret;
 
   if ('file' in secret) {
diff --git a/packages/config-loader/src/lib/types.ts b/packages/config-loader/src/lib/types.ts
index bde324a3c1..ba552537d0 100644
--- a/packages/config-loader/src/lib/types.ts
+++ b/packages/config-loader/src/lib/types.ts
@@ -14,10 +14,13 @@
  * limitations under the License.
  */
 
+import { JsonObject } from '@backstage/config';
+
 export type ReadFileFunc = (path: string) => Promise<string>;
+export type ReadSecretFunc = (desc: JsonObject) => Promise<string | undefined>;
 
 export type ReaderContext = {
-  shouldReadSecrets: boolean;
-  readFile: ReadFileFunc;
   env: { [name in string]?: string };
+  readFile: ReadFileFunc;
+  readSecret: ReadSecretFunc;
 };
diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts
index 8f4ffdcecf..1b6c8f34da 100644
--- a/packages/config-loader/src/loader.ts
+++ b/packages/config-loader/src/loader.ts
@@ -16,8 +16,13 @@
 
 import fs from 'fs-extra';
 import { resolve as resolvePath, dirname } from 'path';
-import { AppConfig } from '@backstage/config';
-import { resolveStaticConfig, readConfigFile, readEnv } from './lib';
+import { AppConfig, JsonObject } from '@backstage/config';
+import {
+  resolveStaticConfig,
+  readConfigFile,
+  readEnv,
+  readSecret,
+} from './lib';
 
 export type LoadConfigOptions = {
   // Config path, defaults to app-config.yaml in project root
@@ -27,6 +32,32 @@ export type LoadConfigOptions = {
   shouldReadSecrets?: boolean;
 };
 
+class Context {
+  constructor(
+    private readonly options: {
+      env: { [name in string]?: string };
+      rootPath: string;
+      shouldReadSecrets: boolean;
+    },
+  ) {}
+
+  get env() {
+    return this.options.env;
+  }
+
+  async readFile(path: string): Promise<string> {
+    return fs.readFile(resolvePath(this.options.rootPath, path), 'utf8');
+  }
+
+  async readSecret(desc: JsonObject): Promise<string | undefined> {
+    if (!this.options.shouldReadSecrets) {
+      return undefined;
+    }
+
+    return readSecret(desc, this);
+  }
+}
+
 export async function loadConfig(
   options: LoadConfigOptions = {},
 ): Promise<AppConfig[]> {
@@ -38,15 +69,14 @@ export async function loadConfig(
 
   try {
     for (const configPath of configPaths) {
-      const rootPath = dirname(configPath);
-
-      const config = await readConfigFile(configPath, {
-        env: process.env,
-        shouldReadSecrets: Boolean(options.shouldReadSecrets),
-        readFile: (path: string) => {
-          return fs.readFile(resolvePath(rootPath, path), 'utf8');
-        },
-      });
+      const config = await readConfigFile(
+        configPath,
+        new Context({
+          env: process.env,
+          rootPath: dirname(configPath),
+          shouldReadSecrets: Boolean(options.shouldReadSecrets),
+        }),
+      );
 
       configs.push(config);
     }

From fc82352988fa1e27297ce74f9aa0ae514bafc2eb Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 19:21:41 +0200
Subject: [PATCH 08/10] packages/config-loader: cover reader in tests and omit
 null values

---
 packages/config-loader/src/lib/reader.test.ts | 123 ++++++++++++++++++
 packages/config-loader/src/lib/reader.ts      |   2 +-
 2 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 packages/config-loader/src/lib/reader.test.ts

diff --git a/packages/config-loader/src/lib/reader.test.ts b/packages/config-loader/src/lib/reader.test.ts
new file mode 100644
index 0000000000..b90c53ee6b
--- /dev/null
+++ b/packages/config-loader/src/lib/reader.test.ts
@@ -0,0 +1,123 @@
+/*
+ * 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}`);
+  };
+}
+
+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', {
+      readFile,
+    } as ReaderContext);
+
+    await expect(config).resolves.toEqual({
+      app: {
+        title: 'Test',
+        x: 1,
+        y: [true],
+      },
+    });
+  });
+
+  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', {
+      readFile,
+    } as ReaderContext);
+
+    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', {
+      readFile,
+    } as ReaderContext);
+
+    await expect(config).rejects.toThrow('Expected object at config root');
+  });
+
+  it('should read 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', {
+      env: {},
+      readFile,
+      readSecret: readSecret as ReadSecretFunc,
+    });
+
+    await expect(config).resolves.toEqual({
+      app: 'secret',
+    });
+    expect(readSecret).toHaveBeenCalledWith({ file: './my-secret' });
+  });
+
+  it('should require 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', {
+      env: {},
+      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: {} }',
+    });
+    const readSecret = jest.fn().mockRejectedValue(new Error('NOPE'));
+
+    const config = readConfigFile('./app-config.yaml', {
+      env: {},
+      readFile,
+      readSecret: readSecret as ReadSecretFunc,
+    });
+
+    await expect(config).rejects.toThrow('Invalid secret at .app: NOPE');
+  });
+});
diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts
index 7ff514b0e2..01a8561f1f 100644
--- a/packages/config-loader/src/lib/reader.ts
+++ b/packages/config-loader/src/lib/reader.ts
@@ -30,7 +30,7 @@ export async function readConfigFile(filePath: string, ctx: ReaderContext) {
     if (typeof obj !== 'object') {
       return obj;
     } else if (obj === null) {
-      return obj;
+      return undefined;
     } else if (Array.isArray(obj)) {
       const arr = new Array<JsonValue>();
 

From ee33cf6751513bcd254ce7f57d4a894559f3b017 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 12 Jun 2020 21:15:20 +0200
Subject: [PATCH 09/10] packages/config-loader: cover secrets in tests and
 tweak validation

---
 .../config-loader/src/lib/secrets.test.ts     | 114 ++++++++++++++++++
 packages/config-loader/src/lib/secrets.ts     |   4 +-
 2 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 packages/config-loader/src/lib/secrets.test.ts

diff --git a/packages/config-loader/src/lib/secrets.test.ts b/packages/config-loader/src/lib/secrets.test.ts
new file mode 100644
index 0000000000..e7fe997a58
--- /dev/null
+++ b/packages/config-loader/src/lib/secrets.test.ts
@@ -0,0 +1,114 @@
+/*
+ * 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 }',
+    } 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 read data secrets', async () => {
+    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!');
+  });
+
+  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', 'data'",
+    );
+    await expect(readSecret({ unknown: 'derp' }, ctx)).rejects.toThrow(
+      "Secret must contain one of 'file', 'env', 'data'",
+    );
+    await expect(readSecret({ data: 'no-data.yml' }, ctx)).rejects.toThrow(
+      'path is a required field',
+    );
+    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),
+    ).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
index 44c07d9822..d43eaa1af5 100644
--- a/packages/config-loader/src/lib/secrets.ts
+++ b/packages/config-loader/src/lib/secrets.ts
@@ -56,7 +56,7 @@ const secretLoaderSchemas = {
 
 const secretSchema = yup.lazy<object>(value => {
   if (typeof value !== 'object' || value === null) {
-    return yup.object().required();
+    return yup.object().required().label('secret');
   }
 
   const loaderTypes = Object.keys(
@@ -87,7 +87,7 @@ export async function readSecret(
   data: JsonObject,
   ctx: ReaderContext,
 ): Promise<string | undefined> {
-  const secret = secretSchema.validateSync(data) as Secret;
+  const secret = secretSchema.validateSync(data, { strict: true }) as Secret;
 
   if ('file' in secret) {
     return ctx.readFile(secret.file);

From fbba6949864507200a30e84296afe360d442fb04 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 13 Jun 2020 13:15:22 +0200
Subject: [PATCH 10/10] packages/config-loader: more docs!

---
 packages/config-loader/src/lib/env.ts      | 18 ++++++++++++++++++
 packages/config-loader/src/lib/reader.ts   |  4 ++++
 packages/config-loader/src/lib/resolver.ts |  2 +-
 packages/config-loader/src/lib/secrets.ts  | 15 +++++++++++++++
 packages/config-loader/src/lib/types.ts    |  3 +++
 5 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/packages/config-loader/src/lib/env.ts b/packages/config-loader/src/lib/env.ts
index e9087c4387..83f86d4212 100644
--- a/packages/config-loader/src/lib/env.ts
+++ b/packages/config-loader/src/lib/env.ts
@@ -21,6 +21,24 @@ const ENV_PREFIX = 'APP_CONFIG_';
 // Update the same pattern in config package if this is changed
 const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i;
 
+/**
+ * Read runtime configuration from the environment.
+ *
+ * Only environment variables prefixed with APP_CONFIG_ will be considered.
+ *
+ * For each variable, the prefix will be removed, and rest of the key will
+ * be split by '_'. Each part will then be used as keys to build up a nested
+ * config object structure. The treatment of the entire environment variable
+ * is case-sensitive.
+ *
+ * The value of the variable should be JSON serialized, as it will be parsed
+ * and the type will be kept intact. For example "true" and true are treated
+ * differently, as well as "42" and 42.
+ *
+ * For example, to set the config app.title to "My Title", use the following:
+ *
+ * APP_CONFIG_app_title='"My Title"'
+ */
 export function readEnv(env: {
   [name: string]: string | undefined;
 }): AppConfig[] {
diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts
index 01a8561f1f..044bf68288 100644
--- a/packages/config-loader/src/lib/reader.ts
+++ b/packages/config-loader/src/lib/reader.ts
@@ -19,6 +19,10 @@ import { isObject } from './utils';
 import { JsonValue, JsonObject } from '@backstage/config';
 import { ReaderContext } from './types';
 
+/**
+ * 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) {
   const configYaml = await ctx.readFile(filePath);
   const config = yaml.parse(configYaml);
diff --git a/packages/config-loader/src/lib/resolver.ts b/packages/config-loader/src/lib/resolver.ts
index d7a1a7f764..4a06f3ebca 100644
--- a/packages/config-loader/src/lib/resolver.ts
+++ b/packages/config-loader/src/lib/resolver.ts
@@ -24,7 +24,7 @@ type ResolveOptions = {
 };
 
 /**
- * Resolves all configuration files that should be loaded.
+ * Resolves all configuration files that should be loaded in the given environment.
  */
 export async function resolveStaticConfig(
   options: ResolveOptions,
diff --git a/packages/config-loader/src/lib/secrets.ts b/packages/config-loader/src/lib/secrets.ts
index d43eaa1af5..c4157120ae 100644
--- a/packages/config-loader/src/lib/secrets.ts
+++ b/packages/config-loader/src/lib/secrets.ts
@@ -21,21 +21,31 @@ 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;
 };
 
+// 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.
+  // Either a '.' separated list, or an array of path segments.
   path: string | string[];
 };
 
 type Secret = FileSecret | EnvSecret | DataSecret;
 
+// Schema for each type of secret description
 const secretLoaderSchemas = {
   file: yup.object({
     file: yup.string().required(),
@@ -54,6 +64,7 @@ const secretLoaderSchemas = {
   }),
 };
 
+// The top-level secret schema, which figures out what type of secret it is.
 const secretSchema = yup.lazy<object>(value => {
   if (typeof value !== 'object' || value === null) {
     return yup.object().required().label('secret');
@@ -75,6 +86,7 @@ const secretSchema = yup.lazy<object>(value => {
   );
 });
 
+// Parsers for each type of data secret file.
 const dataSecretParser: {
   [ext in string]: (content: string) => Promise<JsonObject>;
 } = {
@@ -83,6 +95,9 @@ const dataSecretParser: {
   '.yml': async content => yaml.parse(content),
 };
 
+/**
+ * Transforms a secret description into the actual secret value.
+ */
 export async function readSecret(
   data: JsonObject,
   ctx: ReaderContext,
diff --git a/packages/config-loader/src/lib/types.ts b/packages/config-loader/src/lib/types.ts
index ba552537d0..c0f8b99dcf 100644
--- a/packages/config-loader/src/lib/types.ts
+++ b/packages/config-loader/src/lib/types.ts
@@ -19,6 +19,9 @@ import { JsonObject } from '@backstage/config';
 export type ReadFileFunc = (path: string) => Promise<string>;
 export type ReadSecretFunc = (desc: JsonObject) => Promise<string | undefined>;
 
+/**
+ * Common context that provides all the necessary hooks for reading configuration files.
+ */
 export type ReaderContext = {
   env: { [name in string]?: string };
   readFile: ReadFileFunc;