From 9793bebce4ac675e9cf377723d951baf58328a3c Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 16:58:09 +0200
Subject: [PATCH 01/28] packages/core-api: re-use Config type from config
 package

---
 .../core-api/src/apis/definitions/ConfigApi.ts    | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/packages/core-api/src/apis/definitions/ConfigApi.ts b/packages/core-api/src/apis/definitions/ConfigApi.ts
index 20676df899..63a588fc10 100644
--- a/packages/core-api/src/apis/definitions/ConfigApi.ts
+++ b/packages/core-api/src/apis/definitions/ConfigApi.ts
@@ -14,20 +14,7 @@
  * limitations under the License.
  */
 import { createApiRef } from '../ApiRef';
-
-export type Config = {
-  getConfig(key: string): Config;
-
-  getConfigArray(key: string): Config[];
-
-  getNumber(key: string): number | undefined;
-
-  getBoolean(key: string): boolean | undefined;
-
-  getString(key: string): string | undefined;
-
-  getStringArray(key: string): string[] | undefined;
-};
+import { Config } from '@backstage/config';
 
 // Using interface to make the ConfigApi name show up in docs
 export interface ConfigApi extends Config {}

From 31585800a4f320a76731f92259b3112e9518be1f Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 17:15:31 +0200
Subject: [PATCH 02/28] packages/config: added must* variant for reading
 required primitive values

---
 packages/config/src/reader.test.ts | 25 +++++++++++++++++++++++
 packages/config/src/reader.ts      | 32 ++++++++++++++++++++++++++++++
 packages/config/src/types.ts       |  4 ++++
 3 files changed, 61 insertions(+)

diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index 3d3287dfe7..e33939bb4b 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -49,6 +49,10 @@ function expectValidValues(config: ConfigReader) {
     'string1',
     'string2',
   ]);
+  expect(config.mustNumber('zero')).toBe(0);
+  expect(config.mustBoolean('true')).toBe(true);
+  expect(config.mustString('string')).toBe('string');
+  expect(config.mustStringArray('strings')).toEqual(['string1', 'string2']);
 
   const [config1, config2, config3] = config.getConfigArray('nestlings');
   expect(config1.getBoolean('boolean')).toBe(true);
