Move required configuration check to Backend

The `ensureRequiredOverlay` function is currently ran for any backend,
at this moment this causes no issue, since most configuration backends
are defined per single device type. However, with the option to modify splash
images, which is available for all device types, the function would add
unwanted configuration vars to the splash image configuration. Moving it
to the config txt backend solves this issue.

src/config/backends/backend.ts

@@ -64,4 +64,11 @@ export abstract class ConfigBackend {
 	public async initialise(): Promise<ConfigBackend> {
 		return this;
 	}
+
+	// Ensure that all required fields for device type are included in the
+	// provided configuration. It is expected to modify the configuration if
+	// necessary
+	public ensureRequiredConfig(_deviceType: string, conf: ConfigOptions) {
+		return conf;
+	}
 }

src/config/backends/config-txt.ts

@@ -148,4 +148,29 @@ export class ConfigTxt extends ConfigBackend {
 	public createConfigVarName(configName: string): string {
 		return ConfigTxt.bootConfigVarPrefix + configName;
 	}
+
+	// Ensure that the balena-fin overlay is defined in the target configuration
+	// overrides the parent
+	public ensureRequiredConfig(deviceType: string, conf: ConfigOptions) {
+		if (deviceType === 'fincm3') {
+			this.ensureDtoverlay(conf, 'balena-fin');
+		}
+
+		return conf;
+	}
+
+	// Modifies conf
+	private ensureDtoverlay(conf: ConfigOptions, field: string) {
+		if (conf.dtoverlay == null) {
+			conf.dtoverlay = [];
+		} else if (_.isString(conf.dtoverlay)) {
+			conf.dtoverlay = [conf.dtoverlay];
+		}
+		if (!_.includes(conf.dtoverlay, field)) {
+			conf.dtoverlay.push(field);
+		}
+		conf.dtoverlay = conf.dtoverlay.filter((s) => !_.isEmpty(s));
+
+		return conf;
+	}
 }

src/device-config.ts

@@ -13,7 +13,7 @@ import { DeviceStatus } from './types/state';
 import * as configUtils from './config/utils';
 import { SchemaTypeKey } from './config/schema-type';
 import { matchesAnyBootConfig } from './config/backends';
-import { ConfigOptions, ConfigBackend } from './config/backends/backend';
+import { ConfigBackend } from './config/backends/backend';
 import { Odmdata } from './config/backends/odmdata';
 
 const vpnServiceName = 'openvpn';
