Merge pull request #1346 from spotify/rugvip/mustconf

packages/config: added must* variant for reading required primitive values

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) {

packages/config/src/reader.test.ts

@@ -49,6 +49,10 @@ function expectValidValues(config: ConfigReader) {
     '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);
@@ -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,18 +94,30 @@ function expectInvalidValues(config: ConfigReader) {
   expect(() => config.getConfigArray('one')).toThrow(
     'Invalid type in config for key one, got number, wanted object-array',
   );
+  expect(() => config.getBoolean('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
+  expect(() => config.getNumber('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
+  expect(() => config.getString('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
+  expect(() => config.getStringArray('missing')).toThrow(
+    "Missing required config value at 'missing'",
+  );
 }
 
 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', () => {
@@ -135,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/);
   });
@@ -210,8 +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].getString('a')).toBe('a');
+    expect(() =>
+      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
@@ -220,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();
   });
 });

packages/config/src/reader.ts

@@ -92,21 +92,45 @@ export class ConfigReader implements Config {
     return (configs ?? []).map(obj => new ConfigReader(obj));
   }
 
-  getNumber(key: string): number | undefined {
+  getNumber(key: string): number {
+    const value = this.getOptionalNumber(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
+  getOptionalNumber(key: string): number | undefined {
     return this.readConfigValue(
       key,
       value => typeof value === 'number' || { expected: 'number' },
     );
   }
 
-  getBoolean(key: string): boolean | undefined {
+  getBoolean(key: string): boolean {
+    const value = this.getOptionalBoolean(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
+  getOptionalBoolean(key: string): boolean | undefined {
     return this.readConfigValue(
       key,
       value => typeof value === 'boolean' || { expected: 'boolean' },
     );
   }
 
-  getString(key: string): string | undefined {
+  getString(key: string): string {
+    const value = this.getOptionalString(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
+  getOptionalString(key: string): string | undefined {
     return this.readConfigValue(
       key,
       value =>
@@ -114,7 +138,15 @@ export class ConfigReader implements Config {
     );
   }
 
-  getStringArray(key: string): string[] | undefined {
+  getStringArray(key: string): string[] {
+    const value = this.getOptionalStringArray(key);
+    if (value === undefined) {
+      throw new Error(`Missing required config value at '${key}'`);
+    }
+    return value;
+  }
+
+  getOptionalStringArray(key: string): string[] | undefined {
     return this.readConfigValue(key, values => {
       if (!Array.isArray(values)) {
         return { expected: 'string-array' };

packages/config/src/types.ts

@@ -31,11 +31,15 @@ export type Config = {
 
   getConfigArray(key: string): Config[];
 
-  getNumber(key: string): number | undefined;
+  getNumber(key: string): number;
+  getOptionalNumber(key: string): number | undefined;
 
-  getBoolean(key: string): boolean | undefined;
+  getBoolean(key: string): boolean;
+  getOptionalBoolean(key: string): boolean | undefined;
 
-  getString(key: string): string | undefined;
+  getString(key: string): string;
+  getOptionalString(key: string): string | undefined;
 
-  getStringArray(key: string): string[] | undefined;
+  getStringArray(key: string): string[];
+  getOptionalStringArray(key: string): string[] | undefined;
 };

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 {}

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('/')) {

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>

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 (