Move /v1 routes in deviceState.router to v1.ts

Signed-off-by: Christina Ying Wang <christina@balena.io>

src/device-api/index.ts

@@ -63,7 +63,6 @@ export class SupervisorAPI {
 		this.api.post(
 			'/v1/regenerate-api-key',
 			async (req: apiKeys.AuthorizedRequest, res) => {
-				await deviceState.initialized();
 				await apiKeys.initialized();
 
 				// check if we're updating the cloud API key

src/device-api/v1.ts

@@ -6,15 +6,21 @@ import { AuthorizedRequest } from './api-keys';
 import * as eventTracker from '../event-tracker';
 import { isReadyForUpdates } from '../api-binder';
 import * as config from '../config';
+import * as deviceState from '../device-state';
+
 import * as constants from '../lib/constants';
 import { checkInt, checkTruthy } from '../lib/validation';
 import log from '../lib/supervisor-console';
+import { UpdatesLockedError } from '../lib/errors';
+import * as hostConfig from '../host-config';
 import * as applicationManager from '../compose/application-manager';
 import { generateStep } from '../compose/composition-steps';
 import * as commitStore from '../compose/commit';
 import { getApp } from '../device-state/db-format';
 import * as TargetState from '../device-state/target-state';
 
+const disallowedHostConfigPatchFields = ['local_ip', 'local_port'];
+
 export const router = express.Router();
 
 router.post('/v1/restart', (req: AuthorizedRequest, res, next) => {
@@ -118,6 +124,30 @@ const createV1StopOrStartHandler = (action: 'start' | 'stop') =>
 router.post('/v1/apps/:appId/stop', createV1StopOrStartHandler('stop'));
 router.post('/v1/apps/:appId/start', createV1StopOrStartHandler('start'));
 
+const rebootOrShutdown = async (
+	req: express.Request,
+	res: express.Response,
+	action: deviceState.DeviceStateStepTarget,
+) => {
+	const override = await config.get('lockOverride');
+	const force = checkTruthy(req.body.force) || override;
+	try {
+		const response = await deviceState.executeStepAction({ action }, { force });
+		res.status(202).json(response);
+	} catch (e: any) {
+		const status = e instanceof UpdatesLockedError ? 423 : 500;
+		res.status(status).json({
+			Data: '',
+			Error: (e != null ? e.message : undefined) || e || 'Unknown error',
+		});
+	}
+};
+
+router.post('/v1/reboot', (req, res) => rebootOrShutdown(req, res, 'reboot'));
+router.post('/v1/shutdown', (req, res) =>
+	rebootOrShutdown(req, res, 'shutdown'),
+);
+
 router.get('/v1/apps/:appId', async (req: AuthorizedRequest, res, next) => {
 	const appId = checkInt(req.params.appId);
 	eventTracker.track('GET app (v1)', { appId });
@@ -211,3 +241,116 @@ router.post('/v1/update', (req, res, next) => {
 		res.sendStatus(202);
 	}
 });
+
+router.get('/v1/device/host-config', (_req, res) =>
+	hostConfig
+		.get()
+		.then((conf) => res.json(conf))
+		.catch((err) =>
+			res.status(503).send(err?.message ?? err ?? 'Unknown error'),
+		),
+);
+
+router.patch('/v1/device/host-config', async (req, res) => {
+	// Because v1 endpoints are legacy, and this endpoint might already be used
+	// by multiple users, adding too many throws might have unintended side effects.
+	// Thus we're simply logging invalid fields and allowing the request to continue.
+
+	try {
+		if (!req.body.network) {
+			log.warn("Key 'network' must exist in PATCH body");
+			// If network does not exist, skip all field validation checks below
+			throw new Error();
+		}
+
+		const { proxy } = req.body.network;
+
+		// Validate proxy fields, if they exist
+		if (proxy && Object.keys(proxy).length) {
+			const blacklistedFields = Object.keys(proxy).filter((key) =>
+				disallowedHostConfigPatchFields.includes(key),
+			);
+
+			if (blacklistedFields.length > 0) {
+				log.warn(`Invalid proxy field(s): ${blacklistedFields.join(', ')}`);
+			}
+
+			if (
+				proxy.type &&
+				!constants.validRedsocksProxyTypes.includes(proxy.type)
+			) {
+				log.warn(
+					`Invalid redsocks proxy type, must be one of ${constants.validRedsocksProxyTypes.join(
+						', ',
+					)}`,
+				);
+			}
+
+			if (proxy.noProxy && !Array.isArray(proxy.noProxy)) {
+				log.warn('noProxy field must be an array of addresses');
+			}
+		}
+	} catch (e) {
+		/* noop */
+	}
+
+	try {
+		// If hostname is an empty string, return first 7 digits of device uuid
+		if (req.body.network?.hostname === '') {
+			const uuid = await config.get('uuid');
+			req.body.network.hostname = uuid?.slice(0, 7);
+		}
+		const lockOverride = await config.get('lockOverride');
+		await hostConfig.patch(
+			req.body,
+			checkTruthy(req.body.force) || lockOverride,
+		);
+		res.status(200).send('OK');
+	} catch (err: any) {
+		// TODO: We should be able to throw err if it's UpdatesLockedError
+		// and the error middleware will handle it, but this doesn't work in
+		// the test environment. Fix this when fixing API tests.
+		if (err instanceof UpdatesLockedError) {
+			return res.status(423).send(err?.message ?? err);
+		}
+		res.status(503).send(err?.message ?? err ?? 'Unknown error');
+	}
+});
+
+router.get('/v1/device', async (_req, res) => {
+	try {
+		const state = await deviceState.getLegacyState();
+		const stateToSend = _.pick(state.local, [
+			'api_port',
+			'ip_address',
+			'os_version',
+			'mac_address',
+			'supervisor_version',
+			'update_pending',
+			'update_failed',
+			'update_downloaded',
+		]) as Dictionary<unknown>;
+		if (state.local?.is_on__commit != null) {
+			stateToSend.commit = state.local.is_on__commit;
+		}
+		const service = _.toPairs(
+			_.toPairs(state.local?.apps)[0]?.[1]?.services,
+		)[0]?.[1];
+
+		if (service != null) {
+			stateToSend.status = service.status;
+			if (stateToSend.status === 'Running') {
+				stateToSend.status = 'Idle';
+			}
+			stateToSend.download_progress = service.download_progress;
+		}
+		res.json(stateToSend);
+	} catch (e: any) {
+		res.status(500).json({
+			Data: '',
+			Error: (e != null ? e.message : undefined) || e || 'Unknown error',
+		});
+	}
+});
+
+router.use(applicationManager.router);

src/device-state.ts

@@ -1,25 +1,20 @@
 import * as Bluebird from 'bluebird';
 import { stripIndent } from 'common-tags';
 import { EventEmitter } from 'events';
-import * as express from 'express';
 import * as _ from 'lodash';
 import StrictEventEmitter from 'strict-event-emitter-types';
 import { isRight } from 'fp-ts/lib/Either';
 import Reporter from 'io-ts-reporters';
-
 import prettyMs = require('pretty-ms');
 
 import * as config from './config';
 import * as db from './db';
 import * as logger from './logger';
 
-import {
-	CompositionStepT,
-	CompositionStepAction,
-} from './compose/composition-steps';
-import { loadTargetFromFile } from './device-state/preload';
 import * as globalEventBus from './event-bus';
-import * as hostConfig from './host-config';
+import * as network from './network';
+import * as deviceConfig from './device-config';
+
 import constants = require('./lib/constants');
 import * as dbus from './lib/dbus';
 import {
@@ -28,14 +23,14 @@ import {
 	UpdatesLockedError,
 } from './lib/errors';
 import * as updateLock from './lib/update-lock';
-import * as validation from './lib/validation';
+import * as dbFormat from './device-state/db-format';
-import * as network from './network';
+import * as apiKeys from './device-api/api-keys';
-
+import * as sysInfo from './lib/system-info';
+import { log } from './lib/supervisor-console';
+import { loadTargetFromFile } from './device-state/preload';
 import * as applicationManager from './compose/application-manager';
 import * as commitStore from './compose/commit';
-import * as deviceConfig from './device-config';
+
-import { ConfigStep } from './device-config';
-import { log } from './lib/supervisor-console';
 import {
 	DeviceLegacyState,
 	InstancedDeviceState,
@@ -44,11 +39,10 @@ import {
 	DeviceReport,
 	AppState,
 } from './types';
-import * as dbFormat from './device-state/db-format';
+import type {
-import * as apiKeys from './device-api/api-keys';
+	CompositionStepT,
-import * as sysInfo from './lib/system-info';
+	CompositionStepAction,
-
+} from './compose/composition-steps';
-const disallowedHostConfigPatchFields = ['local_ip', 'local_port'];
 
 function parseTargetState(state: unknown): TargetState {
 	const res = TargetState.decode(state);
@@ -61,149 +55,6 @@ function parseTargetState(state: unknown): TargetState {
 	throw new TargetStateError(errors.join('\n'));
 }
 
-// TODO (refactor): This shouldn't be here, and instead should be part of the other
-// device api stuff in ./device-api
-function createDeviceStateRouter() {
-	router = express.Router();
-
-	const rebootOrShutdown = async (
-		req: express.Request,
-		res: express.Response,
-		action: DeviceStateStepTarget,
-	) => {
-		const override = await config.get('lockOverride');
-		const force = validation.checkTruthy(req.body.force) || override;
-		try {
-			const response = await executeStepAction({ action }, { force });
-			res.status(202).json(response);
-		} catch (e: any) {
-			const status = e instanceof UpdatesLockedError ? 423 : 500;
-			res.status(status).json({
-				Data: '',
-				Error: (e != null ? e.message : undefined) || e || 'Unknown error',
-			});
-		}
-	};
-	router.post('/v1/reboot', (req, res) => rebootOrShutdown(req, res, 'reboot'));
-	router.post('/v1/shutdown', (req, res) =>
-		rebootOrShutdown(req, res, 'shutdown'),
-	);
-
-	router.get('/v1/device/host-config', (_req, res) =>
-		hostConfig
-			.get()
-			.then((conf) => res.json(conf))
-			.catch((err) =>
-				res.status(503).send(err?.message ?? err ?? 'Unknown error'),
-			),
-	);
-
-	router.patch('/v1/device/host-config', async (req, res) => {
-		// Because v1 endpoints are legacy, and this endpoint might already be used
-		// by multiple users, adding too many throws might have unintended side effects.
-		// Thus we're simply logging invalid fields and allowing the request to continue.
-
-		try {
-			if (!req.body.network) {
-				log.warn("Key 'network' must exist in PATCH body");
-				// If network does not exist, skip all field validation checks below
-				throw new Error();
-			}
-
-			const { proxy } = req.body.network;
-
-			// Validate proxy fields, if they exist
-			if (proxy && Object.keys(proxy).length) {
-				const blacklistedFields = Object.keys(proxy).filter((key) =>
-					disallowedHostConfigPatchFields.includes(key),
-				);
-
-				if (blacklistedFields.length > 0) {
-					log.warn(`Invalid proxy field(s): ${blacklistedFields.join(', ')}`);
-				}
-
-				if (
-					proxy.type &&
-					!constants.validRedsocksProxyTypes.includes(proxy.type)
-				) {
-					log.warn(
-						`Invalid redsocks proxy type, must be one of ${constants.validRedsocksProxyTypes.join(
-							', ',
-						)}`,
-					);
-				}
-
-				if (proxy.noProxy && !Array.isArray(proxy.noProxy)) {
-					log.warn('noProxy field must be an array of addresses');
-				}
-			}
-		} catch (e) {
-			/* noop */
-		}
-
-		try {
-			// If hostname is an empty string, return first 7 digits of device uuid
-			if (req.body.network?.hostname === '') {
-				const uuid = await config.get('uuid');
-				req.body.network.hostname = uuid?.slice(0, 7);
-			}
-			const lockOverride = await config.get('lockOverride');
-			await hostConfig.patch(
-				req.body,
-				validation.checkTruthy(req.body.force) || lockOverride,
-			);
-			res.status(200).send('OK');
-		} catch (err: any) {
-			// TODO: We should be able to throw err if it's UpdatesLockedError
-			// and the error middleware will handle it, but this doesn't work in
-			// the test environment. Fix this when fixing API tests.
-			if (err instanceof UpdatesLockedError) {
-				return res.status(423).send(err?.message ?? err);
-			}
-			res.status(503).send(err?.message ?? err ?? 'Unknown error');
-		}
-	});
-
-	router.get('/v1/device', async (_req, res) => {
-		try {
-			const state = await getLegacyState();
-			const stateToSend = _.pick(state.local, [
-				'api_port',
-				'ip_address',
-				'os_version',
-				'mac_address',
-				'supervisor_version',
-				'update_pending',
-				'update_failed',
-				'update_downloaded',
-			]) as Dictionary<unknown>;
-			if (state.local?.is_on__commit != null) {
-				stateToSend.commit = state.local.is_on__commit;
-			}
-			const service = _.toPairs(
-				_.toPairs(state.local?.apps)[0]?.[1]?.services,
-			)[0]?.[1];
-
-			if (service != null) {
-				stateToSend.status = service.status;
-				if (stateToSend.status === 'Running') {
-					stateToSend.status = 'Idle';
-				}
-				stateToSend.download_progress = service.download_progress;
-			}
-			res.json(stateToSend);
-		} catch (e: any) {
-			res.status(500).json({
-				Data: '',
-				Error: (e != null ? e.message : undefined) || e || 'Unknown error',
-			});
-		}
-	});
-
-	router.use(applicationManager.router);
-	return router;
-}
-
 interface DeviceStateEvents {
 	error: Error;
 	change: void;
@@ -234,7 +85,7 @@ export const removeListener: typeof events['removeListener'] =
 export const removeAllListeners: typeof events['removeAllListeners'] =
 	events.removeAllListeners.bind(events);
 
-type DeviceStateStepTarget = 'reboot' | 'shutdown' | 'noop';
+export type DeviceStateStepTarget = 'reboot' | 'shutdown' | 'noop';
 
 type PossibleStepTargets = CompositionStepAction | DeviceStateStepTarget;
 type DeviceStateStep<T extends PossibleStepTargets> =
@@ -244,7 +95,7 @@ type DeviceStateStep<T extends PossibleStepTargets> =
 	| { action: 'shutdown' }
 	| { action: 'noop' }
 	| CompositionStepT<T extends CompositionStepAction ? T : never>
-	| ConfigStep;
+	| deviceConfig.ConfigStep;
 
 let currentVolatile: DeviceReport = {};
 const writeLock = updateLock.writeLock;
@@ -264,8 +115,6 @@ let applyInProgress = false;
 export let connected: boolean;
 export let lastSuccessfulUpdate: number | null = null;
 
-export let router: express.Router;
-
 events.on('error', (err) => log.error('deviceState error: ', err));
 events.on('apply-target-state-end', function (err) {
 	if (err != null) {
@@ -286,7 +135,6 @@ export const initialized = _.once(async () => {
 	await applicationManager.initialized();
 
 	applicationManager.on('change', (d) => reportCurrentState(d));
-	createDeviceStateRouter();
 
 	config.on('change', (changedConfig) => {
 		if (changedConfig.loggingEnabled != null) {
@@ -713,7 +561,7 @@ export async function executeStepAction<T extends PossibleStepTargets>(
 	}: { force?: boolean; initial?: boolean; skipLock?: boolean },
 ) {
 	if (deviceConfig.isValidAction(step.action)) {
-		await deviceConfig.executeStepAction(step as ConfigStep, {
+		await deviceConfig.executeStepAction(step as deviceConfig.ConfigStep, {
 			initial,
 		});
 	} else if (_.includes(applicationManager.validActions, step.action)) {

src/supervisor.ts

@@ -61,14 +61,13 @@ export class Supervisor {
 			await normaliseLegacyDatabase();
 		}
 
-		// Start the state engine, the device API and API binder
+		// Start the state engine, the device API and API binder in parallel
-		// in parallel
 		await Promise.all([
 			deviceState.loadInitialState(),
 			(() => {
 				log.info('Starting API server');
 				this.api = new SupervisorAPI({
-					routers: [v1.router, deviceState.router],
+					routers: [v1.router],
 					healthchecks: [apiBinder.healthcheck, deviceState.healthcheck],
 				});
 				this.api.listen(conf.listenPort, conf.apiTimeout);

test/lib/mocked-device-api.ts

@@ -132,7 +132,7 @@ async function create(
 
 	// Create SupervisorAPI
 	const api = new SupervisorAPI({
-		routers: [v1.router, deviceState.router],
+		routers: [v1.router],
 		healthchecks,
 	});