@@ -351,7 +351,7 @@ export function bootConfigChangeRequired(
 	const currentBootConfig = configUtils.envToBootConfig(configBackend, current);
 
 	// Some devices require specific overlays, here we apply them
-	ensureRequiredOverlay(deviceType, targetBootConfig);
+	configBackend.ensureRequiredConfig(deviceType, targetBootConfig);
 
 	// Search for any unsupported values
 	_.each(targetBootConfig, (value, key) => {
@@ -597,8 +597,8 @@ export async function setBootConfig(
 		'Apply boot config in progress',
 	);
 
-	// Ensure devices already have required overlays
-	ensureRequiredOverlay(await config.get('deviceType'), conf);
+	// Ensure the required target config is available
+	backend.ensureRequiredConfig(await config.get('deviceType'), conf);
 
 	try {
 		await backend.setBootConfig(conf);
@@ -662,28 +662,3 @@ function checkBoolChanged(
 ): boolean {
 	return checkTruthy(current[key]) !== checkTruthy(target[key]);
 }
-
-// Modifies conf
-// exported for tests
-export function ensureRequiredOverlay(deviceType: string, conf: ConfigOptions) {
-	if (deviceType === 'fincm3') {
-		ensureDtoverlay(conf, 'balena-fin');
-	}
-
-	return conf;
-}
-
-// Modifies conf
-function ensureDtoverlay(conf: ConfigOptions, field: string) {
-	if (conf.dtoverlay == null) {
-		conf.dtoverlay = [];
-	} else if (_.isString(conf.dtoverlay)) {
-		conf.dtoverlay = [conf.dtoverlay];
-	}
-	if (!_.includes(conf.dtoverlay, field)) {
-		conf.dtoverlay.push(field);
-	}
-	conf.dtoverlay = conf.dtoverlay.filter((s) => !_.isEmpty(s));
-
-	return conf;
-}

test/12-device-config.spec.ts

@@ -11,6 +11,7 @@ import { ConfigTxt } from '../src/config/backends/config-txt';
 import { Odmdata } from '../src/config/backends/odmdata';
 import { ConfigFs } from '../src/config/backends/config-fs';
 import * as constants from '../src/lib/constants';
+import * as config from '../src/config';
 
 import prepare = require('./lib/prepare');
 
@@ -171,6 +172,54 @@ describe('Device Backend Config', () => {
 		(child_process.exec as SinonStub).restore();
 	});
 
+	it('ensures required fields are written to config.txt', async () => {
+		stub(fsUtils, 'writeFileAtomic').resolves();
+		stub(child_process, 'exec').resolves();
+		stub(config, 'get').withArgs('deviceType').resolves('fincm3');
+		const current = {
+			HOST_CONFIG_initramfs: 'initramf.gz 0x00800000',
+			HOST_CONFIG_dtparam: '"i2c=on","audio=on"',
+			HOST_CONFIG_dtoverlay:
+				'"ads7846","lirc-rpi,gpio_out_pin=17,gpio_in_pin=13"',
+			HOST_CONFIG_foobar: 'baz',
+		};
+		const target = {
+			HOST_CONFIG_initramfs: 'initramf.gz 0x00800000',
+			HOST_CONFIG_dtparam: '"i2c=on","audio=off"',
+			HOST_CONFIG_dtoverlay: '"lirc-rpi,gpio_out_pin=17,gpio_in_pin=13"',
+			HOST_CONFIG_foobar: 'bat',
+			HOST_CONFIG_foobaz: 'bar',
+		};
+
+		expect(
+			// @ts-ignore accessing private value
+			deviceConfig.bootConfigChangeRequired(configTxtBackend, current, target),
+		).to.equal(true);
+
+		// @ts-ignore accessing private value
+		await deviceConfig.setBootConfig(configTxtBackend, target);
+		expect(child_process.exec).to.be.calledOnce;
+		expect(logSpy).to.be.calledTwice;
+		expect(logSpy.getCall(1).args[2]).to.equal('Apply boot config success');
+		expect(fsUtils.writeFileAtomic).to.be.calledWith(
+			'./test/data/mnt/boot/config.txt',
+			stripIndent`
+				initramfs initramf.gz 0x00800000
+				dtparam=i2c=on
+				dtparam=audio=off
+				dtoverlay=lirc-rpi,gpio_out_pin=17,gpio_in_pin=13
+				dtoverlay=balena-fin
+				foobar=bat
+				foobaz=bar
+			` + '\n', // add newline because stripIndent trims last newline
+		);
+
+		// Restore stubs
+		(fsUtils.writeFileAtomic as SinonStub).restore();
+		(child_process.exec as SinonStub).restore();
+		(config.get as SinonStub).restore();
+	});
+
 	it('accepts RESIN_ and BALENA_ variables', async () => {
 		return expect(
 			deviceConfig.formatConfigKeys({
@@ -258,12 +307,14 @@ describe('Device Backend Config', () => {
 
 	describe('Balena fin', () => {
 		it('should always add the balena-fin dtoverlay', () => {
-			expect(deviceConfig.ensureRequiredOverlay('fincm3', {})).to.deep.equal({
-				dtoverlay: ['balena-fin'],
-			});
+			expect(configTxtBackend.ensureRequiredConfig('fincm3', {})).to.deep.equal(
+				{
+					dtoverlay: ['balena-fin'],
+				},
+			);
 
 			expect(
-				deviceConfig.ensureRequiredOverlay('fincm3', {
+				configTxtBackend.ensureRequiredConfig('fincm3', {
 					test: '123',
 					test2: ['123'],
 					test3: ['123', '234'],
@@ -275,12 +326,12 @@ describe('Device Backend Config', () => {
 				dtoverlay: ['balena-fin'],
 			});
 			expect(
-				deviceConfig.ensureRequiredOverlay('fincm3', {
+				configTxtBackend.ensureRequiredConfig('fincm3', {
 					dtoverlay: 'test',
 				}),
 			).to.deep.equal({ dtoverlay: ['test', 'balena-fin'] });
 			expect(
-				deviceConfig.ensureRequiredOverlay('fincm3', {
+				configTxtBackend.ensureRequiredConfig('fincm3', {
 					dtoverlay: ['test'],
 				}),
 			).to.deep.equal({ dtoverlay: ['test', 'balena-fin'] });