From 74ceaa64175d5eecd33577a735ce447081b7f0bc Mon Sep 17 00:00:00 2001 From: Matto Date: Thu, 21 Oct 2021 18:20:36 +1100 Subject: [PATCH] Remove the dependency on Etag headers for reloading config Signed-off-by: Matto --- docs/conf/writing.md | 5 +- packages/cli/package.json | 2 +- packages/config-loader/package.json | 4 +- packages/config-loader/src/loader.test.ts | 79 +++++++++-------- packages/config-loader/src/loader.ts | 102 ++++++---------------- yarn.lock | 7 -- 6 files changed, 73 insertions(+), 126 deletions(-) diff --git a/docs/conf/writing.md b/docs/conf/writing.md index 86b844484b..a0e29820ed 100644 --- a/docs/conf/writing.md +++ b/docs/conf/writing.md @@ -1,6 +1,7 @@ --- -id: writing title: Writing Backstage Configuration Files description: Documentation on Writing -Backstage Configuration Files +id: writing +title: Writing Backstage Configuration Files +description: Documentation on Writing Backstage Configuration Files --- ## File Format diff --git a/packages/cli/package.json b/packages/cli/package.json index 47475780a6..519b45a5ad 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -34,7 +34,6 @@ "@backstage/integration": "^0.6.5", "@backstage/config": "^0.1.10", "@backstage/config-loader": "^0.6.8", - "@backstage/backend-common": "^0.9.4", "@hot-loader/react-dom": "^16.13.0", "@lerna/package-graph": "^4.0.0", "@lerna/project": "^4.0.0", @@ -119,6 +118,7 @@ "yn": "^4.0.0" }, "devDependencies": { + "@backstage/backend-common": "^0.9.4", "@backstage/config": "^0.1.10", "@backstage/core-components": "^0.5.0", "@backstage/core-plugin-api": "^0.1.8", diff --git a/packages/config-loader/package.json b/packages/config-loader/package.json index d20c77bedc..80e4121971 100644 --- a/packages/config-loader/package.json +++ b/packages/config-loader/package.json @@ -41,7 +41,6 @@ "json-schema": "^0.3.0", "json-schema-merge-allof": "^0.8.1", "typescript-json-schema": "^0.50.1", - "uuid": "^8.3.2", "yaml": "^1.9.2", "yup": "^0.29.3", "node-fetch": "2.6.5" @@ -53,8 +52,7 @@ "@types/node": "^14.14.32", "@types/yup": "^0.29.8", "mock-fs": "^5.1.0", - "fetch-mock-jest": "1.5.1", - "fetch-mock": "^9.11.0" + "msw": "^0.29.0" }, "files": [ "dist" diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index 190904a870..e8b5a8828b 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/packages/config-loader/src/loader.test.ts @@ -18,33 +18,47 @@ import { AppConfig } from '@backstage/config'; import { loadConfig } from './loader'; import mockFs from 'mock-fs'; import fs from 'fs-extra'; -import { v4 as uuidv4 } from 'uuid'; - -const fetchMock = require('fetch-mock').sandbox(); -const nodeFetch = require('node-fetch'); - -nodeFetch.default = fetchMock; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; describe('loadConfig', () => { + const server = setupServer(); + const initialLoaderHandler = rest.get( + `https://some.domain.io/app-config.yaml`, + (_req, res, ctx) => { + return res( + ctx.body( + `app: + title: Remote Example App + sessionKey: 'abc123' + escaped: \$\${Escaped} + `, + ), + ); + }, + ); + + const reloadHandler = rest.get( + `https://some.domain.io/app-config.yaml`, + (_req, res, ctx) => { + return res( + ctx.body( + `app: + title: NEW ReMOTe ExaMPLe App + sessionKey: 'abc123' + escaped: \$\${Escaped} + `, + ), + ); + }, + ); + + beforeAll(() => server.listen()); + beforeEach(() => { process.env.MY_SECRET = 'is-secret'; process.env.SUBSTITUTE_ME = 'substituted'; - fetchMock.mock( - { - url: 'https://some.domain.io/app-config.yaml', - method: 'GET', - }, - { - body: `app: - title: Remote Example App - sessionKey: 'abc123' - escaped: \$\${Escaped} - `, - headers: { ETag: uuidv4().toString() }, - }, - ); - mockFs({ '/root/app-config.yaml': ` app: @@ -87,10 +101,12 @@ describe('loadConfig', () => { }); afterEach(() => { - fetchMock.restore(); mockFs.restore(); + server.resetHandlers(); }); + afterAll(() => server.close()); + it('load config from default path', async () => { await expect( loadConfig({ @@ -113,6 +129,8 @@ describe('loadConfig', () => { }); it('load config from remote path', async () => { + server.use(initialLoaderHandler); + const configUrl = 'https://some.domain.io/app-config.yaml'; await expect( @@ -267,6 +285,8 @@ describe('loadConfig', () => { }); it('watches remote config urls', async () => { + server.use(initialLoaderHandler); + const onChange = defer(); const stopSignal = defer(); @@ -296,20 +316,7 @@ describe('loadConfig', () => { }, ]); - fetchMock.mock( - { - url: 'https://some.domain.io/app-config.yaml', - }, - { - body: `app: - title: NEW ReMOTe ExaMPLe App - sessionKey: 'abc123' - escaped: \$\${Escaped} - `, - headers: { ETag: uuidv4().toString() }, - }, - { overwriteRoutes: true }, - ); + server.use(reloadHandler); await expect(onChange.promise).resolves.toEqual([ { diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index fb8d0aab33..b853d30d8a 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -50,27 +50,6 @@ export type Remote = { reloadIntervalSeconds: number; }; -export type RemoteConfigProp = { - /** - * URL of the remote config - */ - url: string; - - /** - * Contents of the remote config - */ - content: string | null; - - /** - * An optional new ETag header value. Used when checking for updated config. - */ - newETag?: string; - - /** - * An optional old ETag header value. Used when checking for updated config - */ - oldETag?: string; -}; /** @public */ export type LoadConfigOptions = { // The root directory of the config loading context. Used to find default configs. @@ -100,8 +79,6 @@ export type LoadConfigOptions = { watch?: Watch; }; -const HTTP_RESPONSE_HEADER_ETAG = 'ETag'; - /** * Load configuration data. * @@ -117,18 +94,15 @@ export async function loadConfig( .filter((e): e is { path: string } => e.hasOwnProperty('path')) .map(configTarget => configTarget.path); - let configUrls: string[] = options.configTargets + const configUrls: string[] = options.configTargets .slice() .filter((e): e is { url: string } => e.hasOwnProperty('url')) .map(configTarget => configTarget.url); - const remoteConfigProps: RemoteConfigProp[] = []; - if (remote === undefined && configUrls.length > 0) { - console.warn( + throw new Error( `Remote config detected, however, this feature is turned off. Remote config will be ignored.`, ); - configUrls = []; } // If no paths are provided, we default to reading @@ -173,47 +147,32 @@ export async function loadConfig( const loadRemoteConfigFiles = async () => { const configs: AppConfig[] = []; - const readConfigFromUrl = async (remoteConfigProp: RemoteConfigProp) => { - const response = await fetch(remoteConfigProp.url); + const readConfigFromUrl = async (url: string) => { + const response = await fetch(url); if (!response.ok) { - throw new Error( - `Could not read config file at ${remoteConfigProp.url}`, - ); + throw new Error(`Could not read config file at ${url}`); } - remoteConfigProp.oldETag = remoteConfigProp.newETag ?? undefined; - remoteConfigProp.newETag = - response.headers.get(HTTP_RESPONSE_HEADER_ETAG) ?? undefined; - remoteConfigProp.content = await response.text(); - - return remoteConfigProp; + return await response.text(); }; for (let i = 0; i < configUrls.length; i++) { - const remoteConfigProp = await readConfigFromUrl({ - url: configUrls[i], - content: null, - }); - - if (!isValidUrl(remoteConfigProp.url)) { - throw new Error( - `Config load path is not valid: '${remoteConfigProp.url}'`, - ); + const configUrl = configUrls[i]; + if (!isValidUrl(configUrl)) { + throw new Error(`Config load path is not valid: '${configUrl}'`); } - const dir = configRoot; - if (!remoteConfigProp.content) { + const remoteConfigContent = await readConfigFromUrl(configUrl); + if (!remoteConfigContent) { throw new Error(`Config is not valid`); } - const input = yaml.parse(remoteConfigProp.content); + const configYaml = yaml.parse(remoteConfigContent); const substitutionTransform = createSubstitutionTransform(env); - const data = await applyConfigTransforms(dir, input, [ + const data = await applyConfigTransforms(configRoot, configYaml, [ substitutionTransform, ]); - configs.push({ data, context: remoteConfigProp.url }); - - remoteConfigProps.push(remoteConfigProp); + configs.push({ data, context: configUrl }); } return configs; @@ -269,17 +228,12 @@ export async function loadConfig( }; const watchRemoteConfig = (watchProp: Watch, remoteProp: Remote) => { - const hasConfigChanged = async (remoteConfigProp: RemoteConfigProp) => { - const requestProps = { method: 'HEAD' }; - const { headers } = await fetch(remoteConfigProp.url, requestProps); - remoteConfigProp.oldETag = remoteConfigProp.newETag ?? undefined; - remoteConfigProp.newETag = - headers.get(HTTP_RESPONSE_HEADER_ETAG) ?? undefined; - + const hasConfigChanged = async ( + oldRemoteConfigs: AppConfig[], + newRemoteConfigs: AppConfig[], + ) => { return ( - remoteConfigProp.oldETag !== undefined && - remoteConfigProp.newETag !== undefined && - remoteConfigProp.oldETag !== remoteConfigProp.newETag + JSON.stringify(oldRemoteConfigs) !== JSON.stringify(newRemoteConfigs) ); }; @@ -287,18 +241,12 @@ export async function loadConfig( try { handle = setInterval(async () => { console.info(`Checking for config update`); - for (const remoteConfigProp of remoteConfigProps) { - if (await hasConfigChanged(remoteConfigProp)) { - console.info(`Remote config change, reloading config ...`); - const newRemoteConfigs = await loadRemoteConfigFiles(); - watchProp.onChange([ - ...newRemoteConfigs, - ...fileConfigs, - ...envConfigs, - ]); - console.info(`Remote config reloaded`); - break; - } + const newRemoteConfigs = await loadRemoteConfigFiles(); + if (await hasConfigChanged(remoteConfigs, newRemoteConfigs)) { + remoteConfigs = newRemoteConfigs; + console.info(`Remote config change, reloading config ...`); + watchProp.onChange([...remoteConfigs, ...fileConfigs, ...envConfigs]); + console.info(`Remote config reloaded`); } }, remoteProp.reloadIntervalSeconds * 1000); } catch (error) { diff --git a/yarn.lock b/yarn.lock index 87991e1bb5..00c91f7783 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14042,13 +14042,6 @@ fetch-blob@2.1.2: resolved "https://registry.npmjs.org/fetch-blob/-/fetch-blob-2.1.2.tgz#a7805db1361bd44c1ef62bb57fb5fe8ea173ef3c" integrity sha512-YKqtUDwqLyfyMnmbw8XD6Q8j9i/HggKtPEI+pZ1+8bvheBu78biSmNaXWusx1TauGqtUUGx/cBb1mKdq2rLYow== -fetch-mock-jest@1.5.1: - version "1.5.1" - resolved "https://registry.npmjs.org/fetch-mock-jest/-/fetch-mock-jest-1.5.1.tgz#0e13df990d286d9239e284f12b279ed509bf53cd" - integrity sha512-+utwzP8C+Pax1GSka3nFXILWMY3Er2L+s090FOgqVNrNCPp0fDqgXnAHAJf12PLHi0z4PhcTaZNTz8e7K3fjqQ== - dependencies: - fetch-mock "^9.11.0" - fetch-mock@^9.11.0: version "9.11.0" resolved "https://registry.npmjs.org/fetch-mock/-/fetch-mock-9.11.0.tgz#371c6fb7d45584d2ae4a18ee6824e7ad4b637a3f"