Merge pull request #1127 from spotify/rugvip/config

packages/cli,core-api: move config reading and loading into separate packages

packages/cli/package.json

@@ -29,6 +29,8 @@
     "backstage-cli": "bin/backstage-cli"
   },
   "dependencies": {
+    "@backstage/config": "^0.1.1-alpha.6",
+    "@backstage/config-loader": "^0.1.1-alpha.6",
     "@hot-loader/react-dom": "^16.13.0",
     "@lerna/package-graph": "^3.18.5",
     "@lerna/project": "^3.18.0",

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

@@ -16,7 +16,7 @@
 
 import { buildBundle } from '../../lib/bundler';
 import { Command } from 'commander';
-import { loadConfig } from '../../lib/app-config';
+import { loadConfig } from '@backstage/config-loader';
 
 export default async (cmd: Command) => {
   await buildBundle({

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

@@ -16,7 +16,7 @@
 
 import { Command } from 'commander';
 import { serveBundle } from '../../lib/bundler';
-import { loadConfig } from '../../lib/app-config';
+import { loadConfig } from '@backstage/config-loader';
 
 export default async (cmd: Command) => {
   const waitForExit = await serveBundle({

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

@@ -16,7 +16,7 @@
 
 import { Command } from 'commander';
 import { serveBundle } from '../../lib/bundler';
-import { loadConfig } from '../../lib/app-config';
+import { loadConfig } from '@backstage/config-loader';
 
 export default async (cmd: Command) => {
   const waitForExit = await serveBundle({

packages/cli/src/lib/bundler/types.ts

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
+import { AppConfig } from '@backstage/config';
 import { BundlingPathsOptions } from './paths';
-import { AppConfig } from '../app-config';
 
 export type BundlingOptions = {
   checksEnabled: boolean;

packages/cli/src/lib/paths.ts

@@ -57,8 +57,7 @@ export function findRootPath(topPath: string): string {
     const exists = fs.pathExistsSync(packagePath);
     if (exists) {
       try {
-        const contents = fs.readFileSync(packagePath, 'utf8');
-        const data = JSON.parse(contents);
+        const data = fs.readJsonSync(packagePath);
         if (data.name === 'root' || data.name.includes('backstage-e2e')) {
           return path;
         }

packages/cli/src/lib/tasks.ts

@@ -111,6 +111,8 @@ export async function templatingTask(
 // List of local packages that we need to modify as a part of an E2E test
 const PATCH_PACKAGES = [
   'cli',
+  'config',
+  'config-loader',
   'core',
   'core-api',
   'dev-utils',
@@ -193,6 +195,11 @@ export async function installWithLocalDeps(dir: string) {
           delete depJson['main:src'];
           depJson.types = 'dist/index.d.ts';
 
+          // Ugly hack until backend packages can point straight to source
+          if (name === 'config' || name === 'config-loader') {
+            depJson.main = 'dist/index.cjs.js';
+          }
+
           await fs
             .writeJSON(depJsonPath, depJson, { encoding: 'utf8', spaces: 2 })
             .catch(error => {

packages/config-loader/.eslintrc.js

@@ -0,0 +1,3 @@
+module.exports = {
+  extends: [require.resolve('@backstage/cli/config/eslint')],
+};

packages/config-loader/README.md

@@ -0,0 +1,12 @@
+# @backstage/config-loader
+
+This package provides config loading functionality used by the backend, and CLI.
+
+## Installation
+
+Do not install this package directly, it is an internal package used by [@backstage/cli](https://www.npmjs.com/package/@backstage/cli), and [@backstage/backend-common](https://www.npmjs.com/package/@backstage/backend-common). Depend on either of those instead.
+
+## Documentation
+
+- [Backstage Readme](https://github.com/spotify/backstage/blob/master/README.md)
+- [Backstage Documentation](https://github.com/spotify/backstage/blob/master/docs/README.md)

packages/config-loader/package.json

@@ -0,0 +1,41 @@
+{
+  "name": "@backstage/config-loader",
+  "description": "Config loading functionality used by Backstage backend, and CLI",
+  "version": "0.1.1-alpha.6",
+  "private": false,
+  "publishConfig": {
+    "access": "public"
+  },
+  "homepage": "https://backstage.io",
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/spotify/backstage",
+    "directory": "packages/config-loader"
+  },
+  "keywords": [
+    "backstage"
+  ],
+  "license": "Apache-2.0",
+  "main": "src/index.ts",
+  "types": "src/index.ts",
+  "scripts": {
+    "build": "backstage-cli build",
+    "lint": "backstage-cli lint",
+    "test": "backstage-cli test",
+    "prepack": "backstage-cli prepack",
+    "postpack": "backstage-cli postpack",
+    "clean": "backstage-cli clean"
+  },
+  "dependencies": {
+    "@backstage/config": "^0.1.1-alpha.6",
+    "fs-extra": "^9.0.0",
+    "yaml": "^1.9.2"
+  },
+  "devDependencies": {
+    "@types/jest": "^25.2.2",
+    "@types/node": "^12.0.0"
+  },
+  "files": [
+    "dist/**/*.{js,d.ts}"
+  ]
+}

packages/config-loader/src/index.ts

@@ -14,5 +14,5 @@
  * limitations under the License.
  */
 
-export type { AppConfig } from './types';
-export { loadConfig } from './loaders';
+export { loadConfig } from './loader';
+export type { LoadConfigOptions } from './types';

packages/config-loader/src/loader.ts

@@ -14,22 +14,25 @@
  * limitations under the License.
  */
 
-import { AppConfig } from './types';
 import fs from 'fs-extra';
 import yaml from 'yaml';
-import { paths } from '../paths';
-
-type LoadConfigOptions = {
-  // Config path, defaults to app-config.yaml in project root
-  configPath?: string;
-};
+import { resolve as resolvePath } from 'path';
+import { AppConfig } from '@backstage/config';
+import { findRootPath } from './paths';
+import { LoadConfigOptions } from './types';
 
 export async function loadConfig(
   options: LoadConfigOptions = {},
 ): Promise<AppConfig[]> {
   // TODO: We'll want this to be a bit more elaborate, probably adding configs for
   //       specific env, and maybe local config for plugins.
-  const { configPath = paths.resolveTargetRoot('app-config.yaml') } = options;
+  let { configPath } = options;
+  if (!configPath) {
+    configPath = resolvePath(
+      findRootPath(fs.realpathSync(process.cwd())),
+      'app-config.yaml',
+    );
+  }
 
   try {
     const configYaml = await fs.readFile(configPath, 'utf8');

packages/config-loader/src/paths.test.ts

@@ -0,0 +1,24 @@
+/*
+ * 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 { findRootPath } from './paths';
+
+describe('findRootPath', () => {
+  it('should find root path', () => {
+    const rootPath = findRootPath(process.cwd());
+    expect(typeof rootPath).toBe('string');
+  });
+});

packages/config-loader/src/paths.ts

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import fs from 'fs-extra';
+import { dirname, resolve as resolvePath } from 'path';
+
+/**
+ * Looks for a package.json that has name: "root" to identify the root of the monorepo
+ *
+ * This is a copy of the same function in the CLI
+ */
+export function findRootPath(topPath: string): string {
+  let path = topPath;
+
+  // Some sanity check to avoid infinite loop
+  for (let i = 0; i < 1000; i++) {
+    const packagePath = resolvePath(path, 'package.json');
+    const exists = fs.pathExistsSync(packagePath);
+    if (exists) {
+      try {
+        const data = fs.readJsonSync(packagePath);
+        if (data.name === 'root' || data.name.includes('backstage-e2e')) {
+          return path;
+        }
+      } catch (error) {
+        throw new Error(
+          `Failed to parse package.json file while searching for root, ${error}`,
+        );
+      }
+    }
+
+    const newPath = dirname(path);
+    if (newPath === path) {
+      throw new Error(
+        `No package.json with name "root" found as a parent of ${topPath}`,
+      );
+    }
+    path = newPath;
+  }
+
+  throw new Error(
+    `Iteration limit reached when searching for root package.json at ${topPath}`,
+  );
+}

packages/config-loader/src/types.ts

@@ -14,4 +14,7 @@
  * limitations under the License.
  */
 
-export type AppConfig = any;
+export type LoadConfigOptions = {
+  // Config path, defaults to app-config.yaml in project root
+  configPath?: string;
+};

packages/config/.eslintrc.js

@@ -0,0 +1,6 @@
+module.exports = {
+  extends: [require.resolve('@backstage/cli/config/eslint')],
+  rules: {
+    'jest/expect-expect': 0,
+  },
+};

packages/config/README.md

@@ -0,0 +1,12 @@
+# @backstage/config
+
+This package provides a config API used by Backstage core, backend, and CLI.
+
+## Installation
+
+Do not install this package directly, it is an internal package used by [@backstage/core](https://www.npmjs.com/package/@backstage/core), [@backstage/cli](https://www.npmjs.com/package/@backstage/cli), and [@backstage/backend-common](https://www.npmjs.com/package/@backstage/backend-common). Depend on either of those instead.
+
+## Documentation
+
+- [Backstage Readme](https://github.com/spotify/backstage/blob/master/README.md)
+- [Backstage Documentation](https://github.com/spotify/backstage/blob/master/docs/README.md)

packages/config/package.json

@@ -0,0 +1,36 @@
+{
+  "name": "@backstage/config",
+  "description": "Config API used by Backstage core, backend, and CLI",
+  "version": "0.1.1-alpha.6",
+  "private": false,
+  "publishConfig": {
+    "access": "public"
+  },
+  "homepage": "https://backstage.io",
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/spotify/backstage",
+    "directory": "packages/config"
+  },
+  "keywords": [
+    "backstage"
+  ],
+  "license": "Apache-2.0",
+  "main": "src/index.ts",
+  "types": "src/index.ts",
+  "scripts": {
+    "build": "backstage-cli build",
+    "lint": "backstage-cli lint",
+    "test": "backstage-cli test",
+    "prepack": "backstage-cli prepack",
+    "postpack": "backstage-cli postpack",
+    "clean": "backstage-cli clean"
+  },
+  "devDependencies": {
+    "@types/jest": "^25.2.2",
+    "@types/node": "^12.0.0"
+  },
+  "files": [
+    "dist/**/*.{js,d.ts}"
+  ]
+}

packages/config/src/index.ts

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export type {
+  AppConfig,
+  Config,
+  JsonArray,
+  JsonObject,
+  JsonValue,
+} from './types';
+export { ConfigReader } from './reader';

packages/config/src/reader.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { ConfigReader } from './ConfigReader';
+import { ConfigReader } from './reader';
 
 const DATA = {
   zero: 0,

packages/config/src/reader.ts

@@ -14,15 +14,10 @@
  * limitations under the License.
  */
 
-import { ConfigApi, Config } from '../../definitions/ConfigApi';
-import { AppConfig } from '../../../app';
+import { AppConfig, Config, JsonValue, JsonObject } from './types';
 
 const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i;
 
-type JsonObject = { [key in string]: JsonValue };
-type JsonArray = JsonValue[];
-type JsonValue = JsonObject | JsonArray | number | string | boolean | null;
-
 function isObject(value: JsonValue | undefined): value is JsonObject {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
@@ -37,31 +32,14 @@ function typeOf(value: JsonValue | undefined): string {
   if (type === 'number' && isNaN(value as number)) {
     return 'nan';
   }
+  if (type === 'string' && value === '') {
+    return 'empty-string';
+  }
   return type;
 }
 
-function typeErrorMessage(key: string, got: string, wanted: string) {
-  return `Invalid type in config for key ${key}, got ${got}, wanted ${wanted}`;
-}
-
-function validateString(
-  key: string,
-  value: JsonValue | undefined,
-): value is string {
-  if (typeof value === 'string' && value.length > 0) {
-    return true;
-  }
-  if (value === '') {
-    throw new TypeError(typeErrorMessage(key, 'empty-string', 'string'));
-  }
-  if (value !== undefined) {
-    throw new TypeError(typeErrorMessage(key, typeOf(value), 'string'));
-  }
-  return false;
-}
-
-export class ConfigReader implements ConfigApi {
-  static nullReader = new ConfigReader({});
+export class ConfigReader implements Config {
+  private static readonly nullReader = new ConfigReader({});
 
   static fromConfigs(configs: AppConfig[]): ConfigReader {
     if (configs.length === 0) {
@@ -70,95 +48,112 @@ export class ConfigReader implements ConfigApi {
 
     // 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((previousReader, nextConfig) => {
+    return configs.reduceRight<ConfigReader>((previousReader, nextConfig) => {
       return new ConfigReader(nextConfig, previousReader);
-    }, undefined);
+    }, undefined!);
   }
 
   constructor(
     private readonly data: JsonObject,
-    private readonly fallback?: ConfigApi,
+    private readonly fallback?: ConfigReader,
   ) {}
 
-  getConfig(key: string): Config {
+  getConfig(key: string): ConfigReader {
     const value = this.readValue(key);
     const fallbackConfig = this.fallback?.getConfig(key);
     if (isObject(value)) {
       return new ConfigReader(value, fallbackConfig);
     }
     if (value !== undefined) {
-      throw new TypeError(typeErrorMessage(key, typeOf(value), 'object'));
+      throw new TypeError(
+        `Invalid type in config for key ${key}, got ${typeOf(
+          value,
+        )}, wanted object`,
+      );
     }
     return fallbackConfig ?? ConfigReader.nullReader;
   }
 
-  getConfigArray(key: string): Config[] {
-    const values = this.readValue(key);
-    if (Array.isArray(values)) {
-      return values.map((value, index) => {
-        if (isObject(value)) {
-          return new ConfigReader(value);
+  getConfigArray(key: string): ConfigReader[] {
+    const configs = this.readConfigValue<JsonObject[]>(key, values => {
+      if (!Array.isArray(values)) {
+        return { expected: 'object-array' };
+      }
+
+      for (const [index, value] of values.entries()) {
+        if (!isObject(value)) {
+          return { expected: 'object-array', value, key: `${key}[${index}]` };
         }
-        throw new TypeError(
-          typeErrorMessage(`${key}[${index}]`, typeOf(value), 'object'),
-        );
-      });
-    }
-    if (values !== undefined) {
-      throw new TypeError(
-        typeErrorMessage(key, typeOf(values), 'object-array'),
-      );
-    }
-    return this.fallback?.getConfigArray(key) ?? [];
+      }
+      return true;
+    });
+
+    return (configs ?? []).map(obj => new ConfigReader(obj));
   }
 
   getNumber(key: string): number | undefined {
-    const value = this.readValue(key);
-    if (typeof value === 'number' && !isNaN(value)) {
-      return value;
-    }
-    if (value !== undefined) {
-      throw new TypeError(typeErrorMessage(key, typeOf(value), 'number'));
-    }
-    return this.fallback?.getNumber(key);
+    return this.readConfigValue(
+      key,
+      value => typeof value === 'number' || { expected: 'number' },
+    );
   }
 
   getBoolean(key: string): boolean | undefined {
-    const value = this.readValue(key);
-    if (typeof value === 'boolean') {
-      return value;
-    }
-    if (value !== undefined) {
-      throw new TypeError(typeErrorMessage(key, typeOf(value), 'boolean'));
-    }
-    return this.fallback?.getBoolean(key);
+    return this.readConfigValue(
+      key,
+      value => typeof value === 'boolean' || { expected: 'boolean' },
+    );
   }
 
   getString(key: string): string | undefined {
-    const value = this.readValue(key);
-    if (validateString(key, value)) {
-      return value;
-    }
-    return this.fallback?.getString(key);
+    return this.readConfigValue(
+      key,
+      value =>
+        (typeof value === 'string' && value !== '') || { expected: 'string' },
+    );
   }
 
   getStringArray(key: string): string[] | undefined {
-    const values = this.readValue(key);
-    if (Array.isArray(values)) {
+    return this.readConfigValue(key, values => {
+      if (!Array.isArray(values)) {
+        return { expected: 'string-array' };
+      }
       for (const [index, value] of values.entries()) {
-        const iKey = `${key}[${index}]`;
-        if (!validateString(iKey, value)) {
-          throw new TypeError(typeErrorMessage(iKey, typeOf(value), 'string'));
+        if (typeof value !== 'string' || value === '') {
+          return { expected: 'string-array', value, key: `${key}[${index}]` };
         }
       }
-      return values as string[];
+      return true;
+    });
+  }
+
+  private readConfigValue<T extends JsonValue>(
+    key: string,
+    validate: (
+      value: JsonValue,
+    ) => { expected: string; value?: JsonValue; key?: string } | true,
+  ): T | undefined {
+    const value = this.readValue(key);
+
+    if (value === undefined) {
+      return this.fallback?.readConfigValue(key, validate);
     }
-    if (values !== undefined) {
-      throw new TypeError(
-        typeErrorMessage(key, typeOf(values), 'string-array'),
-      );
+    if (value !== undefined) {
+      const result = validate(value);
+      if (result !== true) {
+        const {
+          key: keyName = key,
+          value: theValue = value,
+          expected,
+        } = result;
+        const typeName = typeOf(theValue);
+        throw new TypeError(
+          `Invalid type in config for key ${keyName}, got ${typeName}, wanted ${expected}`,
+        );
+      }
     }
-    return this.fallback?.getStringArray(key);
+
+    return value as T;
   }
 
   private readValue(key: string): JsonValue | undefined {

packages/config/src/types.ts

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export type JsonObject = { [key in string]: JsonValue };
+export type JsonArray = JsonValue[];
+export type JsonValue =
+  | JsonObject
+  | JsonArray
+  | number
+  | string
+  | boolean
+  | null;
+
+export type AppConfig = JsonObject;
+
+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;
+};

packages/core-api/package.json

@@ -28,6 +28,7 @@
     "clean": "backstage-cli clean"
   },
   "dependencies": {
+    "@backstage/config": "^0.1.1-alpha.6",
     "@backstage/theme": "^0.1.1-alpha.6",
     "@material-ui/core": "^4.9.1",
     "@material-ui/icons": "^4.9.1",

packages/core-api/src/apis/implementations/ConfigApi/index.ts

@@ -14,4 +14,4 @@
  * limitations under the License.
  */
 
-export { ConfigReader } from './ConfigReader';
+export { ConfigReader } from '@backstage/config';

packages/core-api/src/app/types.ts

@@ -19,6 +19,7 @@ import { IconComponent, SystemIconKey, SystemIcons } from '../icons';
 import { BackstagePlugin } from '../plugin';
 import { ApiHolder } from '../apis';
 import { AppTheme } from '../apis/definitions';
+import { AppConfig } from '@backstage/config';
 
 export type BootErrorPageProps = {
   step: 'load-config';
@@ -31,11 +32,6 @@ export type AppComponents = {
   Progress: ComponentType<{}>;
 };
 
-/**
- * TBD
- */
-export type AppConfig = any;
-
 /**
  * A function that loads in the App config that will be accessible via the ConfigApi.
  *

packages/core/package.json

@@ -28,6 +28,7 @@
     "clean": "backstage-cli clean"
   },
   "dependencies": {
+    "@backstage/config": "^0.1.1-alpha.6",
     "@backstage/core-api": "0.1.1-alpha.6",
     "@backstage/theme": "^0.1.1-alpha.6",
     "@material-ui/core": "^4.9.1",

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

@@ -21,13 +21,13 @@ import privateExports, {
   defaultSystemIcons,
   BootErrorPageProps,
   AppConfigLoader,
-  AppConfig,
 } from '@backstage/core-api';
 import { BrowserRouter as Router } 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';
 
 const { PrivateAppImpl } = privateExports;