@@ -57,6 +61,9 @@ function expectValidValues(config: ConfigReader) {
 }
 
 function expectInvalidValues(config: ConfigReader) {
+  expect(() => config.getBoolean('string')).toThrow(
+    'Invalid type in config for key string, got string, wanted boolean',
+  );
   expect(() => config.getNumber('string')).toThrow(
     'Invalid type in config for key string, got string, wanted number',
   );
@@ -87,6 +94,18 @@ function expectInvalidValues(config: ConfigReader) {
   expect(() => config.getConfigArray('one')).toThrow(
     'Invalid type in config for key one, got number, wanted object-array',
   );
+  expect(() => config.mustBoolean('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
+  expect(() => config.mustNumber('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
+  expect(() => config.mustString('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
+  expect(() => config.mustStringArray('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
 }
 
 describe('ConfigReader', () => {
@@ -210,6 +229,12 @@ describe('ConfigReader with fallback', () => {
     // Config arrays aren't merged either
     expect(config.getConfigArray('merged.configs').length).toBe(1);
     expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a');
+    expect(config.getConfigArray('merged.configs')[0].mustString('a')).toBe(
+      'a',
+    );
+    expect(() =>
+      config.getConfigArray('merged.configs')[0].mustString('missing'),
+    ).toThrow("Missing required config value at 'missing'");
     expect(
       config.getConfigArray('merged.configs')[0].getString('b'),
     ).toBeUndefined();
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index 95c4f47383..7878a392ff 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -92,6 +92,14 @@ export class ConfigReader implements Config {
     return (configs ?? []).map(obj => new ConfigReader(obj));
   }
 
+  mustNumber(key: string): number {
+    const value = this.getNumber(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
   getNumber(key: string): number | undefined {
     return this.readConfigValue(
       key,
@@ -99,6 +107,14 @@ export class ConfigReader implements Config {
     );
   }
 
+  mustBoolean(key: string): boolean {
+    const value = this.getBoolean(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
   getBoolean(key: string): boolean | undefined {
     return this.readConfigValue(
       key,
@@ -106,6 +122,14 @@ export class ConfigReader implements Config {
     );
   }
 
+  mustString(key: string): string {
+    const value = this.getString(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
   getString(key: string): string | undefined {
     return this.readConfigValue(
       key,
@@ -114,6 +138,14 @@ export class ConfigReader implements Config {
     );
   }
 
+  mustStringArray(key: string): string[] {
+    const value = this.getStringArray(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
   getStringArray(key: string): string[] | undefined {
     return this.readConfigValue(key, values => {
       if (!Array.isArray(values)) {
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index 9eda6c8b32..73b8e42130 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -32,10 +32,14 @@ export type Config = {
   getConfigArray(key: string): Config[];
 
   getNumber(key: string): number | undefined;
+  mustNumber(key: string): number;
 
   getBoolean(key: string): boolean | undefined;
+  mustBoolean(key: string): boolean;
 
   getString(key: string): string | undefined;
+  mustString(key: string): string;
 
   getStringArray(key: string): string[] | undefined;
+  mustStringArray(key: string): string[];
 };

From 3dcef70e3c100c059afe1de4c774c32564b8f938 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 20:10:44 +0200
Subject: [PATCH 03/28] packages/config: flip around must* and get* to get* and
 getOptional*

---
 packages/cli/src/lib/bundler/config.ts        |  6 ---
 packages/config/src/reader.test.ts            | 45 ++++++++++---------
 packages/config/src/reader.ts                 | 24 +++++-----
 packages/config/src/types.ts                  | 16 +++----
 packages/core-api/src/app/App.tsx             |  2 +-
 .../core/src/layout/SignInPage/SignInPage.tsx |  2 +-
 .../components/WelcomePage/WelcomePage.tsx    |  3 +-
 7 files changed, 47 insertions(+), 51 deletions(-)

diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts
index 50d174368a..d5fb674ec1 100644
--- a/packages/cli/src/lib/bundler/config.ts
+++ b/packages/cli/src/lib/bundler/config.ts
@@ -33,9 +33,6 @@ import { BundlingOptions, BackendBundlingOptions } from './types';
 
 export function resolveBaseUrl(config: Config): URL {
   const baseUrl = config.getString('app.baseUrl');
-  if (!baseUrl) {
-    throw new Error('app.baseUrl must be set in config');
-  }
   try {
     return new URL(baseUrl, 'http://localhost:3000');
   } catch (error) {
@@ -52,9 +49,6 @@ export function createConfig(
   const { plugins, loaders } = transforms(options);
 
   const baseUrl = options.config.getString('app.baseUrl');
-  if (!baseUrl) {
-    throw new Error('app.baseUrl must be set in config');
-  }
   const validBaseUrl = new URL(baseUrl, 'https://backstage-app.dev');
 
   if (checksEnabled) {
diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index e33939bb4b..f7804d5d49 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -49,10 +49,10 @@ function expectValidValues(config: ConfigReader) {
     'string1',
     'string2',
   ]);
-  expect(config.mustNumber('zero')).toBe(0);
-  expect(config.mustBoolean('true')).toBe(true);
-  expect(config.mustString('string')).toBe('string');
-  expect(config.mustStringArray('strings')).toEqual(['string1', 'string2']);
+  expect(config.getNumber('zero')).toBe(0);
+  expect(config.getBoolean('true')).toBe(true);
+  expect(config.getString('string')).toBe('string');
+  expect(config.getStringArray('strings')).toEqual(['string1', 'string2']);
 
   const [config1, config2, config3] = config.getConfigArray('nestlings');
   expect(config1.getBoolean('boolean')).toBe(true);
@@ -94,16 +94,16 @@ function expectInvalidValues(config: ConfigReader) {
   expect(() => config.getConfigArray('one')).toThrow(
     'Invalid type in config for key one, got number, wanted object-array',
   );
-  expect(() => config.mustBoolean('missing')).toThrow(
+  expect(() => config.getBoolean('missing')).toThrow(
     "Missing required config value at 'missing'",
   );
-  expect(() => config.mustNumber('missing')).toThrow(
+  expect(() => config.getNumber('missing')).toThrow(
     "Missing required config value at 'missing'",
   );
-  expect(() => config.mustString('missing')).toThrow(
+  expect(() => config.getString('missing')).toThrow(
     "Missing required config value at 'missing'",
   );
-  expect(() => config.mustStringArray('missing')).toThrow(
+  expect(() => config.getStringArray('missing')).toThrow(
     "Missing required config value at 'missing'",
   );
 }
@@ -111,13 +111,13 @@ function expectInvalidValues(config: ConfigReader) {
 describe('ConfigReader', () => {
   it('should read empty config with valid keys', () => {
     const config = new ConfigReader({});
-    expect(config.getString('x')).toBeUndefined();
-    expect(config.getString('x_x')).toBeUndefined();
-    expect(config.getString('x-X')).toBeUndefined();
-    expect(config.getString('x0')).toBeUndefined();
-    expect(config.getString('X-x2')).toBeUndefined();
-    expect(config.getString('x0_x0')).toBeUndefined();
-    expect(config.getString('x_x-x_x')).toBeUndefined();
+    expect(config.getOptionalString('x')).toBeUndefined();
+    expect(config.getOptionalString('x_x')).toBeUndefined();
+    expect(config.getOptionalString('x-X')).toBeUndefined();
+    expect(config.getOptionalString('x0')).toBeUndefined();
+    expect(config.getOptionalString('X-x2')).toBeUndefined();
+    expect(config.getOptionalString('x0_x0')).toBeUndefined();
+    expect(config.getOptionalString('x_x-x_x')).toBeUndefined();
   });
 
   it('should throw on invalid keys', () => {
@@ -154,7 +154,7 @@ describe('ConfigReader', () => {
 describe('ConfigReader with fallback', () => {
   it('should behave as if without fallback', () => {
     const config = new ConfigReader({}, new ConfigReader(DATA));
-    expect(config.getString('x')).toBeUndefined();
+    expect(config.getOptionalString('x')).toBeUndefined();
     expect(() => config.getString('.')).toThrow(/^Invalid config key/);
     expect(() => config.getString('a.')).toThrow(/^Invalid config key/);
   });
@@ -229,14 +229,12 @@ describe('ConfigReader with fallback', () => {
     // Config arrays aren't merged either
     expect(config.getConfigArray('merged.configs').length).toBe(1);
     expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a');
-    expect(config.getConfigArray('merged.configs')[0].mustString('a')).toBe(
-      'a',
-    );
+    expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a');
     expect(() =>
-      config.getConfigArray('merged.configs')[0].mustString('missing'),
+      config.getConfigArray('merged.configs')[0].getString('missing'),
     ).toThrow("Missing required config value at 'missing'");
     expect(
-      config.getConfigArray('merged.configs')[0].getString('b'),
+      config.getConfigArray('merged.configs')[0].getOptionalString('b'),
     ).toBeUndefined();
 
     // Config arrays aren't merged either
@@ -245,7 +243,10 @@ describe('ConfigReader with fallback', () => {
       config.getConfig('merged').getConfigArray('configs')[0].getString('a'),
     ).toBe('a');
     expect(
-      config.getConfig('merged').getConfigArray('configs')[0].getString('b'),
+      config
+        .getConfig('merged')
+        .getConfigArray('configs')[0]
+        .getOptionalString('b'),
     ).toBeUndefined();
   });
 });
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index 7878a392ff..ff7c8be6e5 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -92,45 +92,45 @@ export class ConfigReader implements Config {
     return (configs ?? []).map(obj => new ConfigReader(obj));
   }
 
-  mustNumber(key: string): number {
-    const value = this.getNumber(key);
+  getNumber(key: string): number {
+    const value = this.getOptionalNumber(key);
     if (value === undefined) {
       throw new Error(`Missing required config value at '${key}'`);
     }
     return value;
   }
 
-  getNumber(key: string): number | undefined {
+  getOptionalNumber(key: string): number | undefined {
     return this.readConfigValue(
       key,
       value => typeof value === 'number' || { expected: 'number' },
     );
   }
 
-  mustBoolean(key: string): boolean {
-    const value = this.getBoolean(key);
+  getBoolean(key: string): boolean {
+    const value = this.getOptionalBoolean(key);
     if (value === undefined) {
       throw new Error(`Missing required config value at '${key}'`);
     }
     return value;
   }
 
-  getBoolean(key: string): boolean | undefined {
+  getOptionalBoolean(key: string): boolean | undefined {
     return this.readConfigValue(
       key,
       value => typeof value === 'boolean' || { expected: 'boolean' },
     );
   }
 
-  mustString(key: string): string {
-    const value = this.getString(key);
+  getString(key: string): string {
+    const value = this.getOptionalString(key);
     if (value === undefined) {
       throw new Error(`Missing required config value at '${key}'`);
     }
     return value;
   }
 
-  getString(key: string): string | undefined {
+  getOptionalString(key: string): string | undefined {
     return this.readConfigValue(
       key,
       value =>
@@ -138,15 +138,15 @@ export class ConfigReader implements Config {
     );
   }
 
-  mustStringArray(key: string): string[] {
-    const value = this.getStringArray(key);
+  getStringArray(key: string): string[] {
+    const value = this.getOptionalStringArray(key);
     if (value === undefined) {
       throw new Error(`Missing required config value at '${key}'`);
     }
     return value;
   }
 
-  getStringArray(key: string): string[] | undefined {
+  getOptionalStringArray(key: string): string[] | undefined {
     return this.readConfigValue(key, values => {
       if (!Array.isArray(values)) {
         return { expected: 'string-array' };
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index 73b8e42130..03c9dc4383 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -31,15 +31,15 @@ export type Config = {
 
   getConfigArray(key: string): Config[];
 
-  getNumber(key: string): number | undefined;
-  mustNumber(key: string): number;
+  getNumber(key: string): number;
+  getOptionalNumber(key: string): number | undefined;
 
-  getBoolean(key: string): boolean | undefined;
-  mustBoolean(key: string): boolean;
+  getBoolean(key: string): boolean;
+  getOptionalBoolean(key: string): boolean | undefined;
 
-  getString(key: string): string | undefined;
-  mustString(key: string): string;
+  getString(key: string): string;
+  getOptionalString(key: string): string | undefined;
 
-  getStringArray(key: string): string[] | undefined;
-  mustStringArray(key: string): string[];
+  getStringArray(key: string): string[];
+  getOptionalStringArray(key: string): string[] | undefined;
 };
diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx
index 163a546943..12f36a6e8b 100644
--- a/packages/core-api/src/app/App.tsx
+++ b/packages/core-api/src/app/App.tsx
@@ -282,7 +282,7 @@ export class PrivateAppImpl implements BackstageApp {
       const configApi = useApi(configApiRef);
 
       let { pathname } = new URL(
-        configApi.getString('app.baseUrl') ?? '/',
+        configApi.getOptionalString('app.baseUrl') ?? '/',
         'http://dummy.dev', // baseUrl can be specified as just a path
       );
       if (pathname.endsWith('/')) {
diff --git a/packages/core/src/layout/SignInPage/SignInPage.tsx b/packages/core/src/layout/SignInPage/SignInPage.tsx
index 91bc251f90..5911fe86b2 100644
--- a/packages/core/src/layout/SignInPage/SignInPage.tsx
+++ b/packages/core/src/layout/SignInPage/SignInPage.tsx
@@ -39,7 +39,7 @@ export const SignInPage: FC<Props> = ({ onResult, providers }) => {
 
   return (
     <Page>
-      <Header title={configApi.getString('app.title') ?? 'Backstage'} />
+      <Header title={configApi.getString('app.title')} />
       <Content>
         <ContentHeader title="Select a sign-in method" />
         <Grid container>{providerElements}</Grid>
diff --git a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx
index 6a1b0e0008..ce151737b3 100644
--- a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx
+++ b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx
@@ -39,7 +39,8 @@ import {
 } from '@backstage/core';
 
 const WelcomePage: FC<{}> = () => {
-  const appTitle = useApi(configApiRef).getString('app.title') ?? 'Backstage';
+  const appTitle =
+    useApi(configApiRef).getOptionalString('app.title') ?? 'Backstage';
   const profile = { givenName: '' };
 
   return (

From c9ee24b8bb296a6df075a3f56d7de03a739d1aca Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 17:28:26 +0200
Subject: [PATCH 04/28] packages/config: allow config readers to be backed by
 undefined data

---
 packages/config/src/reader.test.ts |  6 ++++++
 packages/config/src/reader.ts      | 19 ++++++++++++-------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index f7804d5d49..2e61a34512 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -118,6 +118,8 @@ describe('ConfigReader', () => {
     expect(config.getOptionalString('X-x2')).toBeUndefined();
     expect(config.getOptionalString('x0_x0')).toBeUndefined();
     expect(config.getOptionalString('x_x-x_x')).toBeUndefined();
+
+    expect(new ConfigReader(undefined).getOptionalString('x')).toBeUndefined();
   });
 
   it('should throw on invalid keys', () => {
@@ -138,6 +140,10 @@ describe('ConfigReader', () => {
     expect(() => config.getString('a.a.a.a.')).toThrow(/^Invalid config key/);
     expect(() => config.getString('a._')).toThrow(/^Invalid config key/);
     expect(() => config.getString('a.-.a')).toThrow(/^Invalid config key/);
+
+    expect(() => new ConfigReader(undefined).getString('.')).toThrow(
+      /^Invalid config key/,
+    );
   });
 
   it('should read valid values', () => {
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index ff7c8be6e5..f1ed8a4f3d 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -40,11 +40,9 @@ function typeOf(value: JsonValue | undefined): string {
 }
 
 export class ConfigReader implements Config {
-  private static readonly nullReader = new ConfigReader({});
-
   static fromConfigs(configs: AppConfig[]): ConfigReader {
     if (configs.length === 0) {
-      return new ConfigReader({});
+      return new ConfigReader(undefined);
     }
 
     // Merge together all configs info a single config with recursive fallback
@@ -55,13 +53,14 @@ export class ConfigReader implements Config {
   }
 
   constructor(
-    private readonly data: JsonObject,
+    private readonly data: JsonObject | undefined,
     private readonly fallback?: ConfigReader,
   ) {}
 
   getConfig(key: string): ConfigReader {
     const value = this.readValue(key);
     const fallbackConfig = this.fallback?.getConfig(key);
+
     if (isObject(value)) {
       return new ConfigReader(value, fallbackConfig);
     }
@@ -72,7 +71,7 @@ export class ConfigReader implements Config {
         )}, wanted object`,
       );
     }
-    return fallbackConfig ?? ConfigReader.nullReader;
+    return fallbackConfig ?? new ConfigReader(undefined, undefined);
   }
 
   getConfigArray(key: string): ConfigReader[] {
@@ -191,12 +190,18 @@ export class ConfigReader implements Config {
 
   private readValue(key: string): JsonValue | undefined {
     const parts = key.split('.');
-
-    let value: JsonValue | undefined = this.data;
     for (const part of parts) {
       if (!CONFIG_KEY_PART_PATTERN.test(part)) {
         throw new TypeError(`Invalid config key '${key}'`);
       }
+    }
+
+    if (this.data === undefined) {
+      return undefined;
+    }
+
+    let value: JsonValue | undefined = this.data;
+    for (const part of parts) {
       if (isObject(value)) {
         value = value[part];
       } else {

From 5294d71fe914d5f7f70dd9ab9281c5a44bb3ec21 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 17:35:16 +0200
Subject: [PATCH 05/28] packages/config: optimize some error message handling

---
 packages/config/src/reader.ts | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index f1ed8a4f3d..2c2b560536 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -39,6 +39,16 @@ function typeOf(value: JsonValue | undefined): string {
   return type;
 }
 
+// Separate out a couple of common error messages to reduce bundle size.
+const errors = {
+  type(key: string, typeName: string, expected: string) {
+    return `Invalid type in config for key ${key}, got ${typeName}, wanted ${expected}`;
+  },
+  missing(key: string) {
+    return `Missing required config value at '${key}'`;
+  },
+};
+
 export class ConfigReader implements Config {
   static fromConfigs(configs: AppConfig[]): ConfigReader {
     if (configs.length === 0) {
@@ -65,11 +75,7 @@ export class ConfigReader implements Config {
       return new ConfigReader(value, fallbackConfig);
     }
     if (value !== undefined) {
-      throw new TypeError(
-        `Invalid type in config for key ${key}, got ${typeOf(
-          value,
-        )}, wanted object`,
-      );
+      throw new TypeError(errors.type(key, typeOf(value), 'object'));
     }
     return fallbackConfig ?? new ConfigReader(undefined, undefined);
   }
@@ -94,7 +100,7 @@ export class ConfigReader implements Config {
   getNumber(key: string): number {
     const value = this.getOptionalNumber(key);
     if (value === undefined) {
-      throw new Error(`Missing required config value at '${key}'`);
+      throw new Error(errors.missing(key));
     }
     return value;
   }
@@ -109,7 +115,7 @@ export class ConfigReader implements Config {
   getBoolean(key: string): boolean {
     const value = this.getOptionalBoolean(key);
     if (value === undefined) {
-      throw new Error(`Missing required config value at '${key}'`);
+      throw new Error(errors.missing(key));
     }
     return value;
   }
@@ -124,7 +130,7 @@ export class ConfigReader implements Config {
   getString(key: string): string {
     const value = this.getOptionalString(key);
     if (value === undefined) {
-      throw new Error(`Missing required config value at '${key}'`);
+      throw new Error(errors.missing(key));
     }
     return value;
   }
@@ -140,7 +146,7 @@ export class ConfigReader implements Config {
   getStringArray(key: string): string[] {
     const value = this.getOptionalStringArray(key);
     if (value === undefined) {
-      throw new Error(`Missing required config value at '${key}'`);
+      throw new Error(errors.missing(key));
     }
     return value;
   }
@@ -178,10 +184,7 @@ export class ConfigReader implements Config {
           value: theValue = value,
           expected,
         } = result;
-        const typeName = typeOf(theValue);
-        throw new TypeError(
-          `Invalid type in config for key ${keyName}, got ${typeName}, wanted ${expected}`,
-        );
+        throw new TypeError(errors.type(keyName, typeOf(theValue), expected));
       }
     }
 

From 625a50989d253eeb9417efac6082d59482281560 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 17:54:11 +0200
Subject: [PATCH 06/28] packages/config: keep track of key prefix to display
 better error messages

---
 packages/config/src/reader.test.ts |  5 ++++-
 packages/config/src/reader.ts      | 31 +++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index 2e61a34512..f23fad8d3b 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -231,6 +231,9 @@ describe('ConfigReader with fallback', () => {
       'a',
       'b',
     ]);
+    expect(() => config.getConfig('merged').getStringArray('x')).toThrow(
+      'Invalid type in config for key merged.x, got string, wanted string-array',
+    );
 
     // Config arrays aren't merged either
     expect(config.getConfigArray('merged.configs').length).toBe(1);
@@ -238,7 +241,7 @@ describe('ConfigReader with fallback', () => {
     expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a');
     expect(() =>
       config.getConfigArray('merged.configs')[0].getString('missing'),
-    ).toThrow("Missing required config value at 'missing'");
+    ).toThrow("Missing required config value at 'merged.configs[0].missing'");
     expect(
       config.getConfigArray('merged.configs')[0].getOptionalString('b'),
     ).toBeUndefined();
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index 2c2b560536..e3c349768b 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -65,19 +65,23 @@ export class ConfigReader implements Config {
   constructor(
     private readonly data: JsonObject | undefined,
     private readonly fallback?: ConfigReader,
+    private readonly prefix: string = '',
   ) {}
 
   getConfig(key: string): ConfigReader {
     const value = this.readValue(key);
     const fallbackConfig = this.fallback?.getConfig(key);
+    const prefix = this.fullKey(key);
 
     if (isObject(value)) {
-      return new ConfigReader(value, fallbackConfig);
+      return new ConfigReader(value, fallbackConfig, prefix);
     }
     if (value !== undefined) {
-      throw new TypeError(errors.type(key, typeOf(value), 'object'));
+      throw new TypeError(
+        errors.type(this.fullKey(key), typeOf(value), 'object'),
+      );
     }
-    return fallbackConfig ?? new ConfigReader(undefined, undefined);
+    return fallbackConfig ?? new ConfigReader(undefined, undefined, prefix);
   }
 
   getConfigArray(key: string): ConfigReader[] {
@@ -94,13 +98,16 @@ export class ConfigReader implements Config {
       return true;
     });
 
-    return (configs ?? []).map(obj => new ConfigReader(obj));
+    return (configs ?? []).map(
+      (obj, index) =>
+        new ConfigReader(obj, undefined, this.fullKey(`${key}[${index}]`)),
+    );
   }
 
   getNumber(key: string): number {
     const value = this.getOptionalNumber(key);
     if (value === undefined) {
-      throw new Error(errors.missing(key));
+      throw new Error(errors.missing(this.fullKey(key)));
     }
     return value;
   }
@@ -115,7 +122,7 @@ export class ConfigReader implements Config {
   getBoolean(key: string): boolean {
     const value = this.getOptionalBoolean(key);
     if (value === undefined) {
-      throw new Error(errors.missing(key));
+      throw new Error(errors.missing(this.fullKey(key)));
     }
     return value;
   }
@@ -130,7 +137,7 @@ export class ConfigReader implements Config {
   getString(key: string): string {
     const value = this.getOptionalString(key);
     if (value === undefined) {
-      throw new Error(errors.missing(key));
+      throw new Error(errors.missing(this.fullKey(key)));
     }
     return value;
   }
@@ -146,7 +153,7 @@ export class ConfigReader implements Config {
   getStringArray(key: string): string[] {
     const value = this.getOptionalStringArray(key);
     if (value === undefined) {
-      throw new Error(errors.missing(key));
+      throw new Error(errors.missing(this.fullKey(key)));
     }
     return value;
   }
@@ -165,6 +172,10 @@ export class ConfigReader implements Config {
     });
   }
 
+  private fullKey(key: string): string {
+    return `${this.prefix}${this.prefix ? '.' : ''}${key}`;
+  }
+
   private readConfigValue<T extends JsonValue>(
     key: string,
     validate: (
@@ -184,7 +195,9 @@ export class ConfigReader implements Config {
           value: theValue = value,
           expected,
         } = result;
-        throw new TypeError(errors.type(keyName, typeOf(theValue), expected));
+        throw new TypeError(
+          errors.type(this.fullKey(keyName), typeOf(theValue), expected),
+        );
       }
     }
 

From 8cda915c70eb68872de454d201324a47682174cb Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 18:43:58 +0200
Subject: [PATCH 07/28] packages/config: added .keys()

---
 packages/config/src/reader.test.ts | 13 +++++++++++++
 packages/config/src/reader.ts      |  6 ++++++
 packages/config/src/types.ts       |  2 ++
 3 files changed, 21 insertions(+)

diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index f23fad8d3b..27dd1b9e3b 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -37,6 +37,7 @@ const DATA = {
 };
 
 function expectValidValues(config: ConfigReader) {
+  expect(config.keys()).toEqual(Object.keys(DATA));
   expect(config.getNumber('zero')).toBe(0);
   expect(config.getNumber('one')).toBe(1);
   expect(config.getBoolean('true')).toBe(true);
@@ -111,6 +112,7 @@ function expectInvalidValues(config: ConfigReader) {
 describe('ConfigReader', () => {
   it('should read empty config with valid keys', () => {
     const config = new ConfigReader({});
+    expect(config.keys()).toEqual([]);
     expect(config.getOptionalString('x')).toBeUndefined();
     expect(config.getOptionalString('x_x')).toBeUndefined();
     expect(config.getOptionalString('x-X')).toBeUndefined();
@@ -208,6 +210,17 @@ describe('ConfigReader with fallback', () => {
 
     const config = new ConfigReader(a, new ConfigReader(b));
 
+    expect(config.keys()).toEqual(['merged']);
+    expect(config.getConfig('merged').keys()).toEqual([
+      'x',
+      'z',
+      'arr',
+      'config',
+      'configs',
+      'y',
+    ]);
+    expect(config.getConfig('merged.config').keys()).toEqual(['d', 'e']);
+
     expect(config.getString('merged.x')).toBe('x');
     expect(config.getString('merged.y')).toBe('y');
     expect(config.getString('merged.z')).toBe('z1');
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index e3c349768b..e992b7caac 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -68,6 +68,12 @@ export class ConfigReader implements Config {
     private readonly prefix: string = '',
   ) {}
 
+  keys(): string[] {
+    const localKeys = this.data ? Object.keys(this.data) : [];
+    const fallbackKeys = this.fallback?.keys() ?? [];
+    return [...new Set([...localKeys, ...fallbackKeys])];
+  }
+
   getConfig(key: string): ConfigReader {
     const value = this.readValue(key);
     const fallbackConfig = this.fallback?.getConfig(key);
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index 03c9dc4383..4f8a12b5bd 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -27,6 +27,8 @@ export type JsonValue =
 export type AppConfig = JsonObject;
 
 export type Config = {
+  keys(): string[];
+
   getConfig(key: string): Config;
 
   getConfigArray(key: string): Config[];

From ef4991fc2b1436f7e639b5ebc30015dbfd3d30b6 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 17 Jun 2020 20:04:49 +0200
Subject: [PATCH 08/28] packages/config: added context to keep track of where
 config is from for error messages

---
 packages/config/src/reader.test.ts | 130 ++++++++++++++++++++++++-----
 packages/config/src/reader.ts      |  46 +++++++---
 packages/config/src/types.ts       |   5 +-
 3 files changed, 144 insertions(+), 37 deletions(-)

diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index 27dd1b9e3b..365fddd4cf 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -63,37 +63,37 @@ function expectValidValues(config: ConfigReader) {
 
 function expectInvalidValues(config: ConfigReader) {
   expect(() => config.getBoolean('string')).toThrow(
-    'Invalid type in config for key string, got string, wanted boolean',
+    "Invalid type in config for key 'string' in 'ctx', got string, wanted boolean",
   );
   expect(() => config.getNumber('string')).toThrow(
-    'Invalid type in config for key string, got string, wanted number',
+    "Invalid type in config for key 'string' in 'ctx', got string, wanted number",
   );
   expect(() => config.getString('one')).toThrow(
-    'Invalid type in config for key one, got number, wanted string',
+    "Invalid type in config for key 'one' in 'ctx', got number, wanted string",
   );
   expect(() => config.getNumber('true')).toThrow(
-    'Invalid type in config for key true, got boolean, wanted number',
+    "Invalid type in config for key 'true' in 'ctx', got boolean, wanted number",
   );
   expect(() => config.getStringArray('null')).toThrow(
-    'Invalid type in config for key null, got null, wanted string-array',
+    "Invalid type in config for key 'null' in 'ctx', got null, wanted string-array",
   );
   expect(() => config.getString('emptyString')).toThrow(
-    'Invalid type in config for key emptyString, got empty-string, wanted string',
+    "Invalid type in config for key 'emptyString' in 'ctx', got empty-string, wanted string",
   );
   expect(() => config.getStringArray('badStrings')).toThrow(
-    'Invalid type in config for key badStrings[1], got empty-string, wanted string',
+    "Invalid type in config for key 'badStrings[1]' in 'ctx', got empty-string, wanted string",
   );
   expect(() => config.getStringArray('worseStrings')).toThrow(
-    'Invalid type in config for key worseStrings[1], got number, wanted string',
+    "Invalid type in config for key 'worseStrings[1]' in 'ctx', got number, wanted string",
   );
   expect(() => config.getStringArray('worstStrings')).toThrow(
-    'Invalid type in config for key worstStrings[2], got object, wanted string',
+    "Invalid type in config for key 'worstStrings[2]' in 'ctx', got object, wanted string",
   );
   expect(() => config.getConfig('one')).toThrow(
-    'Invalid type in config for key one, got number, wanted object',
+    "Invalid type in config for key 'one' in 'ctx', got number, wanted object",
   );
   expect(() => config.getConfigArray('one')).toThrow(
-    'Invalid type in config for key one, got number, wanted object-array',
+    "Invalid type in config for key 'one' in 'ctx', got number, wanted object-array",
   );
   expect(() => config.getBoolean('missing')).toThrow(
     "Missing required config value at 'missing'",
@@ -109,9 +109,11 @@ function expectInvalidValues(config: ConfigReader) {
   );
 }
 
+const CTX = 'ctx';
+
 describe('ConfigReader', () => {
   it('should read empty config with valid keys', () => {
-    const config = new ConfigReader({});
+    const config = new ConfigReader({}, CTX);
     expect(config.keys()).toEqual([]);
     expect(config.getOptionalString('x')).toBeUndefined();
     expect(config.getOptionalString('x_x')).toBeUndefined();
@@ -121,11 +123,13 @@ describe('ConfigReader', () => {
     expect(config.getOptionalString('x0_x0')).toBeUndefined();
     expect(config.getOptionalString('x_x-x_x')).toBeUndefined();
 
-    expect(new ConfigReader(undefined).getOptionalString('x')).toBeUndefined();
+    expect(
+      new ConfigReader(undefined, CTX).getOptionalString('x'),
+    ).toBeUndefined();
   });
 
   it('should throw on invalid keys', () => {
-    const config = new ConfigReader({});
+    const config = new ConfigReader({}, CTX);
 
     expect(() => config.getString('.')).toThrow(/^Invalid config key/);
     expect(() => config.getString('0')).toThrow(/^Invalid config key/);
@@ -143,38 +147,38 @@ describe('ConfigReader', () => {
     expect(() => config.getString('a._')).toThrow(/^Invalid config key/);
     expect(() => config.getString('a.-.a')).toThrow(/^Invalid config key/);
 
-    expect(() => new ConfigReader(undefined).getString('.')).toThrow(
+    expect(() => new ConfigReader(undefined, CTX).getString('.')).toThrow(
       /^Invalid config key/,
     );
   });
 
   it('should read valid values', () => {
-    const config = new ConfigReader(DATA);
+    const config = new ConfigReader(DATA, CTX);
     expectValidValues(config);
   });
 
   it('should fail to read invalid values', () => {
-    const config = new ConfigReader(DATA);
+    const config = new ConfigReader(DATA, CTX);
     expectInvalidValues(config);
   });
 });
 
 describe('ConfigReader with fallback', () => {
   it('should behave as if without fallback', () => {
-    const config = new ConfigReader({}, new ConfigReader(DATA));
+    const config = new ConfigReader({}, CTX, new ConfigReader(DATA, CTX));
     expect(config.getOptionalString('x')).toBeUndefined();
     expect(() => config.getString('.')).toThrow(/^Invalid config key/);
     expect(() => config.getString('a.')).toThrow(/^Invalid config key/);
   });
 
   it('should read values from itself', () => {
-    const config = new ConfigReader(DATA, new ConfigReader({}));
+    const config = new ConfigReader(DATA, CTX, new ConfigReader({}, CTX));
     expectValidValues(config);
     expectInvalidValues(config);
   });
 
   it('should read values from a fallback', () => {
-    const config = new ConfigReader({}, new ConfigReader(DATA));
+    const config = new ConfigReader({}, CTX, new ConfigReader(DATA, CTX));
     expectValidValues(config);
     expectInvalidValues(config);
   });
@@ -182,12 +186,92 @@ describe('ConfigReader with fallback', () => {
   it('should read values from multiple levels of fallbacks', () => {
     const config = new ConfigReader(
       {},
-      new ConfigReader({}, new ConfigReader({}, new ConfigReader(DATA))),
+      CTX,
+      new ConfigReader(
+        {},
+        CTX,
+        new ConfigReader({}, CTX, new ConfigReader(DATA, CTX)),
+      ),
     );
     expectValidValues(config);
     expectInvalidValues(config);
   });
 
+  it('should show error with correct context', () => {
+    const config = ConfigReader.fromConfigs([
+      {
+        data: {
+          c: true,
+        },
+        context: 'x',
+      },
+      {
+        data: {
+          b: true,
+          c: true,
+          nested1: {
+            a: true,
+          },
+          badBefore: true,
+          badAfter: {
+            a: true,
+          },
+        },
+        context: 'y',
+      },
+      {
+        data: {
+          a: true,
+          b: true,
+          c: true,
+          nested1: {
+            a: true,
+            b: true,
+          },
+          badBefore: {
+            a: true,
+          },
+          badAfter: true,
+        },
+        context: 'z',
+      },
+    ]);
+
+    expect(() => config.getNumber('a')).toThrow(
+      "Invalid type in config for key 'a' in 'z', got boolean, wanted number",
+    );
+    expect(() => config.getNumber('b')).toThrow(
+      "Invalid type in config for key 'b' in 'y', got boolean, wanted number",
+    );
+    expect(() => config.getNumber('c')).toThrow(
+      "Invalid type in config for key 'c' in 'x', got boolean, wanted number",
+    );
+    expect(() => config.getNumber('nested1.a')).toThrow(
+      "Invalid type in config for key 'nested1.a' in 'y', got boolean, wanted number",
+    );
+    expect(() => config.getNumber('nested1.b')).toThrow(
+      "Invalid type in config for key 'nested1.b' in 'z', got boolean, wanted number",
+    );
+    expect(() => config.getConfig('nested1').getNumber('a')).toThrow(
+      "Invalid type in config for key 'nested1.a' in 'y', got boolean, wanted number",
+    );
+    expect(() => config.getConfig('nested1').getNumber('b')).toThrow(
+      "Invalid type in config for key 'nested1.b' in 'z', got boolean, wanted number",
+    );
+    expect(() => config.getNumber('badBefore.a')).toThrow(
+      "Invalid type in config for key 'badBefore' in 'y', got boolean, wanted object",
+    );
+    expect(() => config.getNumber('badBefore.b')).toThrow(
+      "Invalid type in config for key 'badBefore' in 'y', got boolean, wanted object",
+    );
+    expect(() => config.getNumber('badAfter.a')).toThrow(
+      "Invalid type in config for key 'badAfter.a' in 'y', got boolean, wanted number",
+    );
+    expect(() => config.getNumber('badAfter.b')).toThrow(
+      "Invalid type in config for key 'badAfter' in 'z', got boolean, wanted object",
+    );
+  });
+
   it('should read merged objects', () => {
     const a = {
       merged: {
@@ -208,7 +292,7 @@ describe('ConfigReader with fallback', () => {
       },
     };
 
-    const config = new ConfigReader(a, new ConfigReader(b));
+    const config = new ConfigReader(a, CTX, new ConfigReader(b, CTX));
 
     expect(config.keys()).toEqual(['merged']);
     expect(config.getConfig('merged').keys()).toEqual([
@@ -245,7 +329,7 @@ describe('ConfigReader with fallback', () => {
       'b',
     ]);
     expect(() => config.getConfig('merged').getStringArray('x')).toThrow(
-      'Invalid type in config for key merged.x, got string, wanted string-array',
+      "Invalid type in config for key 'merged.x' in 'ctx', got string, wanted string-array",
     );
 
     // Config arrays aren't merged either
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index e992b7caac..e4c4c4d304 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -41,8 +41,8 @@ function typeOf(value: JsonValue | undefined): string {
 
 // Separate out a couple of common error messages to reduce bundle size.
 const errors = {
-  type(key: string, typeName: string, expected: string) {
-    return `Invalid type in config for key ${key}, got ${typeName}, wanted ${expected}`;
+  type(key: string, context: string, typeName: string, expected: string) {
+    return `Invalid type in config for key '${key}' in '${context}', got ${typeName}, wanted ${expected}`;
   },
   missing(key: string) {
     return `Missing required config value at '${key}'`;
@@ -57,13 +57,17 @@ export class ConfigReader implements Config {
 
     // 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, nextConfig) => {
-      return new ConfigReader(nextConfig, previousReader);
-    }, undefined!);
+    return configs.reduceRight<ConfigReader>(
+      (previousReader, { data, context }) => {
+        return new ConfigReader(data, context, previousReader);
+      },
+      undefined!,
+    );
   }
 
   constructor(
     private readonly data: JsonObject | undefined,
+    private readonly context: string = 'empty-config',
     private readonly fallback?: ConfigReader,
     private readonly prefix: string = '',
   ) {}
@@ -80,14 +84,17 @@ export class ConfigReader implements Config {
     const prefix = this.fullKey(key);
 
     if (isObject(value)) {
-      return new ConfigReader(value, fallbackConfig, prefix);
+      return new ConfigReader(value, this.context, fallbackConfig, prefix);
     }
     if (value !== undefined) {
       throw new TypeError(
-        errors.type(this.fullKey(key), typeOf(value), 'object'),
+        errors.type(this.fullKey(key), this.context, typeOf(value), 'object'),
       );
     }
-    return fallbackConfig ?? new ConfigReader(undefined, undefined, prefix);
+    return (
+      fallbackConfig ??
+      new ConfigReader(undefined, undefined, undefined, prefix)
+    );
   }
 
   getConfigArray(key: string): ConfigReader[] {
@@ -106,7 +113,12 @@ export class ConfigReader implements Config {
 
     return (configs ?? []).map(
       (obj, index) =>
-        new ConfigReader(obj, undefined, this.fullKey(`${key}[${index}]`)),
+        new ConfigReader(
+          obj,
+          this.context,
+          undefined,
+          this.fullKey(`${key}[${index}]`),
+        ),
     );
   }
 
@@ -202,7 +214,12 @@ export class ConfigReader implements Config {
           expected,
         } = result;
         throw new TypeError(
-          errors.type(this.fullKey(keyName), typeOf(theValue), expected),
+          errors.type(
+            this.fullKey(keyName),
+            this.context,
+            typeOf(theValue),
+            expected,
+          ),
         );
       }
     }
@@ -223,11 +240,14 @@ export class ConfigReader implements Config {
     }
 
     let value: JsonValue | undefined = this.data;
-    for (const part of parts) {
+    for (const [index, part] of parts.entries()) {
       if (isObject(value)) {
         value = value[part];
-      } else {
-        value = undefined;
+      } else if (value !== undefined) {
+        const badKey = this.fullKey(parts.slice(0, index).join('.'));
+        throw new TypeError(
+          errors.type(badKey, this.context, typeOf(value), 'object'),
+        );
       }
     }
 
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index 4f8a12b5bd..9c49a45af4 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -24,7 +24,10 @@ export type JsonValue =
   | boolean
   | null;
 
-export type AppConfig = JsonObject;
+export type AppConfig = {
+  context: string;
+  data: JsonObject;
+};
 
 export type Config = {
   keys(): string[];

From d4653864fb4512ad791d197811ed0e889168e330 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Wed, 17 Jun 2020 21:18:38 +0200
Subject: [PATCH 09/28] build(deps): bump react-helmet from 6.0.0 to 6.1.0
 (#1327)

Bumps [react-helmet](https://github.com/nfl/react-helmet) from 6.0.0 to 6.1.0.
- [Release notes](https://github.com/nfl/react-helmet/releases)
- [Changelog](https://github.com/nfl/react-helmet/blob/master/CHANGELOG.md)
- [Commits](https://github.com/nfl/react-helmet/compare/6.0.0...6.1.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
---
 packages/core/package.json |  2 +-
 yarn.lock                  | 15 ++++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/packages/core/package.json b/packages/core/package.json
index 69b0590802..01b7ed124d 100644
--- a/packages/core/package.json
+++ b/packages/core/package.json
@@ -46,7 +46,7 @@
     "rc-progress": "^3.0.0",
     "react": "^16.12.0",
     "react-dom": "^16.12.0",
-    "react-helmet": "6.0.0",
+    "react-helmet": "6.1.0",
     "react-hook-form": "^5.7.2",
     "react-router": "6.0.0-alpha.5",
     "react-router-dom": "6.0.0-alpha.5",
diff --git a/yarn.lock b/yarn.lock
index b3b0af2615..37247d71be 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -15485,6 +15485,11 @@ react-fast-compare@^2.0.4:
   resolved "https://registry.npmjs.org/react-fast-compare/-/react-fast-compare-2.0.4.tgz#e84b4d455b0fec113e0402c329352715196f81f9"
   integrity sha512-suNP+J1VU1MWFKcyt7RtjiSWUjvidmQSlqu+eHslq+342xCbGTYmC0mEhPCOHxlW0CywylOC1u2DFAT+bv4dBw==
 
+react-fast-compare@^3.1.1:
+  version "3.2.0"
+  resolved "https://registry.npmjs.org/react-fast-compare/-/react-fast-compare-3.2.0.tgz#641a9da81b6a6320f270e89724fb45a0b39e43bb"
+  integrity sha512-rtGImPZ0YyLrscKI9xTpV8psd6I8VAtjKCzQDlzyDvqJA8XOW78TXYQwNRNd8g8JZnDu8q9Fu/1v4HPAVwVdHA==
+
 react-focus-lock@^2.1.0:
   version "2.2.1"
   resolved "https://registry.npmjs.org/react-focus-lock/-/react-focus-lock-2.2.1.tgz#1d12887416925dc53481914b7cedd39494a3b24a"
@@ -15508,14 +15513,14 @@ react-helmet-async@^1.0.2:
     react-fast-compare "^2.0.4"
     shallowequal "^1.1.0"
 
-react-helmet@6.0.0:
-  version "6.0.0"
-  resolved "https://registry.npmjs.org/react-helmet/-/react-helmet-6.0.0.tgz#fcb93ebaca3ba562a686eb2f1f9d46093d83b5f8"
-  integrity sha512-My6S4sa0uHN/IuVUn0HFmasW5xj9clTkB9qmMngscVycQ5vVG51Qp44BEvLJ4lixupTwDlU9qX1/sCrMN4AEPg==
+react-helmet@6.1.0:
+  version "6.1.0"
+  resolved "https://registry.npmjs.org/react-helmet/-/react-helmet-6.1.0.tgz#a750d5165cb13cf213e44747502652e794468726"
+  integrity sha512-4uMzEY9nlDlgxr61NL3XbKRy1hEkXmKNXhjbAIOVw5vcFrsdYbH2FEwcNyWvWinl103nXgzYNlns9ca+8kFiWw==
   dependencies:
     object-assign "^4.1.1"
     prop-types "^15.7.2"
-    react-fast-compare "^2.0.4"
+    react-fast-compare "^3.1.1"
     react-side-effect "^2.1.0"
 
 react-hook-form@^5.7.2:

From 19f4e434db888efcc58991c446622f4241ee1cb2 Mon Sep 17 00:00:00 2001
From: Nikki Beesetti <12538017+nikkibeesetti@users.noreply.github.com>
Date: Wed, 17 Jun 2020 14:21:35 -0500
Subject: [PATCH 10/28] Updated FAQ with Gitlab link (#1352)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Updated FAQ with Gitlab link

added gitlab link

* Update FAQ.md

Co-authored-by: Stefan Ålund <alund@spotify.com>
---
 docs/FAQ.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/FAQ.md b/docs/FAQ.md
index 3bd526f94b..feb1667717 100644
--- a/docs/FAQ.md
+++ b/docs/FAQ.md
@@ -144,7 +144,7 @@ Integrators also configure closed source plugins locally from the monorepo. ​
 ​ We chose GitHub because it is the tool that we are most familiar with, so that
 will naturally lead to integrations for GitHub being developed at an early
 stage. ​ Hosting this project on GitHub does not exclude integrations with
-alternatives, such as GitLab or Bitbucket. We believe that in time there will be
+alternatives, such as [GitLab](https://github.com/spotify/backstage/issues?q=is%3Aissue+is%3Aopen+GitLab) or Bitbucket. We believe that in time there will be
 plugins that will provide functionality for these tools as well. Hopefully,
 contributed by the community! ​ Also note, implementations of Backstage can be
 hosted wherever you feel suits your needs best. ​

From 0f1ba02bc7e8ea9eb568b917fa9659b825d07aa7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Wed, 17 Jun 2020 22:38:45 +0200
Subject: [PATCH 11/28] Starred icon is yellow (#1351)

---
 .../src/components/CatalogPage/CatalogPage.tsx   | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx
index 2a3746b707..8c9313ca66 100644
--- a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx
+++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx
@@ -25,7 +25,13 @@ import {
 } from '@backstage/core';
 import CatalogLayout from './CatalogLayout';
 import { rootRoute as scaffolderRootRoute } from '@backstage/plugin-scaffolder';
-import { Button, Link, makeStyles, Typography } from '@material-ui/core';
+import {
+  Button,
+  Link,
+  makeStyles,
+  Typography,
+  withStyles,
+} from '@material-ui/core';
 import Edit from '@material-ui/icons/Edit';
 import GitHub from '@material-ui/icons/GitHub';
 import Star from '@material-ui/icons/Star';
@@ -113,6 +119,12 @@ export const CatalogPage: FC<{}> = () => {
 
   const styles = useStyles();
 
+  const YellowStar = withStyles({
+    root: {
+      color: '#f3ba37',
+    },
+  })(Star);
+
   const actions = [
     (rowData: Entity) => {
       const location = findLocationForEntityMeta(rowData.metadata);
@@ -152,7 +164,7 @@ export const CatalogPage: FC<{}> = () => {
     (rowData: Entity) => {
       const isStarred = isStarredEntity(rowData);
       return {
-        icon: isStarred ? Star : StarOutline,
+        icon: isStarred ? YellowStar : StarOutline,
         tooltip: isStarred ? 'Remove from favorites' : 'Add to favorites',
         onClick: () => toggleStarredEntity(rowData),
       };

From f763729120d0e6ab976f77e008d639af949a65f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Wed, 17 Jun 2020 22:44:21 +0200
Subject: [PATCH 12/28] Add some air between sidebar sections (#1355)

---
 packages/core/src/layout/Sidebar/Items.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/layout/Sidebar/Items.tsx b/packages/core/src/layout/Sidebar/Items.tsx
index a7f40f56ba..da0b44d273 100644
--- a/packages/core/src/layout/Sidebar/Items.tsx
+++ b/packages/core/src/layout/Sidebar/Items.tsx
@@ -233,5 +233,5 @@ export const SidebarDivider = styled('hr')({
   width: '100%',
   background: '#383838',
   border: 'none',
-  margin: 0,
+  margin: '12px 0px',
 });

From 1150e0a125183d53f02c02d7db23dd6e8d7e3edf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Wed, 17 Jun 2020 22:57:25 +0200
Subject: [PATCH 13/28] Polishing the Create page (#1353)

* Polishing the Create page

* Review comments
---
 .../src/components/ScaffolderPage/index.tsx   | 29 ++++++++++++++-----
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/plugins/scaffolder/src/components/ScaffolderPage/index.tsx b/plugins/scaffolder/src/components/ScaffolderPage/index.tsx
index aa1cf36082..d675b59f52 100644
--- a/plugins/scaffolder/src/components/ScaffolderPage/index.tsx
+++ b/plugins/scaffolder/src/components/ScaffolderPage/index.tsx
@@ -20,23 +20,33 @@ import {
   Content,
   ContentHeader,
   Header,
+  SupportButton,
   Page,
   pageTheme,
 } from '@backstage/core';
-import { Typography, Link, Button } from '@material-ui/core';
+import { Button, Grid, Link, Typography } from '@material-ui/core';
 import { Link as RouterLink } from 'react-router-dom';
 import TemplateCard from '../TemplateCard';
 
 // TODO(blam): Connect to backend
 const STATIC_DATA = [
+  {
+    id: 'springboot-template',
+    type: 'service',
+    name: 'Spring Boot Service',
+    tags: ['Recommended', 'Java'],
+    description:
+      'Standard Spring Boot (Java) microservice with recommended configuration.',
+    ownerId: 'spotify',
+  },
   {
     id: 'react-ssr-template',
-    type: 'web-infra',
+    type: 'website',
     name: 'SSR React Website',
-    tags: ['Experimental'],
+    tags: ['Recommended', 'React'],
     description:
       'Next.js application skeleton for creating isomorphic web applications.',
-    ownerId: 'something',
+    ownerId: 'spotify',
   },
 ];
 const ScaffolderPage: React.FC<{}> = () => {
@@ -46,7 +56,7 @@ const ScaffolderPage: React.FC<{}> = () => {
         pageTitleOverride="Create a new component"
         title={
           <>
-            Create a new component <Lifecycle alpha shorthand />{' '}
+            Create a new component <Lifecycle alpha shorthand />
           </>
         }
         subtitle="Create new software components using standard templates"
@@ -61,6 +71,11 @@ const ScaffolderPage: React.FC<{}> = () => {
           >
             Register existing component
           </Button>
+          <SupportButton>
+            Create new software components using standard templates. Different
+            templates create different kinds of components (services, websites,
+            documentation, ...).
+          </SupportButton>
         </ContentHeader>
         <Typography variant="body2" paragraph style={{ fontStyle: 'italic' }}>
           <strong>NOTE!</strong> This feature is WIP. You can follow progress{' '}
@@ -69,7 +84,7 @@ const ScaffolderPage: React.FC<{}> = () => {
           </Link>
           .
         </Typography>
-        <div style={{ display: 'flex' }}>
+        <Grid container>
           {STATIC_DATA.map(item => {
             return (
               <TemplateCard
@@ -81,7 +96,7 @@ const ScaffolderPage: React.FC<{}> = () => {
               />
             );
           })}
-        </div>
+        </Grid>
       </Content>
     </Page>
   );

From 66a0bab7f981d3f61110d1fd86e491067db6b5e9 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Thu, 18 Jun 2020 09:16:59 +0200
Subject: [PATCH 14/28] build(deps-dev): bump lint-staged from 10.2.9 to
 10.2.11 (#1359)

Bumps [lint-staged](https://github.com/okonet/lint-staged) from 10.2.9 to 10.2.11.
- [Release notes](https://github.com/okonet/lint-staged/releases)
- [Commits](https://github.com/okonet/lint-staged/compare/v10.2.9...v10.2.11)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
---
 yarn.lock | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/yarn.lock b/yarn.lock
index 37247d71be..58ea0f0e06 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -12177,9 +12177,9 @@ linkify-it@^2.0.0:
     uc.micro "^1.0.1"
 
 lint-staged@^10.1.0:
-  version "10.2.9"
-  resolved "https://registry.npmjs.org/lint-staged/-/lint-staged-10.2.9.tgz#6013ecfa80829cd422446b545fd30a96bca3098c"
-  integrity sha512-ziRAuXEqvJLSXg43ezBpHxRW8FOJCXISaXU//BWrxRrp5cBdRkIx7g5IsB3OI45xYGE0S6cOacfekSjDyDKF2g==
+  version "10.2.11"
+  resolved "https://registry.npmjs.org/lint-staged/-/lint-staged-10.2.11.tgz#713c80877f2dc8b609b05bc59020234e766c9720"
+  integrity sha512-LRRrSogzbixYaZItE2APaS4l2eJMjjf5MbclRZpLJtcQJShcvUzKXsNeZgsLIZ0H0+fg2tL4B59fU9wHIHtFIA==
   dependencies:
     chalk "^4.0.0"
     cli-truncate "2.1.0"

From 15b840acb676dbaca8d2eba4e2f374433bd263f3 Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Thu, 18 Jun 2020 09:17:22 +0200
Subject: [PATCH 15/28] build(deps): bump rollup-plugin-esbuild from 2.0.0 to
 2.1.0 (#1361)

Bumps rollup-plugin-esbuild from 2.0.0 to 2.1.0.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
---
 yarn.lock | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/yarn.lock b/yarn.lock
index 58ea0f0e06..8a2314898c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2452,16 +2452,7 @@
     is-module "^1.0.0"
     resolve "^1.14.2"
 
-"@rollup/pluginutils@^3.0.8":
-  version "3.0.10"
-  resolved "https://registry.npmjs.org/@rollup/pluginutils/-/pluginutils-3.0.10.tgz#a659b9025920378494cd8f8c59fbf9b3a50d5f12"
-  integrity sha512-d44M7t+PjmMrASHbhgpSbVgtL6EFyX7J4mYxwQ/c5eoaE6N2VgCgEcWVzNnwycIloti+/MpwFr8qfw+nRw00sw==
-  dependencies:
-    "@types/estree" "0.0.39"
-    estree-walker "^1.0.1"
-    picomatch "^2.2.2"
-
-"@rollup/pluginutils@^3.1.0":
+"@rollup/pluginutils@^3.0.8", "@rollup/pluginutils@^3.1.0":
   version "3.1.0"
   resolved "https://registry.npmjs.org/@rollup/pluginutils/-/pluginutils-3.1.0.tgz#706b4524ee6dc8b103b3c995533e5ad680c02b9b"
   integrity sha512-GksZ6pr6TpIjHm8h9lSQ8pi8BE9VeubNT0OMJ3B5uZJ8pz73NPiqOtCog/x2/QzM1ENChPKxMDhiQuRHsqc+lg==
@@ -8121,10 +8112,10 @@ es6-shim@^0.35.5:
   resolved "https://registry.npmjs.org/es6-shim/-/es6-shim-0.35.5.tgz#46f59dc0a84a1c5029e8ff1166ca0a902077a9ab"
   integrity sha512-E9kK/bjtCQRpN1K28Xh4BlmP8egvZBGJJ+9GtnzOwt7mdqtrjHFuVGr7QJfdjBIKqrlU5duPf3pCBoDrkjVYFg==
 
-esbuild@^0.4.11:
-  version "0.4.14"
-  resolved "https://registry.npmjs.org/esbuild/-/esbuild-0.4.14.tgz#19c3ec44fe3bb6c637dd5287d870a87d36352dcc"
-  integrity sha512-8lx+KpHMQM6t3JFutzCWLckcaVQyv5qvdCzWHQdXGGh16SXdv5nfo/+izWcF367PlMkCMfV7iWW7J5I8Skx7ZQ==
+esbuild@^0.5.3:
+  version "0.5.3"
+  resolved "https://registry.npmjs.org/esbuild/-/esbuild-0.5.3.tgz#18f5bdb618220c6f14bcb1cf5af528d02d4734c9"
+  integrity sha512-RVzTK62svYjnh+agJRh+NWfZX74iKwFNUX52cF7Mo4QPS6bKxP1o+8GacPUMND2QnodVp2D3nKJs8gLspSfZzA==
 
 escape-goat@^2.0.0:
   version "2.1.1"
@@ -16413,12 +16404,12 @@ rollup-plugin-dts@^1.4.6:
     "@babel/code-frame" "^7.8.3"
 
 rollup-plugin-esbuild@^2.0.0:
-  version "2.0.0"
-  resolved "https://registry.npmjs.org/rollup-plugin-esbuild/-/rollup-plugin-esbuild-2.0.0.tgz#ec6a779f5680410801ad47be9fa9447d02cef5f0"
-  integrity sha512-a5jeHL9Ay1xc8RUULcqkHQ6poMMYCbcTYmfFlYavLg3ALXeqhlmueWAZMIMvr8YFit0Ru75/uKKpBRmN395gEA==
+  version "2.1.0"
+  resolved "https://registry.npmjs.org/rollup-plugin-esbuild/-/rollup-plugin-esbuild-2.1.0.tgz#8e12337c63a5b1144e0c5e8adf2f1568ad4d7d69"
+  integrity sha512-XYqmwk4X0SPEExgilARbre/PplhLtE3q6wiZtfgIbwxJOVGXWec1Bkcux7TFTHGX3TQozzqEASTsRJCt7py/5Q==
   dependencies:
     "@rollup/pluginutils" "^3.1.0"
-    esbuild "^0.4.11"
+    esbuild "^0.5.3"
 
 rollup-plugin-image-files@^1.4.2:
   version "1.4.2"

From 44f717deeece580b2b576fb0e887069021366dad Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Thu, 18 Jun 2020 09:17:48 +0200
Subject: [PATCH 16/28] build(deps): bump @types/react from 16.9.25 to 16.9.37
 (#1342)

Bumps [@types/react](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react) from 16.9.25 to 16.9.37.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
---
 yarn.lock | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/yarn.lock b/yarn.lock
index 8a2314898c..b1b521b438 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3940,9 +3940,9 @@
     "@types/react" "*"
 
 "@types/react@*", "@types/react@^16.9":
-  version "16.9.25"
-  resolved "https://registry.npmjs.org/@types/react/-/react-16.9.25.tgz#6ae2159b40138c792058a23c3c04fd3db49e929e"
-  integrity sha512-Dlj2V72cfYLPNscIG3/SMUOzhzj7GK3bpSrfefwt2YT9GLynvLCCZjbhyF6VsT0q0+aRACRX03TDJGb7cA0cqg==
+  version "16.9.37"
+  resolved "https://registry.npmjs.org/@types/react/-/react-16.9.37.tgz#8fb93e7dbd5b1d3796f69aa979a7fe0439bc7bea"
+  integrity sha512-ZqnAXallQiZ08LTSqMfWMNvAfJEzRLOxdlbbbCIJlYGjU98BEU6bE2uBpKPGeWn+v3hIgCraHKtqUcKZBzMP/Q==
   dependencies:
     "@types/prop-types" "*"
     csstype "^2.2.0"

From b010319400ed10a32e09d8ce58420025433f3fc5 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Thu, 18 Jun 2020 09:45:33 +0200
Subject: [PATCH 17/28] packages/config-loader,core: update config usage to
 include context

---
 packages/config-loader/src/lib/env.test.ts    |  9 ++++---
 packages/config-loader/src/lib/env.ts         |  6 ++---
 packages/config-loader/src/lib/reader.test.ts | 16 +++++++++----
 packages/config-loader/src/lib/reader.ts      | 10 +++++---
 .../core/src/api-wrappers/createApp.test.tsx  | 24 +++++++++++++------
 packages/core/src/api-wrappers/createApp.tsx  |  5 ++--
 6 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/packages/config-loader/src/lib/env.test.ts b/packages/config-loader/src/lib/env.test.ts
index 6a0a115b24..9ecc80d29a 100644
--- a/packages/config-loader/src/lib/env.test.ts
+++ b/packages/config-loader/src/lib/env.test.ts
@@ -45,9 +45,12 @@ describe('readEnv', () => {
       }),
     ).toEqual([
       {
-        foo: 'bar',
-        numbers: { a: 1, b: 2, c: false },
-        very: { deep: { nested: { config: { object: {} } } } },
+        data: {
+          foo: 'bar',
+          numbers: { a: 1, b: 2, c: false },
+          very: { deep: { nested: { config: { object: {} } } } },
+        },
+        context: 'env',
       },
     ]);
   });
diff --git a/packages/config-loader/src/lib/env.ts b/packages/config-loader/src/lib/env.ts
index 83f86d4212..aa986931ff 100644
--- a/packages/config-loader/src/lib/env.ts
+++ b/packages/config-loader/src/lib/env.ts
@@ -42,7 +42,7 @@ 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;
+  let data: JsonObject | undefined = undefined;
 
   for (const [name, value] of Object.entries(env)) {
     if (!value) {
@@ -52,7 +52,7 @@ export function readEnv(env: {
       const key = name.replace(ENV_PREFIX, '');
       const keyParts = key.split('_');
 
-      let obj = (config = config ?? {});
+      let obj = (data = data ?? {});
       for (const [index, part] of keyParts.entries()) {
         if (!CONFIG_KEY_PART_PATTERN.test(part)) {
           throw new TypeError(`Invalid env config key '${key}'`);
@@ -87,5 +87,5 @@ export function readEnv(env: {
     }
   }
 
-  return config ? [config] : [];
+  return data ? [{ data, context: 'env' }] : [];
 }
diff --git a/packages/config-loader/src/lib/reader.test.ts b/packages/config-loader/src/lib/reader.test.ts
index b90c53ee6b..719939ad7f 100644
--- a/packages/config-loader/src/lib/reader.test.ts
+++ b/packages/config-loader/src/lib/reader.test.ts
@@ -38,11 +38,14 @@ describe('readConfigFile', () => {
     } as ReaderContext);
 
     await expect(config).resolves.toEqual({
-      app: {
-        title: 'Test',
-        x: 1,
-        y: [true],
+      data: {
+        app: {
+          title: 'Test',
+          x: 1,
+          y: [true],
+        },
       },
+      context: 'app-config.yaml',
     });
   });
 
@@ -83,7 +86,10 @@ describe('readConfigFile', () => {
     });
 
     await expect(config).resolves.toEqual({
-      app: 'secret',
+      data: {
+        app: 'secret',
+      },
+      context: 'app-config.yaml',
     });
     expect(readSecret).toHaveBeenCalledWith({ file: './my-secret' });
   });
diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts
index 044bf68288..4efaf1986a 100644
--- a/packages/config-loader/src/lib/reader.ts
+++ b/packages/config-loader/src/lib/reader.ts
@@ -15,15 +15,19 @@
  */
 
 import yaml from 'yaml';
+import { basename } from 'path';
 import { isObject } from './utils';
-import { JsonValue, JsonObject } from '@backstage/config';
+import { JsonValue, JsonObject, AppConfig } 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) {
+export async function readConfigFile(
+  filePath: string,
+  ctx: ReaderContext,
+): Promise<AppConfig> {
   const configYaml = await ctx.readFile(filePath);
   const config = yaml.parse(configYaml);
 
@@ -76,5 +80,5 @@ export async function readConfigFile(filePath: string, ctx: ReaderContext) {
   if (!isObject(finalConfig)) {
     throw new TypeError('Expected object at config root');
   }
-  return finalConfig;
+  return { data: finalConfig, context: basename(filePath) };
 }
diff --git a/packages/core/src/api-wrappers/createApp.test.tsx b/packages/core/src/api-wrappers/createApp.test.tsx
index 30553d84a7..0a3756ad92 100644
--- a/packages/core/src/api-wrappers/createApp.test.tsx
+++ b/packages/core/src/api-wrappers/createApp.test.tsx
@@ -15,6 +15,7 @@
  */
 
 import { defaultConfigLoader } from './createApp';
+import { AppConfig } from '@backstage/config';
 
 describe('defaultConfigLoader', () => {
   afterEach(() => {
@@ -24,24 +25,33 @@ describe('defaultConfigLoader', () => {
   it('loads static config', async () => {
     Object.defineProperty(process.env, 'APP_CONFIG', {
       configurable: true,
-      value: [{ my: 'config' }, { my: 'override-config' }] as any,
+      value: [
+        { data: { my: 'config' }, context: 'a' },
+        { data: { my: 'override-config' }, context: 'b' },
+      ] as AppConfig[],
     });
     const configs = await defaultConfigLoader();
-    expect(configs).toEqual([{ my: 'config' }, { my: 'override-config' }]);
+    expect(configs).toEqual([
+      { data: { my: 'config' }, context: 'a' },
+      { data: { my: 'override-config' }, context: 'b' },
+    ]);
   });
 
   it('loads runtime config', async () => {
     Object.defineProperty(process.env, 'APP_CONFIG', {
       configurable: true,
-      value: [{ my: 'override-config' }, { my: 'config' }] as any,
+      value: [
+        { data: { my: 'override-config' }, context: 'a' },
+        { data: { my: 'config' }, context: 'b' },
+      ] as AppConfig[],
     });
     const configs = await (defaultConfigLoader as any)(
       '{"my":"runtime-config"}',
     );
     expect(configs).toEqual([
-      { my: 'runtime-config' },
-      { my: 'override-config' },
-      { my: 'config' },
+      { data: { my: 'runtime-config' }, context: 'env' },
+      { data: { my: 'override-config' }, context: 'a' },
+      { data: { my: 'config' }, context: 'b' },
     ]);
   });
 
@@ -64,7 +74,7 @@ describe('defaultConfigLoader', () => {
   it('fails to load bad runtime config', async () => {
     Object.defineProperty(process.env, 'APP_CONFIG', {
       configurable: true,
-      value: [{ my: 'config' }] as any,
+      value: [{ data: { my: 'config' }, context: 'a' }] as AppConfig[],
     });
 
     await expect((defaultConfigLoader as any)('}')).rejects.toThrow(
diff --git a/packages/core/src/api-wrappers/createApp.tsx b/packages/core/src/api-wrappers/createApp.tsx
index f69451249b..c13758a0b9 100644
--- a/packages/core/src/api-wrappers/createApp.tsx
+++ b/packages/core/src/api-wrappers/createApp.tsx
@@ -27,7 +27,7 @@ import { BrowserRouter, MemoryRouter } from 'react-router-dom';
 import { ErrorPage } from '../layout/ErrorPage';
 import Progress from '../components/Progress';
 import { lightTheme, darkTheme } from '@backstage/theme';
-import { AppConfig } from '@backstage/config';
+import { AppConfig, JsonObject } from '@backstage/config';
 
 const { PrivateAppImpl } = privateExports;
 
@@ -59,7 +59,8 @@ export const defaultConfigLoader: AppConfigLoader = async (
   // Avoiding this string also being replaced at runtime
   if (runtimeConfigJson !== '__app_injected_runtime_config__'.toUpperCase()) {
     try {
-      configs.unshift(JSON.parse(runtimeConfigJson));
+      const data = JSON.parse(runtimeConfigJson) as JsonObject;
+      configs.unshift({ data, context: 'env' });
     } catch (error) {
       throw new Error(`Failed to load runtime configuration, ${error}`);
     }

From 32a4da583c3fecf9acae9085de161e5f26e8a74e Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Thu, 18 Jun 2020 11:03:29 +0200
Subject: [PATCH 18/28] packages/config: add getOptionalConfig and
 getOptionalConfigArray to mirror other accessors

---
 packages/config/src/reader.test.ts | 12 ++++++++----
 packages/config/src/reader.ts      | 29 +++++++++++++++++++++++------
 packages/config/src/types.ts       |  2 ++
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index 365fddd4cf..64e4b3ed69 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -46,10 +46,11 @@ function expectValidValues(config: ConfigReader) {
   expect(config.getStringArray('strings')).toEqual(['string1', 'string2']);
   expect(config.getConfig('nested').getNumber('one')).toBe(1);
   expect(config.getConfig('nested').getString('string')).toBe('string');
-  expect(config.getConfig('nested').getStringArray('strings')).toEqual([
-    'string1',
-    'string2',
-  ]);
+  expect(
+    config.getOptionalConfig('nested')!.getStringArray('strings'),
+  ).toEqual(['string1', 'string2']);
+  expect(config.getOptionalConfig('missing')).toBe(undefined);
+  expect(config.getOptionalConfigArray('missing')).toBe(undefined);
   expect(config.getNumber('zero')).toBe(0);
   expect(config.getBoolean('true')).toBe(true);
   expect(config.getString('string')).toBe('string');
@@ -59,6 +60,9 @@ function expectValidValues(config: ConfigReader) {
   expect(config1.getBoolean('boolean')).toBe(true);
   expect(config2.getString('string')).toBe('string');
   expect(config3.getNumber('number')).toBe(42);
+  expect(
+    config.getOptionalConfigArray('nestlings')![0].getBoolean('boolean'),
+  ).toBe(true);
 }
 
 function expectInvalidValues(config: ConfigReader) {
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index e4c4c4d304..4e302a73a4 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -79,8 +79,16 @@ export class ConfigReader implements Config {
   }
 
   getConfig(key: string): ConfigReader {
+    const value = this.getOptionalConfig(key);
+    if (value === undefined) {
+      throw new Error(errors.missing(this.fullKey(key)));
+    }
+    return value;
+  }
+
+  getOptionalConfig(key: string): ConfigReader | undefined {
     const value = this.readValue(key);
-    const fallbackConfig = this.fallback?.getConfig(key);
+    const fallbackConfig = this.fallback?.getOptionalConfig(key);
     const prefix = this.fullKey(key);
 
     if (isObject(value)) {
@@ -91,13 +99,18 @@ export class ConfigReader implements Config {
         errors.type(this.fullKey(key), this.context, typeOf(value), 'object'),
       );
     }
-    return (
-      fallbackConfig ??
-      new ConfigReader(undefined, undefined, undefined, prefix)
-    );
+    return fallbackConfig;
   }
 
   getConfigArray(key: string): ConfigReader[] {
+    const value = this.getOptionalConfigArray(key);
+    if (value === undefined) {
+      throw new Error(errors.missing(this.fullKey(key)));
+    }
+    return value;
+  }
+
+  getOptionalConfigArray(key: string): ConfigReader[] | undefined {
     const configs = this.readConfigValue<JsonObject[]>(key, values => {
       if (!Array.isArray(values)) {
         return { expected: 'object-array' };
@@ -111,7 +124,11 @@ export class ConfigReader implements Config {
       return true;
     });
 
-    return (configs ?? []).map(
+    if (!configs) {
+      return undefined;
+    }
+
+    return configs.map(
       (obj, index) =>
         new ConfigReader(
           obj,
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index 9c49a45af4..180d526b40 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -33,8 +33,10 @@ export type Config = {
   keys(): string[];
 
   getConfig(key: string): Config;
+  getOptionalConfig(key: string): Config | undefined;
 
   getConfigArray(key: string): Config[];
+  getOptionalConfigArray(key: string): Config[] | undefined;
 
   getNumber(key: string): number;
   getOptionalNumber(key: string): number | undefined;

From a23c276bb3a22ddc10f0b0459f2cdf77964ab201 Mon Sep 17 00:00:00 2001
From: Lee Mills <leem@spotify.com>
Date: Thu, 18 Jun 2020 11:12:46 +0200
Subject: [PATCH 19/28] changed  down caret to be up caret on user profile in
 the sidebar

---
 packages/core/src/layout/Sidebar/Settings/UserProfile.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx
index 9cc122e2fa..06c36bf9ce 100644
--- a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx
+++ b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx
@@ -104,7 +104,7 @@ export const UserProfile: FC<{ open: boolean; setOpen: Function }> = ({
         onClick={handleClick}
         icon={avatar || AccountCircleIcon}
       >
-        {open ? <ExpandLess /> : <ExpandMore />}
+        {open ? <ExpandMore /> : <ExpandLess />}
       </SidebarItem>
     </>
   );

From 02b74c376affbbf7e42b7202b1a3cadcb8b79725 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Thu, 18 Jun 2020 11:58:53 +0200
Subject: [PATCH 20/28] packages/config: added get and getOptional

---
 packages/config/package.json       |   3 +
 packages/config/src/reader.test.ts | 172 +++++++++++++++++++++++++++++
 packages/config/src/reader.ts      |  30 +++++
 packages/config/src/types.ts       |   3 +
 4 files changed, 208 insertions(+)

diff --git a/packages/config/package.json b/packages/config/package.json
index 8bf4001e62..95a5852ae5 100644
--- a/packages/config/package.json
+++ b/packages/config/package.json
@@ -29,6 +29,9 @@
     "postpack": "backstage-cli postpack",
     "clean": "backstage-cli clean"
   },
+  "dependencies": {
+    "lodash": "^4.17.15"
+  },
   "devDependencies": {
     "@types/jest": "^25.2.2",
     "@types/node": "^12.0.0"
diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts
index 64e4b3ed69..9d06e0b025 100644
--- a/packages/config/src/reader.test.ts
+++ b/packages/config/src/reader.test.ts
@@ -38,17 +38,26 @@ const DATA = {
 
 function expectValidValues(config: ConfigReader) {
   expect(config.keys()).toEqual(Object.keys(DATA));
+  expect(config.get('zero')).toBe(0);
   expect(config.getNumber('zero')).toBe(0);
   expect(config.getNumber('one')).toBe(1);
+  expect(config.getOptional('true')).toBe(true);
   expect(config.getBoolean('true')).toBe(true);
   expect(config.getBoolean('false')).toBe(false);
   expect(config.getString('string')).toBe('string');
+  expect(config.get('strings')).toEqual(['string1', 'string2']);
   expect(config.getStringArray('strings')).toEqual(['string1', 'string2']);
   expect(config.getConfig('nested').getNumber('one')).toBe(1);
+  expect(config.get('nested')).toEqual({
+    one: 1,
+    string: 'string',
+    strings: ['string1', 'string2'],
+  });
   expect(config.getConfig('nested').getString('string')).toBe('string');
   expect(
     config.getOptionalConfig('nested')!.getStringArray('strings'),
   ).toEqual(['string1', 'string2']);
+  expect(config.getOptional('missing')).toBe(undefined);
   expect(config.getOptionalConfig('missing')).toBe(undefined);
   expect(config.getOptionalConfigArray('missing')).toBe(undefined);
   expect(config.getNumber('zero')).toBe(0);
@@ -360,3 +369,166 @@ describe('ConfigReader with fallback', () => {
     ).toBeUndefined();
   });
 });
+
+describe('ConfigReader.get()', () => {
+  const config1 = {
+    a: {
+      x: 'x1',
+      y: ['y11', 'y12', 'y13'],
+      z: false,
+    },
+    b: {
+      x: 'x1',
+      y: ['y11'],
+    },
+  };
+  const config2 = {
+    b: {
+      x: 'x2',
+      y: ['y21', 'y22'],
+      z: 'z2',
+    },
+    c: {
+      c1: {
+        c2: 'c2',
+      },
+    },
+  };
+  const config3 = {
+    c: {
+      c1: 'c1',
+    },
+  };
+  const configs = [
+    {
+      data: config1,
+      context: '1',
+    },
+    {
+      data: config2,
+      context: '2',
+    },
+    {
+      data: config3,
+      context: '3',
+    },
+  ];
+
+  it('should be able to select sub-configs', () => {
+    expect(new ConfigReader(config1).get('a')).toEqual(config1.a);
+    expect(new ConfigReader(config1).get('b')).toEqual(config1.b);
+    expect(new ConfigReader(config2).get('b')).toEqual(config2.b);
+    expect(new ConfigReader(config2).get('c')).toEqual(config2.c);
+    expect(new ConfigReader(config3).get('c')).toEqual(config3.c);
+    expect(new ConfigReader(config2).get('c.c1')).toEqual(config2.c.c1);
+    expect(new ConfigReader(config2).getConfig('c').get('c1')).toEqual(
+      config2.c.c1,
+    );
+  });
+
+  it('should merge in fallback configs', () => {
+    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('a')).toEqual(
+      {
+        x: 'x1',
+        y: ['y11', 'y12', 'y13'],
+        z: false,
+      },
+    );
+    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('b')).toEqual(
+      {
+        x: 'x1',
+        y: ['y11'],
+        z: 'z2',
+      },
+    );
+    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('c')).toEqual(
+      {
+        c1: {
+          c2: 'c2',
+        },
+      },
+    );
+    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('a')).toEqual(
+      {
+        x: 'x1',
+        y: ['y11', 'y12', 'y13'],
+        z: false,
+      },
+    );
+    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('b')).toEqual(
+      {
+        x: 'x1',
+        y: ['y11'],
+        z: 'z2',
+      },
+    );
+    expect(ConfigReader.fromConfigs([configs[0], configs[1]]).get('c')).toEqual(
+      {
+        c1: {
+          c2: 'c2',
+        },
+      },
+    );
+
+    expect(
+      ConfigReader.fromConfigs([configs[2], configs[1]]).getOptional('b'),
+    ).toEqual({
+      x: 'x2',
+      y: ['y21', 'y22'],
+      z: 'z2',
+    });
+    expect(
+      ConfigReader.fromConfigs([configs[2], configs[1]]).getOptional('c'),
+    ).toEqual({
+      c1: 'c1',
+    });
+  });
+
+  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'],
+          b: ['1'],
+          c: ['1'],
+          d: ['2'],
+          e: {
+            y: 'y',
+          },
+          f: { x: 'x' },
+          g: 'bar',
+          h: {
+            a: 'a2',
+            b: 'b2',
+          },
+        },
+        context: '2',
+      },
+    ]);
+    expect(config.get('a')).toEqual(['1', '2']);
+    expect(config.get('b')).toEqual(['1']);
+    expect(config.get('c')).toEqual([]);
+    expect(config.get('d')).toEqual({ x: 'x' });
+    expect(config.get('e')).toEqual(['3']);
+    expect(config.get('f')).toEqual('foo');
+    expect(config.get('g')).toEqual({ z: 'z' });
+    expect(config.get('h')).toEqual({ a: 'a1', b: 'b2', c: 'c1' });
+  });
+});
diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts
index 4e302a73a4..0669bcb6d8 100644
--- a/packages/config/src/reader.ts
+++ b/packages/config/src/reader.ts
@@ -15,6 +15,8 @@
  */
 
 import { AppConfig, Config, JsonValue, JsonObject } from './types';
+import cloneDeep from 'lodash/cloneDeep';
+import mergeWith from 'lodash/mergeWith';
 
 // Update the same pattern in config-loader package if this is changed
 const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i;
@@ -78,6 +80,34 @@ export class ConfigReader implements Config {
     return [...new Set([...localKeys, ...fallbackKeys])];
   }
 
+  get(key: string): JsonValue {
+    const value = this.getOptional(key);
+    if (value === undefined) {
+      throw new Error(errors.missing(this.fullKey(key)));
+    }
+    return value;
+  }
+
+  getOptional(key: string): JsonValue | undefined {
+    const value = this.readValue(key);
+    const fallbackValue = this.fallback?.getOptional(key);
+
+    if (value === undefined) {
+      return fallbackValue;
+    } else if (fallbackValue === undefined) {
+      return value;
+    }
+
+    // Avoid merging arrays and primitive values, since that's how merging works for other
+    // methods for reading config.
+    return mergeWith(
+      {},
+      { value: cloneDeep(fallbackValue) },
+      { value },
+      (into, from) => (!isObject(from) || !isObject(into) ? from : undefined),
+    ).value;
+  }
+
   getConfig(key: string): ConfigReader {
     const value = this.getOptionalConfig(key);
     if (value === undefined) {
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index 180d526b40..aaadcd70dc 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -32,6 +32,9 @@ export type AppConfig = {
 export type Config = {
   keys(): string[];
 
+  get(key: string): JsonValue;
+  getOptional(key: string): JsonValue | undefined;
+
   getConfig(key: string): Config;
   getOptionalConfig(key: string): Config | undefined;
 

From bf93cfb343367455c95c030e4ffc409fa457fb13 Mon Sep 17 00:00:00 2001
From: Raghunandan <soapraj@gmail.com>
Date: Thu, 18 Jun 2020 12:07:12 +0200
Subject: [PATCH 21/28] docs docs

---
 .../core-api/src/apis/definitions/auth.ts     |  24 +-
 .../src/lib/PassportStrategyHelper.ts         |  11 +-
 plugins/auth-backend/src/providers/types.ts   | 214 +++++++++++++++---
 3 files changed, 212 insertions(+), 37 deletions(-)

diff --git a/packages/core-api/src/apis/definitions/auth.ts b/packages/core-api/src/apis/definitions/auth.ts
index 7abab45780..a9b82cefc7 100644
--- a/packages/core-api/src/apis/definitions/auth.ts
+++ b/packages/core-api/src/apis/definitions/auth.ts
@@ -157,25 +157,47 @@ export type ProfileInfoOptions = {
   optional?: boolean;
 };
 
+/**
+ * This API provides access to profile information of the signed in user.
+ */
 export type ProfileInfoApi = {
   getProfile(options?: ProfileInfoOptions): Promise<ProfileInfo | undefined>;
 };
 
+/**
+ * Profile information of a signed in user.
+ */
 export type ProfileInfo = {
-  provider: string;
+  /**
+   * Email ID.
+   */
   email: string;
+  /**
+   * Display name that can be presented to the user.
+   */
   name?: string;
+  /**
+   * URL to an avatar image of the user.
+   */
   picture?: string;
 };
 
+/**
+ * Session state values passed to subscribers of the SessionStateApi.
+ */
 export enum SessionState {
   SignedIn = 'SignedIn',
   SignedOut = 'SignedOut',
 }
 
+/**
+ * This API provides access to an sessionState$ observable which provides an update when the
+ * user performs a sign in or sign out from an auth provider.
+ */
 export type SessionStateApi = {
   sessionState$(): Observable<SessionState>;
 };
+
 /**
  * Provides authentication towards Google APIs and identities.
  *
diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts
index f9e32a2b6f..6fa65d01da 100644
--- a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts
+++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts
@@ -27,7 +27,7 @@ export const makeProfileInfo = (
   profile: passport.Profile,
   params: any,
 ): ProfileInfo => {
-  const { provider, displayName: name } = profile;
+  const { displayName: name } = profile;
 
   let email = '';
   if (profile.emails) {
@@ -51,7 +51,6 @@ export const makeProfileInfo = (
   }
 
   return {
-    provider,
     name,
     email,
     picture,
@@ -100,12 +99,12 @@ export const executeFrameHandlerStrategy = async (
 };
 
 export const executeRefreshTokenStrategy = async (
-  providerstrategy: passport.Strategy,
+  providerStrategy: passport.Strategy,
   refreshToken: string,
   scope: string,
 ): Promise<RefreshTokenResponse> => {
   return new Promise((resolve, reject) => {
-    const anyStrategy = providerstrategy as any;
+    const anyStrategy = providerStrategy as any;
     const OAuth2 = anyStrategy._oauth2.constructor;
     const oauth2 = new OAuth2(
       anyStrategy._oauth2._clientId,
@@ -149,12 +148,12 @@ export const executeRefreshTokenStrategy = async (
 };
 
 export const executeFetchUserProfileStrategy = async (
-  providerstrategy: passport.Strategy,
+  providerStrategy: passport.Strategy,
   accessToken: string,
   params: any,
 ): Promise<ProfileInfo> => {
   return new Promise((resolve, reject) => {
-    const anyStrategy = providerstrategy as any;
+    const anyStrategy = providerStrategy as any;
     anyStrategy.userProfile(
       accessToken,
       (error: Error, passportProfile: passport.Profile) => {
diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts
index 419a041637..fa2ef7f49e 100644
--- a/plugins/auth-backend/src/providers/types.ts
+++ b/plugins/auth-backend/src/providers/types.ts
@@ -18,48 +18,160 @@ import express from 'express';
 import { Logger } from 'winston';
 
 export type OAuthProviderOptions = {
+  /**
+   * Client ID of the auth provider.
+   */
   clientID: string;
+  /**
+   * Client Secret of the auth provider.
+   */
   clientSecret: string;
+  /**
+   * Callback URL to be passed to the auth provider to redirect to after the user signs in.
+   */
   callbackURL: string;
 };
 
-export type SAMLProviderConfig = {
-  entryPoint: string;
-  issuer: string;
-};
-
-export type EnvironmentProviderConfig = {
-  [key: string]: OAuthProviderConfig | SAMLProviderConfig;
-};
-
-export type AuthProviderConfig = {
-  baseUrl: string;
-};
-
 export type OAuthProviderConfig = {
+  /**
+   * If the cookies set by the provider have to be marked secure. For development environment
+   * we don't mark the cookie as secure.
+   */
   secure: boolean;
-  appOrigin: string; // http://localhost:3000
+  /**
+   * The domain:port where the app (frontend) is hosted. This is used to post messages back
+   * to the window that initiates an auth request.
+   */
+  appOrigin: string;
+  /**
+   * Client ID of the auth provider.
+   */
   clientId: string;
+  /**
+   * Client Secret of the auth provider.
+   */
   clientSecret: string;
 };
 
+export type EnvironmentProviderConfig = {
+  /**
+   * key, values are environment names and OAuthProviderConfigs
+   *
+   * For e.g
+   * {
+   *   development: DevelopmentOAuthProviderConfig
+   *   production: ProductionOAuthProviderConfig
+   * }
+   */
+  [key: string]: OAuthProviderConfig;
+};
+
+export type AuthProviderConfig = {
+  /**
+   * The domain:port where the app is hosted. This is used to construct the
+   * callbackURL to redirect to once the user signs in to the auth provider.
+   */
+  baseUrl: string;
+};
+
+/**
+ * Any OAuth provider needs to implement this interface which has provider specific
+ * handlers for different methods to perform authentication, get access tokens,
+ * refresh tokens and perform sign out.
+ */
 export interface OAuthProviderHandlers {
+  /**
+   * This method initiates a sign in request with an auth provider.
+   * @param {express.Request} req
+   * @param options
+   */
   start(req: express.Request, options: any): Promise<any>;
+
+  /**
+   * Handles the redirect from the auth provider when the user has signed in.
+   * @param {express.Request} req
+   */
   handler(req: express.Request): Promise<any>;
+
+  /**
+   * (Optional) Given a refresh token and scope fetches a new access token from the auth provider.
+   * @param {string} refreshToken
+   * @param {string} scope
+   */
   refresh?(refreshToken: string, scope: string): Promise<any>;
+
+  /**
+   * (Optional) Sign out of the auth provider.
+   */
   logout?(): Promise<any>;
 }
 
+/**
+ * Any Auth provider needs to implement this interface which handles the routes in the
+ * auth backend. Any auth API requests from the frontend reaches these methods.
+ *
+ * The routes in the auth backend API are tied to these methods like below
+ *
+ * /auth/[provider]/start -> start
+ * /auth/[provider]/handler/frame -> frameHandler
+ * /auth/[provider]/refresh -> refresh
+ * /auth/[provider]/logout -> logout
+ */
 export interface AuthProviderRouteHandlers {
+  /**
+   * Handles the start route of the API. This initiates a sign in request with an auth provider.
+   *
+   * Request
+   * - scopes for the auth request (Optional)
+   * Response
+   * - redirect to the auth provider for the user to sign in or consent.
+   * - sets a nonce cookie and also pass the nonce as 'state' query parameter in the redirect request
+   *
+   * @param {express.Request} req
+   * @param {express.Response} res
+   */
   start(req: express.Request, res: express.Response): Promise<any>;
-  frameHandler(req: express.Request, res: express.Response): Promise<any>;
-  refresh?(req: express.Request, res: express.Response): Promise<any>;
-  logout(req: express.Request, res: express.Response): Promise<any>;
-}
 
-export type SAMLEnvironmentProviderConfig = {
-  [key: string]: SAMLProviderConfig;
-};
+  /**
+   * Once the user signs in or consents in the OAuth screen, the auth provider redirects to the
+   * callbackURL which is handled by this method.
+   *
+   * Request
+   * - to contain a nonce cookie and a 'state' query parameter
+   * Response
+   * - postMessage to the window with a payload that contains accessToken, expiryInSeconds?, idToken? and scope.
+   * - sets a refresh token cookie if the auth provider supports refresh tokens
+   *
+   * @param {express.Request} req
+   * @param {express.Response} res
+   */
+  frameHandler(req: express.Request, res: express.Response): Promise<any>;
+
+  /**
+   * (Optional) If the auth provider supports refresh tokens then this method handles
+   * requests to get a new access token.
+   *
+   * Request
+   * - to contain a refresh token cookie and scope (Optional) query parameter.
+   * Response
+   * - payload with accessToken, expiryInSeconds?, idToken?, scope and user profile information.
+   *
+   * @param {express.Request} req
+   * @param {express.Response} res
+   */
+  refresh?(req: express.Request, res: express.Response): Promise<any>;
+
+  /**
+   * (Optional) Handles logout requests
+   *
+   * Response
+   * - removes the refresh token cookie
+   *
+   * @param {express.Request} req
+   * @param {express.Response} res
+   */
+  logout?(req: express.Request, res: express.Response): Promise<any>;
+}
 
 export type AuthProviderFactory = (
   globalConfig: AuthProviderConfig,
@@ -68,27 +180,42 @@ export type AuthProviderFactory = (
 ) => AuthProviderRouteHandlers;
 
 export type AuthInfoBase = {
+  /**
+   * An access token issued for the signed in user.
+   */
   accessToken: string;
+  /**
+   * (Optional) Id token issued for the signed in user.
+   */
   idToken?: string;
+  /**
+   * Expiry of the access token in seconds.
+   */
   expiresInSeconds?: number;
+  /**
+   * Scopes granted for the access token.
+   */
   scope: string;
 };
 
 export type AuthInfoWithProfile = AuthInfoBase & {
-  profile:
-    | {
-        provider: string;
-        email: string;
-        name?: string;
-        picture?: string;
-      }
-    | undefined;
+  /**
+   * Profile information of the signed in user.
+   */
+  profile: ProfileInfo | undefined;
 };
 
 export type AuthInfoPrivate = {
+  /**
+   * A refresh token issued for the signed in user.
+   */
   refreshToken: string;
 };
 
+/**
+ * Payload sent as a post message after the auth request is complete.
+ * If successful then has a valid payload with Auth information else contains an error.
+ */
 export type AuthResponse =
   | {
       type: 'auth-result';
@@ -100,18 +227,45 @@ export type AuthResponse =
     };
 
 export type RedirectInfo = {
+  /**
+   * URL to redirect to
+   */
   url: string;
+  /**
+   * Status code to use for the redirect
+   */
   status?: number;
 };
 
 export type ProfileInfo = {
-  provider: string;
+  /**
+   * Email ID of the signed in user.
+   */
   email: string;
+  /**
+   * Display name that can be presented to the signed in user.
+   */
   name: string;
+  /**
+   * URL to an image that can be used as the display image or avatar of the
+   * signed in user.
+   */
   picture: string;
 };
 
 export type RefreshTokenResponse = {
+  /**
+   * An access token issued for the signed in user.
+   */
   accessToken: string;
   params: any;
 };
+
+export type SAMLProviderConfig = {
+  entryPoint: string;
+  issuer: string;
+};
+
+export type SAMLEnvironmentProviderConfig = {
+  [key: string]: SAMLProviderConfig;
+};

From efd632d9cef76a8abf992339963e55064ee50026 Mon Sep 17 00:00:00 2001
From: Raghunandan <soapraj@gmail.com>
Date: Thu, 18 Jun 2020 12:10:44 +0200
Subject: [PATCH 22/28] logout -> sign out

---
 plugins/auth-backend/src/providers/types.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts
index fa2ef7f49e..074c78663e 100644
--- a/plugins/auth-backend/src/providers/types.ts
+++ b/plugins/auth-backend/src/providers/types.ts
@@ -162,7 +162,7 @@ export interface AuthProviderRouteHandlers {
   refresh?(req: express.Request, res: express.Response): Promise<any>;
 
   /**
-   * (Optional) Handles logout requests
+   * (Optional) Handles sign out requests
    *
    * Response
    * - removes the refresh token cookie

From a098ebca27c1e5ab18cc7d2ecddb0beaf2e3e1f7 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Thu, 18 Jun 2020 12:20:53 +0200
Subject: [PATCH 23/28] packages,plugins: remove main:src and fix some main
 fields

---
 packages/catalog-model/package.json           |  4 +--
 packages/cli/config/jest.js                   | 25 +++----------------
 packages/cli/src/lib/bundler/config.ts        |  4 +--
 packages/cli/src/lib/tasks.ts                 |  5 +---
 .../plugins/welcome/package.json.hbs          |  3 +--
 .../templates/default-plugin/package.json.hbs |  3 +--
 packages/core-api/package.json                |  3 +--
 packages/core/package.json                    |  3 +--
 packages/dev-utils/package.json               |  3 +--
 packages/storybook/.storybook/main.js         |  2 +-
 packages/test-utils-core/package.json         |  3 +--
 packages/test-utils/package.json              |  3 +--
 packages/theme/package.json                   |  3 +--
 plugins/catalog/package.json                  |  3 +--
 plugins/circleci/package.json                 |  3 +--
 plugins/explore/package.json                  |  3 +--
 plugins/gitops-profiles/package.json          |  3 +--
 plugins/graphiql/package.json                 |  3 +--
 plugins/lighthouse/package.json               |  3 +--
 plugins/register-component/package.json       |  3 +--
 plugins/scaffolder/package.json               |  3 +--
 plugins/sentry/package.json                   |  3 +--
 plugins/tech-radar/package.json               |  3 +--
 plugins/welcome/package.json                  |  3 +--
 24 files changed, 27 insertions(+), 70 deletions(-)

diff --git a/packages/catalog-model/package.json b/packages/catalog-model/package.json
index 60208aeebb..64f49e34e6 100644
--- a/packages/catalog-model/package.json
+++ b/packages/catalog-model/package.json
@@ -1,9 +1,7 @@
 {
   "name": "@backstage/catalog-model",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.cjs.js",
-  "module": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/packages/cli/config/jest.js b/packages/cli/config/jest.js
index 4567d8c593..ad24976ad0 100644
--- a/packages/cli/config/jest.js
+++ b/packages/cli/config/jest.js
@@ -25,32 +25,13 @@ async function getConfig() {
     return require(path.resolve('jest.config.ts'));
   }
 
-  const moduleNameMapper = {
-    '\\.(css|less|scss|sss|styl)$': require.resolve('jest-css-modules'),
-  };
-
-  // Only point to src/ if we're not in CI, there we just build packages first anyway
-  if (!process.env.CI) {
-    const LernaProject = require('@lerna/project');
-    const project = new LernaProject(path.resolve('.'));
-    const packages = await project.getPackages();
-
-    // To avoid having to build all deps inside the monorepo before running tests,
-    // we point directory to src/ where applicable.
-    // For example, @backstage/core = <repo-root>/packages/core/src/index.ts is added to moduleNameMapper
-    for (const pkg of packages) {
-      const mainSrc = pkg.get('main:src');
-      if (mainSrc) {
-        moduleNameMapper[`^${pkg.name}$`] = path.resolve(pkg.location, mainSrc);
-      }
-    }
-  }
-
   const options = {
     rootDir: path.resolve('src'),
     coverageDirectory: path.resolve('coverage'),
     collectCoverageFrom: ['**/*.{js,jsx,ts,tsx}', '!**/*.d.ts'],
-    moduleNameMapper,
+    moduleNameMapper: {
+      '\\.(css|less|scss|sss|styl)$': require.resolve('jest-css-modules'),
+    },
 
     // We build .esm.js files with plugin:build, so to be able to load these in tests they need to be transformed
     // TODO: jest is working on module support, it's possible that we can remove this in the future
diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts
index d5fb674ec1..6fe8092e50 100644
--- a/packages/cli/src/lib/bundler/config.ts
+++ b/packages/cli/src/lib/bundler/config.ts
@@ -109,7 +109,7 @@ export function createConfig(
     entry: [require.resolve('react-hot-loader/patch'), paths.targetEntry],
     resolve: {
       extensions: ['.ts', '.tsx', '.mjs', '.js', '.jsx'],
-      mainFields: ['main:src', 'browser', 'module', 'main'],
+      mainFields: ['browser', 'module', 'main'],
       plugins: [
         new ModuleScopePlugin(
           [paths.targetSrc, paths.targetDev],
@@ -183,7 +183,7 @@ export function createBackendConfig(
     ],
     resolve: {
       extensions: ['.ts', '.tsx', '.mjs', '.js', '.jsx'],
-      mainFields: ['main:src', 'browser', 'module', 'main'],
+      mainFields: ['browser', 'module', 'main'],
       modules: [paths.targetNodeModules, paths.rootNodeModules],
       plugins: [
         new ModuleScopePlugin(
diff --git a/packages/cli/src/lib/tasks.ts b/packages/cli/src/lib/tasks.ts
index 481241a869..b6e2bebb41 100644
--- a/packages/cli/src/lib/tasks.ts
+++ b/packages/cli/src/lib/tasks.ts
@@ -171,9 +171,7 @@ export async function installWithLocalDeps(dir: string) {
   });
 
   // This takes care of pointing all the installed packages from this repo to
-  // dist instead of the local src.
-  // For example node_modules/@backstage/core/packages.json is rewritten to point
-  // types to dist/index.d.ts and the main:src field is removed.
+  // dist instead of the local src, using the field overrides in publishConfig.
   // Without this we get type checking errors in the e2e test
   if (process.env.BACKSTAGE_E2E_CLI_TEST) {
     Task.section('Patching local dependencies for e2e tests');
@@ -192,7 +190,6 @@ export async function installWithLocalDeps(dir: string) {
           const depJson = await fs.readJson(depJsonPath);
 
           // We want dist to be used for e2e tests
-          delete depJson['main:src'];
           for (const key of Object.keys(depJson.publishConfig)) {
             if (key !== 'access') {
               depJson[key] = depJson.publishConfig[key];
diff --git a/packages/cli/templates/default-app/plugins/welcome/package.json.hbs b/packages/cli/templates/default-app/plugins/welcome/package.json.hbs
index c590b68324..d916677d33 100644
--- a/packages/cli/templates/default-app/plugins/welcome/package.json.hbs
+++ b/packages/cli/templates/default-app/plugins/welcome/package.json.hbs
@@ -1,8 +1,7 @@
 {
   "name": "plugin-welcome",
   "version": "0.0.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "private": true,
   "publishConfig": {
diff --git a/packages/cli/templates/default-plugin/package.json.hbs b/packages/cli/templates/default-plugin/package.json.hbs
index c92795cc04..b04da6b793 100644
--- a/packages/cli/templates/default-plugin/package.json.hbs
+++ b/packages/cli/templates/default-plugin/package.json.hbs
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-{{id}}",
   "version": "{{version}}",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": true,
diff --git a/packages/core-api/package.json b/packages/core-api/package.json
index 2659a23ef6..4e3c8ef139 100644
--- a/packages/core-api/package.json
+++ b/packages/core-api/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli build --outputs types,esm",
diff --git a/packages/core/package.json b/packages/core/package.json
index 01b7ed124d..f84181f626 100644
--- a/packages/core/package.json
+++ b/packages/core/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli build --outputs types,esm",
diff --git a/packages/dev-utils/package.json b/packages/dev-utils/package.json
index af1f813148..425d507b17 100644
--- a/packages/dev-utils/package.json
+++ b/packages/dev-utils/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli build --outputs types,esm",
diff --git a/packages/storybook/.storybook/main.js b/packages/storybook/.storybook/main.js
index b8f78f5d2c..2ee58a8c4f 100644
--- a/packages/storybook/.storybook/main.js
+++ b/packages/storybook/.storybook/main.js
@@ -16,7 +16,7 @@ module.exports = {
     const coreSrc = path.resolve(__dirname, '../../core/src');
 
     // Mirror config in packages/cli/src/lib/bundler
-    config.resolve.mainFields = ['main:src', 'browser', 'module', 'main'];
+    config.resolve.mainFields = ['browser', 'module', 'main'];
 
     // Remove the default babel-loader for js files, we're using sucrase instead
     const [jsLoader] = config.module.rules.splice(0, 1);
diff --git a/packages/test-utils-core/package.json b/packages/test-utils-core/package.json
index a71a23c844..3b18ff4c1c 100644
--- a/packages/test-utils-core/package.json
+++ b/packages/test-utils-core/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli build --outputs types,esm",
diff --git a/packages/test-utils/package.json b/packages/test-utils/package.json
index 7c5a229aba..c0752f87ac 100644
--- a/packages/test-utils/package.json
+++ b/packages/test-utils/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli build --outputs types,esm",
diff --git a/packages/theme/package.json b/packages/theme/package.json
index 72a10eb1f3..1c6c096a8f 100644
--- a/packages/theme/package.json
+++ b/packages/theme/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli build --outputs types,esm",
diff --git a/plugins/catalog/package.json b/plugins/catalog/package.json
index 54d5737adb..ddffb1f932 100644
--- a/plugins/catalog/package.json
+++ b/plugins/catalog/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-catalog",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/circleci/package.json b/plugins/circleci/package.json
index 0104c5cc7b..f59d93106a 100644
--- a/plugins/circleci/package.json
+++ b/plugins/circleci/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-circleci",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/explore/package.json b/plugins/explore/package.json
index ad3205f514..74076e9fbb 100644
--- a/plugins/explore/package.json
+++ b/plugins/explore/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-explore",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/gitops-profiles/package.json b/plugins/gitops-profiles/package.json
index c9dca36e08..ee4406be4c 100644
--- a/plugins/gitops-profiles/package.json
+++ b/plugins/gitops-profiles/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-gitops-profiles",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/graphiql/package.json b/plugins/graphiql/package.json
index 49293acbfb..9d4a797c82 100644
--- a/plugins/graphiql/package.json
+++ b/plugins/graphiql/package.json
@@ -18,8 +18,7 @@
     "backstage"
   ],
   "license": "Apache-2.0",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "scripts": {
     "build": "backstage-cli plugin:build",
diff --git a/plugins/lighthouse/package.json b/plugins/lighthouse/package.json
index a3c65d05ea..c96013a814 100644
--- a/plugins/lighthouse/package.json
+++ b/plugins/lighthouse/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-lighthouse",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/register-component/package.json b/plugins/register-component/package.json
index d999ab1dcc..e4c84e448d 100644
--- a/plugins/register-component/package.json
+++ b/plugins/register-component/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-register-component",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json
index 84132a42f9..5c6859cf57 100644
--- a/plugins/scaffolder/package.json
+++ b/plugins/scaffolder/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-scaffolder",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/sentry/package.json b/plugins/sentry/package.json
index 7a8197ab87..80435cf38a 100644
--- a/plugins/sentry/package.json
+++ b/plugins/sentry/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-sentry",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/tech-radar/package.json b/plugins/tech-radar/package.json
index e0146e8a72..16a36dba5d 100644
--- a/plugins/tech-radar/package.json
+++ b/plugins/tech-radar/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-tech-radar",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
   "private": false,
diff --git a/plugins/welcome/package.json b/plugins/welcome/package.json
index 6780f7df98..deed9012da 100644
--- a/plugins/welcome/package.json
+++ b/plugins/welcome/package.json
@@ -1,8 +1,7 @@
 {
   "name": "@backstage/plugin-welcome",
   "version": "0.1.1-alpha.9",
-  "main": "dist/index.esm.js",
-  "main:src": "src/index.ts",
+  "main": "src/index.ts",
   "types": "src/index.ts",
   "private": false,
   "license": "Apache-2.0",

From 8d961027a90184776e004cd008c3ecb4b76fe597 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Wed, 17 Jun 2020 11:25:21 +0200
Subject: [PATCH 24/28] chore(catalog-model): move shared entity model logic
 here

---
 packages/catalog-model/package.json           |   1 +
 packages/catalog-model/src/entity/Entity.ts   |   2 +-
 packages/catalog-model/src/entity/index.ts    |   6 +
 .../catalog-model/src/entity/util.test.ts     | 204 ++++++++++++++++++
 packages/catalog-model/src/entity/util.ts     | 140 ++++++++++++
 .../catalog/DatabaseEntitiesCatalog.test.ts   |  69 +++++-
 .../src/catalog/DatabaseEntitiesCatalog.ts    |  49 +++--
 .../src/database/CommonDatabase.test.ts       |  38 +---
 .../src/database/CommonDatabase.ts            | 124 +++--------
 plugins/catalog-backend/src/database/types.ts |  18 +-
 .../src/ingestion/HigherOrderOperations.ts    |  66 +-----
 11 files changed, 507 insertions(+), 210 deletions(-)
 create mode 100644 packages/catalog-model/src/entity/util.test.ts
 create mode 100644 packages/catalog-model/src/entity/util.ts

diff --git a/packages/catalog-model/package.json b/packages/catalog-model/package.json
index 64f49e34e6..a9bece27ab 100644
--- a/packages/catalog-model/package.json
+++ b/packages/catalog-model/package.json
@@ -22,6 +22,7 @@
   "dependencies": {
     "@types/yup": "^0.28.2",
     "lodash": "^4.17.15",
+    "uuid": "^8.0.0",
     "yup": "^0.28.5"
   },
   "devDependencies": {
diff --git a/packages/catalog-model/src/entity/Entity.ts b/packages/catalog-model/src/entity/Entity.ts
index 70d2ba4d2b..d4f10ab883 100644
--- a/packages/catalog-model/src/entity/Entity.ts
+++ b/packages/catalog-model/src/entity/Entity.ts
@@ -48,7 +48,7 @@ export type Entity = {
  * @see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta
  * @see https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/
  */
-export type EntityMeta = {
+export type EntityMeta = Record<string, any> & {
   /**
    * A globally unique ID for the entity.
    *
diff --git a/packages/catalog-model/src/entity/index.ts b/packages/catalog-model/src/entity/index.ts
index 9e96021336..a1aed856ff 100644
--- a/packages/catalog-model/src/entity/index.ts
+++ b/packages/catalog-model/src/entity/index.ts
@@ -16,3 +16,9 @@
 
 export type { Entity, EntityMeta } from './Entity';
 export * from './policies';
+export {
+  entityHasChanges,
+  generateEntityEtag,
+  generateEntityUid,
+  generateUpdatedEntity,
+} from './util';
diff --git a/packages/catalog-model/src/entity/util.test.ts b/packages/catalog-model/src/entity/util.test.ts
new file mode 100644
index 0000000000..e4399bbe05
--- /dev/null
+++ b/packages/catalog-model/src/entity/util.test.ts
@@ -0,0 +1,204 @@
+/*
+ * 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 lodash from 'lodash';
+import {
+  generateEntityEtag,
+  generateEntityUid,
+  entityHasChanges,
+  generateUpdatedEntity,
+} from './util';
+import { Entity } from './Entity';
+
+describe('util', () => {
+  describe('generateEntityUid', () => {
+    it('generates randomness', () => {
+      expect(generateEntityUid()).not.toEqual('');
+      expect(generateEntityUid()).not.toEqual(generateEntityUid());
+    });
+  });
+
+  describe('generateEntityEtag', () => {
+    it('generates randomness', () => {
+      expect(generateEntityEtag()).not.toEqual('');
+      expect(generateEntityEtag()).not.toEqual(generateEntityEtag());
+    });
+  });
+
+  describe('entityHasChanges', () => {
+    let a: Entity;
+    beforeEach(() => {
+      a = {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'Component',
+        metadata: {
+          name: 'name',
+          custom: 'custom',
+          labels: {
+            labelKey: 'labelValue',
+          },
+          annotations: {
+            annotationKey: 'annotationValue',
+          },
+        },
+        spec: {
+          a: 'a',
+        },
+      };
+    });
+
+    it('happy path: clone has no changes', () => {
+      const b = lodash.cloneDeep(a);
+      expect(entityHasChanges(a, b)).toBe(false);
+    });
+
+    it('detects root field changes', () => {
+      let b: any = lodash.cloneDeep(a);
+      b.apiVersion += 'a';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      delete b.apiVersion;
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      b.kind += 'a';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      delete b.kind;
+      expect(entityHasChanges(a, b)).toBe(true);
+    });
+
+    it('detects metadata changes', () => {
+      let b: any = lodash.cloneDeep(a);
+      b.metadata.name += 'a';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      delete b.metadata.custom;
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      delete b.metadata.custom;
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      b.metadata.labels.n = 'n';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      b.metadata.labels.labelKey += 'a';
+      expect(entityHasChanges(a, b)).toBe(true);
+    });
+
+    it('detects annotation changes, but not removals', () => {
+      let b: any = lodash.cloneDeep(a);
+      b.metadata.annotations.annotationKey += 'a';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      b.metadata.annotations.n = 'n';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      delete b.metadata.annotations.annotationKey;
+      expect(entityHasChanges(a, b)).toBe(false);
+    });
+
+    it('detects spec changes', () => {
+      let b: any = lodash.cloneDeep(a);
+      b.spec.a += 'a';
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      delete b.spec.a;
+      expect(entityHasChanges(a, b)).toBe(true);
+      b = lodash.cloneDeep(a);
+      b.spec.n = 'n';
+      expect(entityHasChanges(a, b)).toBe(true);
+    });
+  });
+
+  describe('generateUpdatedEntity', () => {
+    let a: Entity;
+    let b: any;
+    beforeEach(() => {
+      a = {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'Component',
+        metadata: {
+          uid: 'da921f56-f655-4e6e-9b8b-bb19a57818d8',
+          etag: 'NzY5NDA5NzQtYmEwNC00MDY0LWFiYmItNTYxYzQxM2JhZDcx',
+          generation: 2,
+          name: 'name',
+          custom: 'custom',
+          labels: {
+            labelKey: 'labelValue',
+          },
+          annotations: {
+            annotationKey: 'annotationValue',
+          },
+        },
+        spec: {
+          a: 'a',
+        },
+      };
+      b = lodash.cloneDeep(a);
+      delete b.metadata.uid;
+      delete b.metadata.etag;
+      delete b.metadata.generation;
+    });
+
+    it('happy path: running on itself leaves it unchanged', () => {
+      const result = generateUpdatedEntity(a, b);
+      expect(result).toEqual(a);
+    });
+
+    it('bumps etag and generation when spec is changed', () => {
+      b.spec.a += 'a';
+      const result = generateUpdatedEntity(a, b);
+      expect(result.metadata.uid).toEqual(a.metadata.uid);
+      expect(result.metadata.etag).not.toEqual(a.metadata.etag);
+      expect(result.metadata.generation).toEqual(a.metadata.generation! + 1);
+      expect(result.spec).toEqual({ a: 'aa' });
+    });
+
+    it('bumps only etag when other things than spec are changed', () => {
+      b.metadata.n = 'n';
+      const result = generateUpdatedEntity(a, b);
+      expect(result.metadata.uid).toEqual(a.metadata.uid);
+      expect(result.metadata.etag).not.toEqual(a.metadata.etag);
+      expect(result.metadata.generation).toEqual(a.metadata.generation);
+      expect(result.metadata.n).toEqual('n');
+    });
+
+    it('retains new annotations', () => {
+      b.metadata.annotations.annotationKey = 'changedValue';
+      b.metadata.annotations.newKey = 'newValue';
+      const result = generateUpdatedEntity(a, b);
+      expect(result.metadata.uid).toEqual(a.metadata.uid);
+      expect(result.metadata.etag).not.toEqual(a.metadata.etag);
+      expect(result.metadata.generation).toEqual(a.metadata.generation);
+      expect(result.metadata.annotations).toEqual({
+        annotationKey: 'changedValue',
+        newKey: 'newValue',
+      });
+    });
+
+    it('retains old annotations', () => {
+      b.metadata.annotations.newKey = 'newValue';
+      const result = generateUpdatedEntity(a, b);
+      expect(result.metadata.uid).toEqual(a.metadata.uid);
+      expect(result.metadata.etag).not.toEqual(a.metadata.etag);
+      expect(result.metadata.generation).toEqual(a.metadata.generation);
+      expect(result.metadata.annotations).toEqual({
+        annotationKey: 'annotationValue',
+        newKey: 'newValue',
+      });
+    });
+  });
+});
diff --git a/packages/catalog-model/src/entity/util.ts b/packages/catalog-model/src/entity/util.ts
new file mode 100644
index 0000000000..2a196a65d8
--- /dev/null
+++ b/packages/catalog-model/src/entity/util.ts
@@ -0,0 +1,140 @@
+/*
+ * 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 lodash from 'lodash';
+import { v4 as uuidv4 } from 'uuid';
+import { Entity } from './Entity';
+
+/**
+ * Generates a new random UID for an entity.
+ *
+ * @returns A string with enough randomness to uniquely identify an entity
+ */
+export function generateEntityUid(): string {
+  return uuidv4();
+}
+
+/**
+ * Generates a new random Etag for an entity.
+ *
+ * @returns A string with enough randomness to uniquely identify an entity
+ *          revision
+ */
+export function generateEntityEtag(): string {
+  return Buffer.from(uuidv4(), 'utf8').toString('base64').replace(/[^\w]/g, '');
+}
+
+/**
+ * Checks whether there are any significant changes going from the previous to
+ * the next version of this entity.
+ *
+ * Significance, in this case, means that we do not compare generated fields
+ * such as uid, etag and generation, and we only check that no new annotations
+ * are added or existing annotations were changed (since they are effectively
+ * merged when doing updates).
+ *
+ * @param previous The old state of the entity
+ * @param next The new state of the entity
+ */
+export function entityHasChanges(previous: Entity, next: Entity): boolean {
+  if (entityHasAnnotationChanges(previous, next)) {
+    return true;
+  }
+
+  const e1 = lodash.cloneDeep(previous);
+  const e2 = lodash.cloneDeep(next);
+
+  if (!e1.metadata.labels) {
+    e1.metadata.labels = {};
+  }
+  if (!e2.metadata.labels) {
+    e2.metadata.labels = {};
+  }
+
+  // Remove generated fields
+  delete e1.metadata.uid;
+  delete e1.metadata.etag;
+  delete e1.metadata.generation;
+  delete e2.metadata.uid;
+  delete e2.metadata.etag;
+  delete e2.metadata.generation;
+
+  // Remove already compared things
+  delete e1.metadata.annotations;
+  delete e2.metadata.annotations;
+
+  return !lodash.isEqual(e1, e2);
+}
+
+/**
+ * Takes an old revision of an entity and a new desired state, and merges
+ * them into a complete new state.
+ *
+ * The previous revision is expected to be a complete model loaded from the
+ * catalog, including the uid, etag and generation fields.
+ *
+ * @param previous The old state of the entity
+ * @param next The new state of the entity
+ * @returns An entity with the merged state of both
+ */
+export function generateUpdatedEntity(previous: Entity, next: Entity): Entity {
+  const { uid, etag, generation } = previous.metadata;
+  if (!uid || !etag || !generation) {
+    throw new Error('Previous entity must have uid, etag and generation');
+  }
+
+  const result = lodash.cloneDeep(next);
+
+  // Annotations are merged, with the new ones taking precedence
+  if (previous.metadata.annotations) {
+    next.metadata.annotations = {
+      ...previous.metadata.annotations,
+      ...next.metadata.annotations,
+    };
+  }
+
+  // Generated fields are copied and updated
+  const bumpEtag = entityHasChanges(previous, result);
+  const bumpGeneration = !lodash.isEqual(previous.spec, result.spec);
+  result.metadata.uid = uid;
+  result.metadata.etag = bumpEtag ? generateEntityEtag() : etag;
+  result.metadata.generation = bumpGeneration ? generation + 1 : generation;
+
+  return result;
+}
+
+function entityHasAnnotationChanges(previous: Entity, next: Entity): boolean {
+  // Since the next annotations get merged into the previous, extract only
+  // the overlapping keys and check if their values match.
+  if (next.metadata.annotations) {
+    if (!previous.metadata.annotations) {
+      return true;
+    }
+    if (
+      !lodash.isEqual(
+        next.metadata.annotations,
+        lodash.pick(
+          previous.metadata.annotations,
+          Object.keys(next.metadata.annotations),
+        ),
+      )
+    ) {
+      return true;
+    }
+  }
+
+  return false;
+}
diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts
index 30b480f348..5d35fe814e 100644
--- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts
+++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts
@@ -62,6 +62,11 @@ describe('DatabaseEntitiesCatalog', () => {
       const result = await catalog.addOrUpdateEntity(entity);
 
       expect(db.entities).toHaveBeenCalledTimes(1);
+      expect(db.entities).toHaveBeenCalledWith(expect.anything(), [
+        { key: 'kind', values: ['b'] },
+        { key: 'name', values: ['c'] },
+        { key: 'namespace', values: ['d'] },
+      ]);
       expect(db.addEntity).toHaveBeenCalledTimes(1);
       expect(result).toBe(entity);
     });
@@ -71,20 +76,52 @@ describe('DatabaseEntitiesCatalog', () => {
         apiVersion: 'a',
         kind: 'b',
         metadata: {
-          uid: 'uuuu',
+          uid: 'u',
           name: 'c',
           namespace: 'd',
         },
       };
 
-      db.entities.mockResolvedValue([]);
+      db.entityByUid.mockResolvedValue({
+        entity: {
+          apiVersion: 'a',
+          kind: 'b',
+          metadata: {
+            uid: 'u',
+            etag: 'e',
+            generation: 1,
+            name: 'c',
+            namespace: 'd',
+          },
+        },
+      });
       db.updateEntity.mockResolvedValue({ entity });
 
       const catalog = new DatabaseEntitiesCatalog(db);
       const result = await catalog.addOrUpdateEntity(entity);
 
       expect(db.entities).toHaveBeenCalledTimes(0);
+      expect(db.entityByUid).toHaveBeenCalledTimes(1);
+      expect(db.entityByUid).toHaveBeenCalledWith(expect.anything(), 'u');
       expect(db.updateEntity).toHaveBeenCalledTimes(1);
+      expect(db.updateEntity).toHaveBeenCalledWith(
+        expect.anything(),
+        {
+          entity: {
+            apiVersion: 'a',
+            kind: 'b',
+            metadata: {
+              uid: 'u',
+              etag: 'e',
+              generation: 1,
+              name: 'c',
+              namespace: 'd',
+            },
+          },
+        },
+        'e',
+        1,
+      );
       expect(result).toBe(entity);
     });
 
@@ -101,19 +138,45 @@ describe('DatabaseEntitiesCatalog', () => {
         apiVersion: 'a',
         kind: 'b',
         metadata: {
+          uid: 'u',
+          etag: 'e',
+          generation: 1,
           name: 'c',
           namespace: 'd',
         },
       };
 
       db.entities.mockResolvedValue([{ entity: existing }]);
-      db.updateEntity.mockResolvedValue({ entity: added });
+      db.updateEntity.mockResolvedValue({ entity: existing });
 
       const catalog = new DatabaseEntitiesCatalog(db);
       const result = await catalog.addOrUpdateEntity(added);
 
       expect(db.entities).toHaveBeenCalledTimes(1);
+      expect(db.entities).toHaveBeenCalledWith(expect.anything(), [
+        { key: 'kind', values: ['b'] },
+        { key: 'name', values: ['c'] },
+        { key: 'namespace', values: ['d'] },
+      ]);
       expect(db.updateEntity).toHaveBeenCalledTimes(1);
+      expect(db.updateEntity).toHaveBeenCalledWith(
+        expect.anything(),
+        {
+          entity: {
+            apiVersion: 'a',
+            kind: 'b',
+            metadata: {
+              uid: 'u',
+              etag: 'e',
+              generation: 1,
+              name: 'c',
+              namespace: 'd',
+            },
+          },
+        },
+        'e',
+        1,
+      );
       expect(result).toEqual(existing);
     });
   });
diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts
index 7c4b964b1b..c7cc4249db 100644
--- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts
+++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts
@@ -15,7 +15,10 @@
  */
 
 import { NotFoundError } from '@backstage/backend-common';
-import { LOCATION_ANNOTATION } from '@backstage/catalog-model';
+import {
+  generateUpdatedEntity,
+  LOCATION_ANNOTATION,
+} from '@backstage/catalog-model';
 import type { Entity } from '@backstage/catalog-model';
 import type { Database, DbEntityResponse, EntityFilters } from '../database';
 import type { EntitiesCatalog } from './types';
@@ -43,9 +46,10 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
     namespace: string | undefined,
     name: string,
   ): Promise<Entity | undefined> {
-    return await this.database.transaction(tx =>
+    const response = await this.database.transaction(tx =>
       this.entityByNameInternal(tx, kind, name, namespace),
     );
+    return response ? response.entity : undefined;
   }
 
   async addOrUpdateEntity(
@@ -53,25 +57,30 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
     locationId?: string,
   ): Promise<Entity> {
     return await this.database.transaction(async tx => {
-      let response: DbEntityResponse;
+      // Find a matching (by uid, or by compound name, depending on the given
+      // entity) existing entity, to know whether to update or add
+      const existing = entity.metadata.uid
+        ? await this.database.entityByUid(tx, entity.metadata.uid)
+        : await this.entityByNameInternal(
+            tx,
+            entity.kind,
+            entity.metadata.name,
+            entity.metadata.namespace,
+          );
 
-      if (entity.metadata.uid) {
-        response = await this.database.updateEntity(tx, { locationId, entity });
-      } else {
-        const existing = await this.entityByNameInternal(
+      // If it's an update, run the algorithm for annotation merging, updating
+      // etag/generation, etc.
+      let response: DbEntityResponse;
+      if (existing) {
+        const updated = generateUpdatedEntity(existing.entity, entity);
+        response = await this.database.updateEntity(
           tx,
-          entity.kind,
-          entity.metadata.name,
-          entity.metadata.namespace,
+          { locationId, entity: updated },
+          existing.entity.metadata.etag,
+          existing.entity.metadata.generation,
         );
-        if (existing) {
-          response = await this.database.updateEntity(tx, {
-            locationId,
-            entity,
-          });
-        } else {
-          response = await this.database.addEntity(tx, { locationId, entity });
-        }
+      } else {
+        response = await this.database.addEntity(tx, { locationId, entity });
       }
 
       return response.entity;
@@ -110,7 +119,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
     kind: string,
     name: string,
     namespace: string | undefined,
-  ): Promise<Entity | undefined> {
+  ): Promise<DbEntityResponse | undefined> {
     const matches = await this.database.entities(tx, [
       { key: 'kind', values: [kind] },
       { key: 'name', values: [name] },
@@ -123,6 +132,6 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
       },
     ]);
 
-    return matches.length ? matches[0].entity : undefined;
+    return matches.length ? matches[0] : undefined;
   }
 }
diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts
index 169880e3c9..3d1455a886 100644
--- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts
+++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { ConflictError, NotFoundError } from '@backstage/backend-common';
+import { ConflictError } from '@backstage/backend-common';
 import type { Entity, Location } from '@backstage/catalog-model';
 import { DatabaseManager } from './DatabaseManager';
 import { Database, DatabaseLocationUpdateLogStatus } from './types';
@@ -199,9 +199,7 @@ describe('CommonDatabase', () => {
       );
       expect(updated.entity.apiVersion).toEqual(added.entity.apiVersion);
       expect(updated.entity.kind).toEqual(added.entity.kind);
-      expect(updated.entity.metadata.etag).not.toEqual(
-        added.entity.metadata.etag,
-      );
+      expect(updated.entity.metadata.etag).toEqual(added.entity.metadata.etag);
       expect(updated.entity.metadata.generation).toEqual(
         added.entity.metadata.generation,
       );
@@ -220,41 +218,21 @@ describe('CommonDatabase', () => {
       expect(updated.entity.metadata.name).toEqual('new!');
     });
 
-    it('can update fields if kind, name, and namespace match', async () => {
-      const added = await db.transaction(tx => db.addEntity(tx, entityRequest));
-      added.entity.apiVersion = 'something.new';
-      delete added.entity.metadata.uid;
-      delete added.entity.metadata.generation;
-      const updated = await db.transaction(tx =>
-        db.updateEntity(tx, { entity: added.entity }),
-      );
-      expect(updated.entity.apiVersion).toEqual('something.new');
-    });
-
-    it('rejects if kind, name, but not namespace match', async () => {
-      const added = await db.transaction(tx => db.addEntity(tx, entityRequest));
-      added.entity.apiVersion = 'something.new';
-      delete added.entity.metadata.uid;
-      delete added.entity.metadata.generation;
-      added.entity.metadata.namespace = 'something.wrong';
-      await expect(
-        db.transaction(tx => db.updateEntity(tx, { entity: added.entity })),
-      ).rejects.toThrow(NotFoundError);
-    });
-
     it('fails to update an entity if etag does not match', async () => {
       const added = await db.transaction(tx => db.addEntity(tx, entityRequest));
-      added.entity.metadata.etag = 'garbage';
       await expect(
-        db.transaction(tx => db.updateEntity(tx, { entity: added.entity })),
+        db.transaction(tx =>
+          db.updateEntity(tx, { entity: added.entity }, 'garbage'),
+        ),
       ).rejects.toThrow(ConflictError);
     });
 
     it('fails to update an entity if generation does not match', async () => {
       const added = await db.transaction(tx => db.addEntity(tx, entityRequest));
-      added.entity.metadata.generation! += 100;
       await expect(
-        db.transaction(tx => db.updateEntity(tx, { entity: added.entity })),
+        db.transaction(tx =>
+          db.updateEntity(tx, { entity: added.entity }, undefined, 1e20),
+        ),
       ).rejects.toThrow(ConflictError);
     });
   });
diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts
index 24e9fd3d5e..e986d25c34 100644
--- a/plugins/catalog-backend/src/database/CommonDatabase.ts
+++ b/plugins/catalog-backend/src/database/CommonDatabase.ts
@@ -19,7 +19,13 @@ import {
   InputError,
   NotFoundError,
 } from '@backstage/backend-common';
-import { Entity, EntityMeta, Location } from '@backstage/catalog-model';
+import {
+  Entity,
+  EntityMeta,
+  generateEntityEtag,
+  generateEntityUid,
+  Location,
+} from '@backstage/catalog-model';
 import Knex from 'knex';
 import lodash from 'lodash';
 import { v4 as uuidv4 } from 'uuid';
@@ -38,16 +44,12 @@ import type {
   EntityFilters,
 } from './types';
 
-function getStrippedMetadata(metadata: EntityMeta): EntityMeta {
-  const output = lodash.cloneDeep(metadata);
-  delete output.uid;
-  delete output.etag;
-  delete output.generation;
-  return output;
-}
-
 function serializeMetadata(metadata: EntityMeta): string {
-  return JSON.stringify(getStrippedMetadata(metadata));
+  const withoutGeneratedFields = lodash.cloneDeep(metadata);
+  delete withoutGeneratedFields.uid;
+  delete withoutGeneratedFields.etag;
+  delete withoutGeneratedFields.generation;
+  return JSON.stringify(withoutGeneratedFields);
 }
 
 function serializeSpec(spec: Entity['spec']): DbEntitiesRow['spec'] {
@@ -99,27 +101,6 @@ function toEntityResponse(row: DbEntitiesRow): DbEntityResponse {
   };
 }
 
-function specsAreEqual(
-  first: string | null,
-  second: object | undefined,
-): boolean {
-  if (!first && !second) {
-    return true;
-  } else if (!first || !second) {
-    return false;
-  }
-
-  return lodash.isEqual(JSON.parse(first), second);
-}
-
-function generateUid(): string {
-  return uuidv4();
-}
-
-function generateEtag(): string {
-  return Buffer.from(uuidv4(), 'utf8').toString('base64').replace(/[^\w]/g, '');
-}
-
 export class CommonDatabase implements Database {
   constructor(
     private readonly database: Knex,
@@ -163,8 +144,8 @@ export class CommonDatabase implements Database {
     const newEntity = lodash.cloneDeep(request.entity);
     newEntity.metadata = {
       ...newEntity.metadata,
-      uid: generateUid(),
-      etag: generateEtag(),
+      uid: generateEntityUid(),
+      etag: generateEntityEtag(),
       generation: 1,
     };
 
@@ -178,35 +159,20 @@ export class CommonDatabase implements Database {
   async updateEntity(
     txOpaque: unknown,
     request: DbEntityRequest,
+    matchingEtag?: string,
+    matchingGeneration?: number,
   ): Promise<DbEntityResponse> {
     const tx = txOpaque as Knex.Transaction<any, any>;
 
-    const { kind } = request.entity;
-    const {
-      uid,
-      etag: expectedOldEtag,
-      generation: expectedOldGeneration,
-      name,
-      namespace,
-    } = request.entity.metadata ?? {};
+    const { uid } = request.entity.metadata;
 
-    // Find existing entities that match the given metadata
-    let entitySelector: Partial<DbEntitiesRow>;
-    if (uid) {
-      entitySelector = { id: uid };
-    } else if (kind && name) {
-      entitySelector = {
-        kind,
-        name: name,
-        namespace: namespace || null,
-      };
-    } else {
-      throw new InputError(
-        'Must specify either uid, or kind + name + namespace to be able to identify an entity',
-      );
+    if (uid === undefined) {
+      throw new InputError('Must specify uid when updating entities');
     }
+
+    // Find existing entity
     const oldRows = await tx<DbEntitiesRow>('entities')
-      .where(entitySelector)
+      .where({ id: uid })
       .select();
     if (oldRows.length !== 1) {
       throw new NotFoundError('No matching entity found');
@@ -217,51 +183,26 @@ export class CommonDatabase implements Database {
     // The Number cast is here because sqlite reads it as a string, no matter
     // what the table actually says
     oldRow.generation = Number(oldRow.generation);
-    if (expectedOldEtag) {
-      if (expectedOldEtag !== oldRow.etag) {
+    if (matchingEtag) {
+      if (matchingEtag !== oldRow.etag) {
         throw new ConflictError(
-          `Etag mismatch, expected="${expectedOldEtag}" found="${oldRow.etag}"`,
+          `Etag mismatch, expected="${matchingEtag}" found="${oldRow.etag}"`,
         );
       }
     }
-    if (expectedOldGeneration) {
-      if (expectedOldGeneration !== oldRow.generation) {
+    if (matchingGeneration) {
+      if (matchingGeneration !== oldRow.generation) {
         throw new ConflictError(
-          `Generation mismatch, expected="${expectedOldGeneration}" found="${oldRow.generation}"`,
+          `Generation mismatch, expected="${matchingGeneration}" found="${oldRow.generation}"`,
         );
       }
     }
 
-    // Build the new shape of the entity
-    const newEtag = generateEtag();
-    const newGeneration = specsAreEqual(oldRow.spec, request.entity.spec)
-      ? oldRow.generation
-      : oldRow.generation + 1;
-    const newEntity = lodash.cloneDeep(request.entity);
-    newEntity.metadata = {
-      ...newEntity.metadata,
-      uid: oldRow.id,
-      etag: newEtag,
-      generation: newGeneration,
-    };
-
-    // Preserve annotations that were set on the old version of the entity,
-    // unless the new version overwrites them
-    if (oldRow.metadata) {
-      const oldMetadata = JSON.parse(oldRow.metadata) as EntityMeta;
-      if (oldMetadata.annotations) {
-        newEntity.metadata.annotations = {
-          ...oldMetadata.annotations,
-          ...newEntity.metadata.annotations,
-        };
-      }
-    }
-
-    await this.ensureNoSimilarNames(tx, newEntity);
+    await this.ensureNoSimilarNames(tx, request.entity);
 
     // Store the updated entity; select on the old etag to ensure that we do
     // not lose to another writer
-    const newRow = toEntityRow(request.locationId, newEntity);
+    const newRow = toEntityRow(request.locationId, request.entity);
     const updatedRows = await tx<DbEntitiesRow>('entities')
       .where({ id: oldRow.id, etag: oldRow.etag })
       .update(newRow);
@@ -271,8 +212,9 @@ export class CommonDatabase implements Database {
       throw new ConflictError(`Failed to update entity`);
     }
 
-    await this.updateEntitiesSearch(tx, oldRow.id, newEntity);
-    return { locationId: request.locationId, entity: newEntity };
+    await this.updateEntitiesSearch(tx, oldRow.id, request.entity);
+
+    return request;
   }
 
   async entities(
diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts
index fc81e124ba..0537b87800 100644
--- a/plugins/catalog-backend/src/database/types.ts
+++ b/plugins/catalog-backend/src/database/types.ts
@@ -104,21 +104,27 @@ export type Database = {
   /**
    * Updates an existing entity in the catalog.
    *
-   * The given entity must contain enough information to identify an already
-   * stored entity in the catalog - either by uid, or by kind + namespace +
-   * name. If no matching entity is found, the operation fails.
+   * The given entity must contain an uid to identify an already stored entity
+   * in the catalog. If it is missing or if no matching entity is found, the
+   * operation fails.
    *
-   * If etag or generation are given, they are taken into account. Attempts to
-   * update a matching entity, but where the etag and/or generation are not
-   * equal to the passed values, will fail.
+   * If matchingEtag or matchingGeneration are given, they are taken into
+   * account. Attempts to update a matching entity, but where the etag and/or
+   * generation are not equal to the passed values, will fail.
    *
    * @param tx An ongoing transaction
    * @param request The entity being updated
+   * @param matchingEtag If specified, reject with ConflictError if not
+   *                     matching the entry in the database
+   * @param matchingGeneration If specified, reject with ConflictError if not
+   *                           matching the entry in the database
    * @returns The updated entity
    */
   updateEntity(
     tx: unknown,
     request: DbEntityRequest,
+    matchingEtag?: string,
+    matchingGeneration?: number,
   ): Promise<DbEntityResponse>;
 
   entities(tx: unknown, filters?: EntityFilters): Promise<DbEntityResponse[]>;
diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts
index 2080b1c878..0224dc7f1d 100644
--- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts
+++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts
@@ -15,8 +15,12 @@
  */
 
 import { InputError } from '@backstage/backend-common';
-import { Entity, Location, LocationSpec } from '@backstage/catalog-model';
-import lodash from 'lodash';
+import {
+  Entity,
+  entityHasChanges,
+  Location,
+  LocationSpec,
+} from '@backstage/catalog-model';
 import { v4 as uuidv4 } from 'uuid';
 import { Logger } from 'winston';
 import { EntitiesCatalog, LocationsCatalog } from '../catalog';
@@ -173,7 +177,7 @@ export class HigherOrderOperations implements HigherOrderOperation {
         if (!previous) {
           this.logger.debug(`No such entity found, adding`);
           await this.entitiesCatalog.addOrUpdateEntity(entity, location.id);
-        } else if (!this.entitiesAreEqual(previous, entity)) {
+        } else if (entityHasChanges(previous, entity)) {
           this.logger.debug(`Different from existing entity, updating`);
           await this.entitiesCatalog.addOrUpdateEntity(entity, location.id);
         } else {
@@ -199,60 +203,4 @@ export class HigherOrderOperations implements HigherOrderOperation {
       }
     }
   }
-
-  // Compares entities, ignoring generated and irrelevant data
-  private entitiesAreEqual(previous: Entity, next: Entity): boolean {
-    if (
-      previous.apiVersion !== next.apiVersion ||
-      previous.kind !== next.kind ||
-      !lodash.isEqual(previous.spec, next.spec) // Accept that {} !== undefined
-    ) {
-      return false;
-    }
-
-    // Since the next annotations get merged into the previous, extract only
-    // the overlapping keys and check if their values match.
-    if (next.metadata.annotations) {
-      if (!previous.metadata.annotations) {
-        return false;
-      }
-      if (
-        !lodash.isEqual(
-          next.metadata.annotations,
-          lodash.pick(
-            previous.metadata.annotations,
-            Object.keys(next.metadata.annotations),
-          ),
-        )
-      ) {
-        return false;
-      }
-    }
-
-    const e1 = lodash.cloneDeep(previous);
-    const e2 = lodash.cloneDeep(next);
-
-    if (!e1.metadata.labels) {
-      e1.metadata.labels = {};
-    }
-    if (!e2.metadata.labels) {
-      e2.metadata.labels = {};
-    }
-
-    // Remove generated fields
-    delete e1.metadata.uid;
-    delete e1.metadata.etag;
-    delete e1.metadata.generation;
-    delete e2.metadata.uid;
-    delete e2.metadata.etag;
-    delete e2.metadata.generation;
-
-    // Remove already compared things
-    delete e1.metadata.annotations;
-    delete e1.spec;
-    delete e2.metadata.annotations;
-    delete e2.spec;
-
-    return lodash.isEqual(e1, e2);
-  }
 }

From 3bff1d6a683ed22330e49869cff295dfa973df21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Thu, 18 Jun 2020 11:54:12 +0200
Subject: [PATCH 25/28] address comments

---
 packages/catalog-model/package.json           |   1 +
 packages/catalog-model/src/entity/Entity.ts   |  11 +-
 packages/catalog-model/src/entity/index.ts    |   1 +
 packages/config-loader/src/lib/reader.ts      |  15 ++-
 packages/config-loader/src/lib/secrets.ts     |   3 +-
 packages/config/src/types.ts                  |   2 +-
 .../src/catalog/DatabaseEntitiesCatalog.ts    |   2 +-
 .../src/database/CommonDatabase.ts            | 114 ++++++++----------
 .../RegisterComponentResultDialog.test.tsx    |  13 +-
 9 files changed, 83 insertions(+), 79 deletions(-)

diff --git a/packages/catalog-model/package.json b/packages/catalog-model/package.json
index a9bece27ab..2516263e8a 100644
--- a/packages/catalog-model/package.json
+++ b/packages/catalog-model/package.json
@@ -20,6 +20,7 @@
     "clean": "backstage-cli clean"
   },
   "dependencies": {
+    "@backstage/config": "^0.1.1-alpha.9",
     "@types/yup": "^0.28.2",
     "lodash": "^4.17.15",
     "uuid": "^8.0.0",
diff --git a/packages/catalog-model/src/entity/Entity.ts b/packages/catalog-model/src/entity/Entity.ts
index d4f10ab883..4f245da8b7 100644
--- a/packages/catalog-model/src/entity/Entity.ts
+++ b/packages/catalog-model/src/entity/Entity.ts
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+import { JsonObject } from '@backstage/config';
+
 /**
  * The format envelope that's common to all versions/kinds of entity.
  *
@@ -39,7 +41,7 @@ export type Entity = {
   /**
    * The specification data describing the entity itself.
    */
-  spec?: object;
+  spec?: JsonObject;
 };
 
 /**
@@ -48,7 +50,7 @@ export type Entity = {
  * @see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta
  * @see https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/
  */
-export type EntityMeta = Record<string, any> & {
+export type EntityMeta = JsonObject & {
   /**
    * A globally unique ID for the entity.
    *
@@ -112,3 +114,8 @@ export type EntityMeta = Record<string, any> & {
    */
   annotations?: Record<string, string>;
 };
+
+/**
+ * The keys of EntityMeta that are auto-generated.
+ */
+export const entityMetaGeneratedFields = ['uid', 'etag', 'generation'] as const;
diff --git a/packages/catalog-model/src/entity/index.ts b/packages/catalog-model/src/entity/index.ts
index a1aed856ff..380f5458cc 100644
--- a/packages/catalog-model/src/entity/index.ts
+++ b/packages/catalog-model/src/entity/index.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+export { entityMetaGeneratedFields } from './Entity';
 export type { Entity, EntityMeta } from './Entity';
 export * from './policies';
 export {
diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts
index 4efaf1986a..584d0e0591 100644
--- a/packages/config-loader/src/lib/reader.ts
+++ b/packages/config-loader/src/lib/reader.ts
@@ -14,11 +14,11 @@
  * limitations under the License.
  */
 
-import yaml from 'yaml';
+import { AppConfig, JsonObject, JsonValue } from '@backstage/config';
 import { basename } from 'path';
-import { isObject } from './utils';
-import { JsonValue, JsonObject, AppConfig } from '@backstage/config';
+import yaml from 'yaml';
 import { ReaderContext } from './types';
+import { isObject } from './utils';
 
 /**
  * Reads and parses, and validates, and transforms a single config file.
@@ -67,9 +67,12 @@ export async function readConfigFile(
     const out: JsonObject = {};
 
     for (const [key, value] of Object.entries(obj)) {
-      const result = await transform(value, `${path}.${key}`);
-      if (result !== undefined) {
-        out[key] = result;
+      // undefined covers optional fields
+      if (value !== undefined) {
+        const result = await transform(value, `${path}.${key}`);
+        if (result !== undefined) {
+          out[key] = result;
+        }
       }
     }
 
diff --git a/packages/config-loader/src/lib/secrets.ts b/packages/config-loader/src/lib/secrets.ts
index c4157120ae..348a41db58 100644
--- a/packages/config-loader/src/lib/secrets.ts
+++ b/packages/config-loader/src/lib/secrets.ts
@@ -122,7 +122,7 @@ export async function readSecret(
     const { path } = secret;
     const parts = typeof path === 'string' ? path.split('.') : path;
 
-    let value: JsonValue = await parser(content);
+    let value: JsonValue | undefined = await parser(content);
     for (const [index, part] of parts.entries()) {
       if (!isObject(value)) {
         const errPath = parts.slice(0, index).join('.');
@@ -132,6 +132,7 @@ export async function readSecret(
       }
       value = value[part];
     }
+
     return String(value);
   }
 
diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts
index aaadcd70dc..3e46874c1d 100644
--- a/packages/config/src/types.ts
+++ b/packages/config/src/types.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-export type JsonObject = { [key in string]: JsonValue };
+export type JsonObject = { [key in string]?: JsonValue };
 export type JsonArray = JsonValue[];
 export type JsonValue =
   | JsonObject
diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts
index c7cc4249db..d6809e468f 100644
--- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts
+++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts
@@ -49,7 +49,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
     const response = await this.database.transaction(tx =>
       this.entityByNameInternal(tx, kind, name, namespace),
     );
-    return response ? response.entity : undefined;
+    return response?.entity;
   }
 
   async addOrUpdateEntity(
diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts
index e986d25c34..c860f52d1e 100644
--- a/plugins/catalog-backend/src/database/CommonDatabase.ts
+++ b/plugins/catalog-backend/src/database/CommonDatabase.ts
@@ -22,6 +22,7 @@ import {
 import {
   Entity,
   EntityMeta,
+  entityMetaGeneratedFields,
   generateEntityEtag,
   generateEntityUid,
   Location,
@@ -44,63 +45,9 @@ import type {
   EntityFilters,
 } from './types';
 
-function serializeMetadata(metadata: EntityMeta): string {
-  const withoutGeneratedFields = lodash.cloneDeep(metadata);
-  delete withoutGeneratedFields.uid;
-  delete withoutGeneratedFields.etag;
-  delete withoutGeneratedFields.generation;
-  return JSON.stringify(withoutGeneratedFields);
-}
-
-function serializeSpec(spec: Entity['spec']): DbEntitiesRow['spec'] {
-  if (!spec) {
-    return null;
-  }
-
-  return JSON.stringify(spec);
-}
-
-function toEntityRow(
-  locationId: string | undefined,
-  entity: Entity,
-): DbEntitiesRow {
-  return {
-    id: entity.metadata.uid!,
-    location_id: locationId || null,
-    etag: entity.metadata.etag!,
-    generation: entity.metadata.generation!,
-    api_version: entity.apiVersion,
-    kind: entity.kind,
-    name: entity.metadata.name,
-    namespace: entity.metadata.namespace || null,
-    metadata: serializeMetadata(entity.metadata),
-    spec: serializeSpec(entity.spec),
-  };
-}
-
-function toEntityResponse(row: DbEntitiesRow): DbEntityResponse {
-  const entity: Entity = {
-    apiVersion: row.api_version,
-    kind: row.kind,
-    metadata: {
-      ...(JSON.parse(row.metadata) as Entity['metadata']),
-      uid: row.id,
-      etag: row.etag,
-      generation: Number(row.generation), // cast because of sqlite
-    },
-  };
-
-  if (row.spec) {
-    const spec = JSON.parse(row.spec);
-    entity.spec = spec;
-  }
-
-  return {
-    locationId: row.location_id || undefined,
-    entity,
-  };
-}
-
+/**
+ * The core database implementation.
+ */
 export class CommonDatabase implements Database {
   constructor(
     private readonly database: Knex,
@@ -149,7 +96,7 @@ export class CommonDatabase implements Database {
       generation: 1,
     };
 
-    const newRow = toEntityRow(request.locationId, newEntity);
+    const newRow = this.toEntityRow(request.locationId, newEntity);
     await tx<DbEntitiesRow>('entities').insert(newRow);
     await this.updateEntitiesSearch(tx, newRow.id, newEntity);
 
@@ -202,7 +149,7 @@ export class CommonDatabase implements Database {
 
     // Store the updated entity; select on the old etag to ensure that we do
     // not lose to another writer
-    const newRow = toEntityRow(request.locationId, request.entity);
+    const newRow = this.toEntityRow(request.locationId, request.entity);
     const updatedRows = await tx<DbEntitiesRow>('entities')
       .where({ id: oldRow.id, etag: oldRow.etag })
       .update(newRow);
@@ -270,7 +217,7 @@ export class CommonDatabase implements Database {
       .orderBy('name', 'asc')
       .groupBy('id');
 
-    return rows.map(row => toEntityResponse(row));
+    return rows.map(row => this.toEntityResponse(row));
   }
 
   async entity(
@@ -289,7 +236,7 @@ export class CommonDatabase implements Database {
       return undefined;
     }
 
-    return toEntityResponse(rows[0]);
+    return this.toEntityResponse(rows[0]);
   }
 
   async entityByUid(
@@ -304,7 +251,7 @@ export class CommonDatabase implements Database {
       return undefined;
     }
 
-    return toEntityResponse(rows[0]);
+    return this.toEntityResponse(rows[0]);
   }
 
   async removeEntity(txOpaque: unknown, uid: string): Promise<void> {
@@ -466,4 +413,47 @@ export class CommonDatabase implements Database {
       }
     }
   }
+
+  private toEntityRow(
+    locationId: string | undefined,
+    entity: Entity,
+  ): DbEntitiesRow {
+    return {
+      id: entity.metadata.uid!,
+      location_id: locationId || null,
+      etag: entity.metadata.etag!,
+      generation: entity.metadata.generation!,
+      api_version: entity.apiVersion,
+      kind: entity.kind,
+      name: entity.metadata.name,
+      namespace: entity.metadata.namespace || null,
+      metadata: JSON.stringify(
+        lodash.omit(entity.metadata, ...entityMetaGeneratedFields),
+      ),
+      spec: entity.spec ? JSON.stringify(entity.spec) : null,
+    };
+  }
+
+  private toEntityResponse(row: DbEntitiesRow): DbEntityResponse {
+    const entity: Entity = {
+      apiVersion: row.api_version,
+      kind: row.kind,
+      metadata: {
+        ...(JSON.parse(row.metadata) as EntityMeta),
+        uid: row.id,
+        etag: row.etag,
+        generation: Number(row.generation), // cast because of sqlite
+      },
+    };
+
+    if (row.spec) {
+      const spec = JSON.parse(row.spec);
+      entity.spec = spec;
+    }
+
+    return {
+      locationId: row.location_id || undefined,
+      entity,
+    };
+  }
 }
diff --git a/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.test.tsx b/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.test.tsx
index d918e72489..71933a0302 100644
--- a/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.test.tsx
+++ b/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.test.tsx
@@ -14,13 +14,12 @@
  * limitations under the License.
  */
 
-import React, { ComponentProps } from 'react';
-import { render, cleanup } from '@testing-library/react';
-import { RegisterComponentResultDialog } from './RegisterComponentResultDialog';
-import { ThemeProvider } from '@material-ui/core';
 import { lightTheme } from '@backstage/theme';
+import { ThemeProvider } from '@material-ui/core';
+import { cleanup, render } from '@testing-library/react';
+import React, { ComponentProps } from 'react';
 import { MemoryRouter } from 'react-router-dom';
-import { Entity } from '@backstage/catalog-model';
+import { RegisterComponentResultDialog } from './RegisterComponentResultDialog';
 
 const setup = (
   props?: Partial<ComponentProps<typeof RegisterComponentResultDialog>>,
@@ -52,6 +51,7 @@ it('should show a list of components if success', async () => {
   const { rendered } = setup({
     entities: [
       {
+        apiVersion: 'backstage.io/v1alpha1',
         kind: 'Component',
         metadata: {
           name: 'Component1',
@@ -61,6 +61,7 @@ it('should show a list of components if success', async () => {
         },
       },
       {
+        apiVersion: 'backstage.io/v1alpha1',
         kind: 'Component',
         metadata: {
           name: 'Component2',
@@ -69,7 +70,7 @@ it('should show a list of components if success', async () => {
           type: 'service',
         },
       },
-    ] as Entity[],
+    ],
   });
 
   expect(

From b421ba6b8dba09edda935a8a4fbf5c3771fefd9d Mon Sep 17 00:00:00 2001
From: Raghunandan <soapraj@gmail.com>
Date: Thu, 18 Jun 2020 13:55:03 +0200
Subject: [PATCH 26/28] review fixes

---
 packages/core-api/src/apis/definitions/auth.ts    |  4 ++--
 .../auth-backend/src/lib/EnvironmentHandler.ts    |  4 +++-
 .../src/lib/PassportStrategyHelper.ts             |  3 ++-
 plugins/auth-backend/src/providers/factories.ts   |  4 +++-
 plugins/auth-backend/src/providers/types.ts       | 15 +++++++++++----
 5 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/packages/core-api/src/apis/definitions/auth.ts b/packages/core-api/src/apis/definitions/auth.ts
index a9b82cefc7..c884bf975b 100644
--- a/packages/core-api/src/apis/definitions/auth.ts
+++ b/packages/core-api/src/apis/definitions/auth.ts
@@ -158,14 +158,14 @@ export type ProfileInfoOptions = {
 };
 
 /**
- * This API provides access to profile information of the signed in user.
+ * This API provides access to profile information of the user from an auth provider.
  */
 export type ProfileInfoApi = {
   getProfile(options?: ProfileInfoOptions): Promise<ProfileInfo | undefined>;
 };
 
 /**
- * Profile information of a signed in user.
+ * Profile information of the user from an auth provider.
  */
 export type ProfileInfo = {
   /**
diff --git a/plugins/auth-backend/src/lib/EnvironmentHandler.ts b/plugins/auth-backend/src/lib/EnvironmentHandler.ts
index c4d8d2b6e7..6c16b3a712 100644
--- a/plugins/auth-backend/src/lib/EnvironmentHandler.ts
+++ b/plugins/auth-backend/src/lib/EnvironmentHandler.ts
@@ -57,6 +57,8 @@ export class EnvironmentHandler implements AuthProviderRouteHandlers {
 
   async logout(req: express.Request, res: express.Response): Promise<void> {
     const provider = this.getProviderForEnv(req);
-    await provider.logout(req, res);
+    if (provider.logout) {
+      await provider.logout(req, res);
+    }
   }
 }
diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts
index 6fa65d01da..1e8dfca5ed 100644
--- a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts
+++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts
@@ -21,6 +21,7 @@ import {
   RedirectInfo,
   RefreshTokenResponse,
   ProfileInfo,
+  ProviderStrategy,
 } from '../providers/types';
 
 export const makeProfileInfo = (
@@ -153,7 +154,7 @@ export const executeFetchUserProfileStrategy = async (
   params: any,
 ): Promise<ProfileInfo> => {
   return new Promise((resolve, reject) => {
-    const anyStrategy = providerStrategy as any;
+    const anyStrategy = (providerStrategy as unknown) as ProviderStrategy;
     anyStrategy.userProfile(
       accessToken,
       (error: Error, passportProfile: passport.Profile) => {
diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts
index 4cc9212620..b5e59b713b 100644
--- a/plugins/auth-backend/src/providers/factories.ts
+++ b/plugins/auth-backend/src/providers/factories.ts
@@ -44,7 +44,9 @@ export const createAuthProviderRouter = (
   router.get('/start', provider.start.bind(provider));
   router.get('/handler/frame', provider.frameHandler.bind(provider));
   router.post('/handler/frame', provider.frameHandler.bind(provider));
-  router.post('/logout', provider.logout.bind(provider));
+  if (provider.logout) {
+    router.post('/logout', provider.logout.bind(provider));
+  }
   if (provider.refresh) {
     router.get('/refresh', provider.refresh.bind(provider));
   }
diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts
index 074c78663e..7c16436021 100644
--- a/plugins/auth-backend/src/providers/types.ts
+++ b/plugins/auth-backend/src/providers/types.ts
@@ -34,12 +34,15 @@ export type OAuthProviderOptions = {
 
 export type OAuthProviderConfig = {
   /**
-   * If the cookies set by the provider have to be marked secure. For development environment
-   * we don't mark the cookie as secure.
+   * Cookies can be marked with a secure flag to send cookies only when the request
+   * is over an encrypted channel (HTTPS).
+   *
+   * For development environment we don't mark the cookie as secure since we serve
+   * localhost over HTTP.
    */
   secure: boolean;
   /**
-   * The domain:port where the app (frontend) is hosted. This is used to post messages back
+   * The protocol://domain[:port] where the app (frontend) is hosted. This is used to post messages back
    * to the window that initiates an auth request.
    */
   appOrigin: string;
@@ -68,7 +71,7 @@ export type EnvironmentProviderConfig = {
 
 export type AuthProviderConfig = {
   /**
-   * The domain:port where the app is hosted. This is used to construct the
+   * The protocol://domain[:port] where the app is hosted. This is used to construct the
    * callbackURL to redirect to once the user signs in to the auth provider.
    */
   baseUrl: string;
@@ -261,6 +264,10 @@ export type RefreshTokenResponse = {
   params: any;
 };
 
+export type ProviderStrategy = {
+  userProfile(accessToken: string, callback: Function): void;
+};
+
 export type SAMLProviderConfig = {
   entryPoint: string;
   issuer: string;

From 97311ba5c69098f2c80f31b495517e029163b0ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Thu, 18 Jun 2020 16:57:27 +0200
Subject: [PATCH 27/28] Update entity overview card grid (#1366)

---
 .../components/EntityMetadataCard/EntityMetadataCard.tsx    | 2 +-
 plugins/catalog/src/components/EntityPage/EntityPage.tsx    | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/catalog/src/components/EntityMetadataCard/EntityMetadataCard.tsx b/plugins/catalog/src/components/EntityMetadataCard/EntityMetadataCard.tsx
index d7a3bc81f6..57082a9c91 100644
--- a/plugins/catalog/src/components/EntityMetadataCard/EntityMetadataCard.tsx
+++ b/plugins/catalog/src/components/EntityMetadataCard/EntityMetadataCard.tsx
@@ -23,7 +23,7 @@ type Props = {
 };
 
 export const EntityMetadataCard: FC<Props> = ({ entity }) => (
-  <InfoCard title="Metadata">
+  <InfoCard title="Information">
     <StructuredMetadataTable metadata={entity.metadata} />
   </InfoCard>
 );
diff --git a/plugins/catalog/src/components/EntityPage/EntityPage.tsx b/plugins/catalog/src/components/EntityPage/EntityPage.tsx
index e74276697b..d9953498ec 100644
--- a/plugins/catalog/src/components/EntityPage/EntityPage.tsx
+++ b/plugins/catalog/src/components/EntityPage/EntityPage.tsx
@@ -149,11 +149,11 @@ export const EntityPage: FC<{}> = () => {
           <HeaderTabs tabs={tabs} />
 
           <Content>
-            <Grid container spacing={3} direction="column">
-              <Grid item>
+            <Grid container spacing={3}>
+              <Grid item sm={4}>
                 <EntityMetadataCard entity={entity} />
               </Grid>
-              <Grid item>
+              <Grid item sm={8}>
                 <SentryIssuesWidget
                   sentryProjectId="sample-sentry-project-id"
                   statsFor="24h"

From b7a1946efeac3e944e04795911bb81bea7698d9f Mon Sep 17 00:00:00 2001
From: "dependabot-preview[bot]"
 <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date: Fri, 19 Jun 2020 22:57:48 +0200
Subject: [PATCH 28/28] build(deps-dev): bump msw from 0.19.3 to 0.19.4 (#1375)

Bumps [msw](https://github.com/mswjs/msw) from 0.19.3 to 0.19.4.
- [Release notes](https://github.com/mswjs/msw/releases)
- [Commits](https://github.com/mswjs/msw/compare/v0.19.3...v0.19.4)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
---
 yarn.lock | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/yarn.lock b/yarn.lock
index b1b521b438..c18da809e9 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -13169,9 +13169,9 @@ ms@^2.0.0, ms@^2.1.1:
   integrity sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==
 
 msw@^0.19.0:
-  version "0.19.3"
-  resolved "https://registry.npmjs.org/msw/-/msw-0.19.3.tgz#88f39edbd37313bff15a0e7cd00c71406cbdbbae"
-  integrity sha512-HYLnyrCDDPP72GG/CeHPhBjHsZFYkz36rJLXDWccZWNA24gYjgrcp9iVqqitk2cI6NAJwsupRml9GkfpJBB74w==
+  version "0.19.4"
+  resolved "https://registry.npmjs.org/msw/-/msw-0.19.4.tgz#059026de0cc3303847c27d3aa0e98705a1033df6"
+  integrity sha512-rNfGgIuO0MyfN2F+FOHRqNd2LJIESaaMOJNNiNMcv/Be7Kdzz/ZmghfS/5zFy2SpHWladccqDYgo74JN+YlEyg==
   dependencies:
     "@open-draft/until" "^1.0.0"
     "@types/cookie" "^0.3.3"