From 7298f8b26576042b05b37ec38fac7e6a8ac1f2b6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 8 Nov 2020 15:30:03 +0100 Subject: [PATCH] config-loader: remove support for conditionally reading secrets --- packages/backend-common/src/config.ts | 1 - packages/cli/src/lib/config.ts | 6 +- packages/config-loader/src/lib/reader.test.ts | 19 ------ packages/config-loader/src/lib/reader.ts | 4 -- .../config-loader/src/lib/secrets.test.ts | 1 - packages/config-loader/src/lib/types.ts | 1 - packages/config-loader/src/loader.test.ts | 59 ++----------------- packages/config-loader/src/loader.ts | 27 +-------- 8 files changed, 7 insertions(+), 111 deletions(-) diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index 69e46bd5b4..86beb65805 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -40,7 +40,6 @@ export async function loadBackendConfig(options: Options): Promise { env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'development', configRoot: paths.targetRoot, configPaths: configOpts.map(opt => resolvePath(opt)), - shouldReadSecrets: true, }); options.logger.info( diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index 5801632ea5..0ce19c80f5 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -18,14 +18,10 @@ import { loadConfig } from '@backstage/config-loader'; import { ConfigReader } from '@backstage/config'; import { paths } from './paths'; -export async function loadCliConfig( - configArgs: string[], - shouldReadSecrets: boolean = false, -) { +export async function loadCliConfig(configArgs: string[]) { const configPaths = configArgs.map(arg => paths.resolveTarget(arg)); const appConfigs = await loadConfig({ - shouldReadSecrets, env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'production', configRoot: paths.targetRoot, configPaths, diff --git a/packages/config-loader/src/lib/reader.test.ts b/packages/config-loader/src/lib/reader.test.ts index 6ccceef60d..a0a8495714 100644 --- a/packages/config-loader/src/lib/reader.test.ts +++ b/packages/config-loader/src/lib/reader.test.ts @@ -28,7 +28,6 @@ function memoryFiles(files: { [path: string]: string }) { const mockContext: ReaderContext = { env: {}, - skip: () => false, readFile: jest.fn(), readSecret: jest.fn(), }; @@ -179,22 +178,4 @@ describe('readConfigFile', () => { await expect(config).rejects.toThrow('Invalid secret at .app: NOPE'); }); - - it('should omit skipped values', async () => { - const readFile = memoryFiles({ - './app-config.yaml': 'app: { title: skip, name: include }', - }); - - const config = readConfigFile('./app-config.yaml', { - ...mockContext, - readFile, - skip: (path: string) => path === '.app.title', - readSecret: jest.fn() as ReadSecretFunc, - }); - - await expect(config).resolves.toEqual({ - context: 'app-config.yaml', - data: { app: { name: 'include' } }, - }); - }); }); diff --git a/packages/config-loader/src/lib/reader.ts b/packages/config-loader/src/lib/reader.ts index 8eadae0fe0..9eba58be97 100644 --- a/packages/config-loader/src/lib/reader.ts +++ b/packages/config-loader/src/lib/reader.ts @@ -37,10 +37,6 @@ export async function readConfigFile( obj: JsonValue, path: string, ): Promise { - if (ctx.skip(path)) { - return undefined; - } - if (typeof obj !== 'object') { return obj; } else if (obj === null) { diff --git a/packages/config-loader/src/lib/secrets.test.ts b/packages/config-loader/src/lib/secrets.test.ts index cfc4150d68..d80ada193b 100644 --- a/packages/config-loader/src/lib/secrets.test.ts +++ b/packages/config-loader/src/lib/secrets.test.ts @@ -21,7 +21,6 @@ const ctx: ReaderContext = { env: { SECRET: 'my-secret', }, - skip: () => false, readSecret: jest.fn(), async readFile(path) { const content = ({ diff --git a/packages/config-loader/src/lib/types.ts b/packages/config-loader/src/lib/types.ts index 02b8a9d053..e189aef20d 100644 --- a/packages/config-loader/src/lib/types.ts +++ b/packages/config-loader/src/lib/types.ts @@ -28,7 +28,6 @@ export type SkipFunc = (path: string) => boolean; */ export type ReaderContext = { env: { [name in string]?: string }; - skip: SkipFunc; readFile: ReadFileFunc; readSecret: ReadSecretFunc; }; diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index 3b0d8e6e92..a9857a784f 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/packages/config-loader/src/loader.test.ts @@ -44,47 +44,6 @@ describe('loadConfig', () => { configRoot: '/root', configPaths: [], env: 'production', - shouldReadSecrets: false, - }), - ).resolves.toEqual([ - { - context: 'app-config.yaml', - data: { - app: { - title: 'Example App', - }, - }, - }, - ]); - }); - - it('loads config without secrets', async () => { - await expect( - loadConfig({ - configRoot: '/root', - configPaths: ['/root/app-config.yaml'], - env: 'production', - shouldReadSecrets: false, - }), - ).resolves.toEqual([ - { - context: 'app-config.yaml', - data: { - app: { - title: 'Example App', - }, - }, - }, - ]); - }); - - it('loads config with secrets', async () => { - await expect( - loadConfig({ - configRoot: '/root', - configPaths: ['/root/app-config.yaml'], - env: 'production', - shouldReadSecrets: true, }), ).resolves.toEqual([ { @@ -99,16 +58,12 @@ describe('loadConfig', () => { ]); }); - it('loads development config without secrets', async () => { + it('loads config with secrets', async () => { await expect( loadConfig({ configRoot: '/root', - configPaths: [ - '/root/app-config.yaml', - '/root/app-config.development.yaml', - ], - env: 'development', - shouldReadSecrets: false, + configPaths: ['/root/app-config.yaml'], + env: 'production', }), ).resolves.toEqual([ { @@ -116,15 +71,10 @@ describe('loadConfig', () => { data: { app: { title: 'Example App', + sessionKey: 'abc123', }, }, }, - { - context: 'app-config.development.yaml', - data: { - app: {}, - }, - }, ]); }); @@ -137,7 +87,6 @@ describe('loadConfig', () => { '/root/app-config.development.yaml', ], env: 'development', - shouldReadSecrets: true, }), ).resolves.toEqual([ { diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 00414f86eb..a647367469 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -28,18 +28,13 @@ export type LoadConfigOptions = { // TODO(Rugvip): This will be removed in the future, but for now we use it to warn about possible mistakes. env: string; - - // Whether to read secrets or omit them, defaults to false. - shouldReadSecrets?: boolean; }; class Context { constructor( private readonly options: { - secretPaths: Set; env: { [name in string]?: string }; rootPath: string; - shouldReadSecrets: boolean; }, ) {} @@ -47,35 +42,21 @@ class Context { return this.options.env; } - skip(path: string): boolean { - if (this.options.shouldReadSecrets) { - return false; - } - return this.options.secretPaths.has(path); - } - async readFile(path: string): Promise { return fs.readFile(resolvePath(this.options.rootPath, path), 'utf8'); } async readSecret( - path: string, + _path: string, desc: JsonObject, ): Promise { - this.options.secretPaths.add(path); - if (!this.options.shouldReadSecrets) { - return undefined; - } - return readSecret(desc, this); } } -type LoadedConfig = AppConfig[]; - export async function loadConfig( options: LoadConfigOptions, -): Promise { +): Promise { const configs = []; const { configRoot } = options; const configPaths = options.configPaths.slice(); @@ -102,8 +83,6 @@ export async function loadConfig( } try { - const secretPaths = new Set(); - for (const configPath of configPaths) { if (!isAbsolute(configPath)) { throw new Error(`Config load path is not absolute: '${configPath}'`); @@ -111,10 +90,8 @@ export async function loadConfig( const config = await readConfigFile( configPath, new Context({ - secretPaths, env: process.env, rootPath: dirname(configPath), - shouldReadSecrets: Boolean(options.shouldReadSecrets), }), );