diff --git a/src/config/configJson.ts b/src/config/configJson.ts index 44fd87e1..c1773cf3 100644 --- a/src/config/configJson.ts +++ b/src/config/configJson.ts @@ -3,7 +3,6 @@ import _ from 'lodash'; import * as constants from '../lib/constants'; import * as hostUtils from '../lib/host-utils'; -import * as osRelease from '../lib/os-release'; import { takeGlobalLockRO, takeGlobalLockRW } from '../lib/process-lock'; import type * as Schema from './schema'; @@ -12,17 +11,20 @@ export default class ConfigJsonConfigBackend { private readonly writeLockConfigJson: () => Bluebird.Disposer<() => void>; private readonly schema: Schema.Schema; + /** + * @deprecated configPath is only set by legacy tests + */ private readonly configPath?: string; private cache: { [key: string]: unknown } = {}; - private readonly init = _.once(async () => - Object.assign(this.cache, await this.read()), - ); + private readonly init = _.once(async () => { + Object.assign(this.cache, await this.read()); + }); public constructor(schema: Schema.Schema, configPath?: string) { - this.configPath = configPath; this.schema = schema; + this.configPath = configPath; this.writeLockConfigJson = () => takeGlobalLockRW('config.json').disposer((release) => release()); @@ -91,6 +93,12 @@ export default class ConfigJsonConfigBackend { return JSON.parse(await hostUtils.readFromBoot(filename, 'utf-8')); } + /** + * @deprecated Either read the config.json path from lib/constants, or + * pass a validated path to the constructor and fail if no path is passed. + * TODO: Remove this once api-binder tests are migrated. The only + * time configPath is passed to the constructor is in the legacy tests. + */ private async path(): Promise { // TODO: Remove this once api-binder tests are migrated. The only // time configPath is passed to the constructor is in the legacy tests. @@ -98,11 +106,6 @@ export default class ConfigJsonConfigBackend { return this.configPath; } - const osVersion = await osRelease.getOSVersion(constants.hostOSVersionPath); - if (osVersion == null) { - throw new Error('Failed to detect OS version!'); - } - // The default path in the boot partition return constants.configJsonPath; } diff --git a/test/integration/config/configJson.spec.ts b/test/integration/config/configJson.spec.ts new file mode 100644 index 00000000..39c8f1b9 --- /dev/null +++ b/test/integration/config/configJson.spec.ts @@ -0,0 +1,101 @@ +import { expect } from 'chai'; +import { testfs } from 'mocha-pod'; +import { promises as fs } from 'fs'; + +import ConfigJsonConfigBackend from '~/src/config/configJson'; +import { schema } from '~/src/config/schema'; + +describe('ConfigJsonConfigBackend', () => { + const CONFIG_PATH = '/mnt/boot/config.json'; + let configJsonConfigBackend: ConfigJsonConfigBackend; + + beforeEach(() => { + configJsonConfigBackend = new ConfigJsonConfigBackend(schema); + }); + + it('should get value for config.json key', async () => { + await testfs({ + [CONFIG_PATH]: JSON.stringify({ + apiEndpoint: 'foo', + }), + '/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'), + }).enable(); + + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo'); + + await testfs.restore(); + }); + + it('should set value for config.json key', async () => { + await testfs({ + [CONFIG_PATH]: JSON.stringify({ + apiEndpoint: 'foo', + }), + '/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'), + }).enable(); + + await configJsonConfigBackend.set({ + apiEndpoint: 'bar', + }); + + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('bar'); + + await testfs.restore(); + }); + + // The following test cases may be unnecessary as they test cases where another party + // writes to config.json directly (instead of through setting config vars on the API). + it('should get cached value even if actual value has changed', async () => { + await testfs({ + [CONFIG_PATH]: JSON.stringify({ + apiEndpoint: 'foo', + }), + '/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'), + }).enable(); + + // The cached value should be returned + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo'); + + // Change the value in the file + await fs.writeFile( + CONFIG_PATH, + JSON.stringify({ + apiEndpoint: 'bar', + }), + ); + + // Unintended behavior: the cached value should not be overwritten + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo'); + + await testfs.restore(); + }); + + it('should set value and refresh cache to equal new value', async () => { + await testfs({ + [CONFIG_PATH]: JSON.stringify({ + apiEndpoint: 'foo', + }), + '/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'), + }).enable(); + + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo'); + + await fs.writeFile( + CONFIG_PATH, + JSON.stringify({ + apiEndpoint: 'bar', + }), + ); + + // Unintended behavior: cached value should not have been updated + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo'); + + await configJsonConfigBackend.set({ + apiEndpoint: 'baz', + }); + + expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('baz'); + + await testfs.restore(); + }); +});