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

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

src/api-binder/index.ts

@@ -1,6 +1,5 @@
 import * as Bluebird from 'bluebird';
 import { stripIndent } from 'common-tags';
-import * as express from 'express';
 import { isLeft } from 'fp-ts/lib/Either';
 import * as t from 'io-ts';
 import * as _ from 'lodash';
@@ -45,6 +44,10 @@ interface DeviceTag {
 
 let readyForUpdates = false;
 
+export function isReadyForUpdates() {
+	return readyForUpdates;
+}
+
 export async function healthcheck() {
 	const { appUpdatePollInterval, unmanaged, connectivityCheckEnabled } =
 		await config.getMany([
@@ -570,27 +573,3 @@ export const initialized = _.once(async () => {
 
 	log.info(`API Binder bound to: ${baseUrl}`);
 });
-
-export const router = express.Router();
-
-router.post('/v1/update', (req, res, next) => {
-	eventTracker.track('Update notification');
-	if (readyForUpdates) {
-		config
-			.get('instantUpdates')
-			.then((instantUpdates) => {
-				if (instantUpdates) {
-					TargetState.update(req.body.force, true).catch(_.noop);
-					res.sendStatus(204);
-				} else {
-					log.debug(
-						'Ignoring update notification because instant updates are disabled',
-					);
-					res.sendStatus(202);
-				}
-			})
-			.catch(next);
-	} else {
-		res.sendStatus(202);
-	}
-});

src/compose/application-manager.ts

@@ -1,48 +1,47 @@
 import * as express from 'express';
 import * as _ from 'lodash';
+import { EventEmitter } from 'events';
 import StrictEventEmitter from 'strict-event-emitter-types';
 
 import * as config from '../config';
 import { transaction, Transaction } from '../db';
+import * as logger from '../logger';
+import LocalModeManager from '../local-mode';
+import { Proxyvisor } from '../proxyvisor';
+
 import * as dbFormat from '../device-state/db-format';
 import { validateTargetContracts } from '../lib/contracts';
 import constants = require('../lib/constants');
 import { docker } from '../lib/docker-utils';
-import * as logger from '../logger';
 import log from '../lib/supervisor-console';
-import LocalModeManager from '../local-mode';
 import {
 	ContractViolationError,
 	InternalInconsistencyError,
 } from '../lib/errors';
 import { lock } from '../lib/update-lock';
+import { checkTruthy } from '../lib/validation';
+import { createV2Api } from '../device-api/v2';
 
 import App from './app';
 import * as volumeManager from './volume-manager';
 import * as networkManager from './network-manager';
 import * as serviceManager from './service-manager';
 import * as imageManager from './images';
-import type { Image } from './images';
-import { getExecutors, CompositionStepT } from './composition-steps';
 import * as commitStore from './commit';
-
 import Service from './service';
 import Network from './network';
 import Volume from './volume';
+import { generateStep, getExecutors } from './composition-steps';
 
-import { createV1Api } from '../device-api/v1';
+import type {
-import { createV2Api } from '../device-api/v2';
-import { CompositionStep, generateStep } from './composition-steps';
-import {
 	InstancedAppState,
 	TargetApps,
 	DeviceLegacyReport,
 	AppState,
 	ServiceState,
 } from '../types/state';
-import { checkTruthy } from '../lib/validation';
+import type { Image } from './images';
-import { Proxyvisor } from '../proxyvisor';
+import type { CompositionStep, CompositionStepT } from './composition-steps';
-import { EventEmitter } from 'events';
 
 type ApplicationManagerEventEmitter = StrictEventEmitter<
 	EventEmitter,
@@ -62,7 +61,6 @@ const localModeManager = new LocalModeManager();
 export const router = (() => {
 	const $router = express.Router();
 
-	createV1Api($router);
 	createV2Api($router);
 
 	$router.use(proxyvisor.router);

src/device-api/v1.ts

@@ -1,181 +1,140 @@
 import * as express from 'express';
 import * as _ from 'lodash';
 
+import { doRestart, doPurge } from './common';
+import { AuthorizedRequest } from './api-keys';
 import * as eventTracker from '../event-tracker';
+import { isReadyForUpdates } from '../api-binder';
+import * as config from '../config';
 import * as constants from '../lib/constants';
 import { checkInt, checkTruthy } from '../lib/validation';
-import { doRestart, doPurge } from './common';
+import log from '../lib/supervisor-console';
-
 import * as applicationManager from '../compose/application-manager';
 import { generateStep } from '../compose/composition-steps';
 import * as commitStore from '../compose/commit';
-import { AuthorizedRequest } from './api-keys';
 import { getApp } from '../device-state/db-format';
+import * as TargetState from '../device-state/target-state';
 
-export function createV1Api(router: express.Router) {
+export const router = express.Router();
-	router.post('/v1/restart', (req: AuthorizedRequest, res, next) => {
-		const appId = checkInt(req.body.appId);
-		const force = checkTruthy(req.body.force);
-		eventTracker.track('Restart container (v1)', { appId });
-		if (appId == null) {
-			return res.status(400).send('Missing app id');
-		}
 
-		// handle the case where the appId is out of scope
+router.post('/v1/restart', (req: AuthorizedRequest, res, next) => {
-		if (!req.auth.isScoped({ apps: [appId] })) {
+	const appId = checkInt(req.body.appId);
-			res.status(401).json({
+	const force = checkTruthy(req.body.force);
-				status: 'failed',
+	eventTracker.track('Restart container (v1)', { appId });
-				message: 'Application is not available',
+	if (appId == null) {
-			});
+		return res.status(400).send('Missing app id');
-			return;
+	}
-		}
 
-		return doRestart(appId, force)
+	// handle the case where the appId is out of scope
-			.then(() => res.status(200).send('OK'))
+	if (!req.auth.isScoped({ apps: [appId] })) {
-			.catch(next);
+		res.status(401).json({
-	});
+			status: 'failed',
+			message: 'Application is not available',
+		});
+		return;
+	}
 
-	const v1StopOrStart = (
+	return doRestart(appId, force)
-		req: AuthorizedRequest,
+		.then(() => res.status(200).send('OK'))
-		res: express.Response,
+		.catch(next);
-		next: express.NextFunction,
+});
-		action: 'start' | 'stop',
-	) => {
-		const appId = checkInt(req.params.appId);
-		const force = checkTruthy(req.body.force);
-		if (appId == null) {
-			return res.status(400).send('Missing app id');
-		}
 
-		return Promise.all([applicationManager.getCurrentApps(), getApp(appId)])
+const v1StopOrStart = (
-			.then(([apps, targetApp]) => {
+	req: AuthorizedRequest,
-				if (apps[appId] == null) {
+	res: express.Response,
-					return res.status(400).send('App not found');
+	next: express.NextFunction,
-				}
+	action: 'start' | 'stop',
-				const app = apps[appId];
+) => {
-				let service = app.services[0];
+	const appId = checkInt(req.params.appId);
-				if (service == null) {
+	const force = checkTruthy(req.body.force);
-					return res.status(400).send('No services on app');
+	if (appId == null) {
-				}
+		return res.status(400).send('Missing app id');
-				if (app.services.length > 1) {
+	}
-					return res
-						.status(400)
-						.send(
-							'Some v1 endpoints are only allowed on single-container apps',
-						);
-				}
 
-				// check that the request is scoped to cover this application
+	return Promise.all([applicationManager.getCurrentApps(), getApp(appId)])
-				if (!req.auth.isScoped({ apps: [app.appId] })) {
+		.then(([apps, targetApp]) => {
-					return res.status(401).send('Unauthorized');
+			if (apps[appId] == null) {
-				}
-
-				// Get the service from the target state (as we do in v2)
-				// TODO: what if we want to start a service belonging to the current app?
-				const targetService = _.find(targetApp.services, {
-					serviceName: service.serviceName,
-				});
-
-				applicationManager.setTargetVolatileForService(service.imageId, {
-					running: action !== 'stop',
-				});
-
-				const stopOpts = { wait: true };
-				const step = generateStep(action, {
-					current: service,
-					target: targetService,
-					...stopOpts,
-				});
-
-				return applicationManager
-					.executeStep(step, { force })
-					.then(function () {
-						if (action === 'stop') {
-							return service;
-						}
-						// We refresh the container id in case we were starting an app with no container yet
-						return applicationManager.getCurrentApps().then(function (apps2) {
-							const app2 = apps2[appId];
-							service = app2.services[0];
-							if (service == null) {
-								throw new Error('App not found after running action');
-							}
-							return service;
-						});
-					})
-					.then((service2) =>
-						res.status(200).json({ containerId: service2.containerId }),
-					);
-			})
-			.catch(next);
-	};
-
-	const createV1StopOrStartHandler = (action: 'start' | 'stop') =>
-		_.partial(v1StopOrStart, _, _, _, action);
-
-	router.post('/v1/apps/:appId/stop', createV1StopOrStartHandler('stop'));
-	router.post('/v1/apps/:appId/start', createV1StopOrStartHandler('start'));
-
-	router.get('/v1/apps/:appId', async (req: AuthorizedRequest, res, next) => {
-		const appId = checkInt(req.params.appId);
-		eventTracker.track('GET app (v1)', { appId });
-		if (appId == null) {
-			return res.status(400).send('Missing app id');
-		}
-
-		try {
-			const apps = await applicationManager.getCurrentApps();
-			const app = apps[appId];
-			const service = app?.services?.[0];
-			if (service == null) {
 				return res.status(400).send('App not found');
 			}
-
+			const app = apps[appId];
-			// handle the case where the appId is out of scope
+			let service = app.services[0];
-			if (!req.auth.isScoped({ apps: [app.appId] })) {
+			if (service == null) {
-				res.status(401).json({
+				return res.status(400).send('No services on app');
-					status: 'failed',
-					message: 'Application is not available',
-				});
-				return;
 			}
-
 			if (app.services.length > 1) {
 				return res
 					.status(400)
 					.send('Some v1 endpoints are only allowed on single-container apps');
 			}
 
-			// Because we only have a single app, we can fetch the commit for that
+			// check that the request is scoped to cover this application
-			// app, and maintain backwards compatability
+			if (!req.auth.isScoped({ apps: [app.appId] })) {
-			const commit = await commitStore.getCommitForApp(appId);
+				return res.status(401).send('Unauthorized');
+			}
 
-			// Don't return data that will be of no use to the user
+			// Get the service from the target state (as we do in v2)
-			const appToSend = {
+			// TODO: what if we want to start a service belonging to the current app?
-				appId,
+			const targetService = _.find(targetApp.services, {
-				commit,
+				serviceName: service.serviceName,
-				containerId: service.containerId,
+			});
-				env: _.omit(service.config.environment, constants.privateAppEnvVars),
-				imageId: service.config.image,
-				releaseId: service.releaseId,
-			};
 
-			return res.json(appToSend);
+			applicationManager.setTargetVolatileForService(service.imageId, {
-		} catch (e) {
+				running: action !== 'stop',
-			next(e);
+			});
-		}
-	});
 
-	router.post('/v1/purge', (req: AuthorizedRequest, res, next) => {
+			const stopOpts = { wait: true };
-		const appId = checkInt(req.body.appId);
+			const step = generateStep(action, {
-		const force = checkTruthy(req.body.force);
+				current: service,
-		if (appId == null) {
+				target: targetService,
-			const errMsg = 'Invalid or missing appId';
+				...stopOpts,
-			return res.status(400).send(errMsg);
+			});
+
+			return applicationManager
+				.executeStep(step, { force })
+				.then(function () {
+					if (action === 'stop') {
+						return service;
+					}
+					// We refresh the container id in case we were starting an app with no container yet
+					return applicationManager.getCurrentApps().then(function (apps2) {
+						const app2 = apps2[appId];
+						service = app2.services[0];
+						if (service == null) {
+							throw new Error('App not found after running action');
+						}
+						return service;
+					});
+				})
+				.then((service2) =>
+					res.status(200).json({ containerId: service2.containerId }),
+				);
+		})
+		.catch(next);
+};
+
+const createV1StopOrStartHandler = (action: 'start' | 'stop') =>
+	_.partial(v1StopOrStart, _, _, _, action);
+
+router.post('/v1/apps/:appId/stop', createV1StopOrStartHandler('stop'));
+router.post('/v1/apps/:appId/start', createV1StopOrStartHandler('start'));
+
+router.get('/v1/apps/:appId', async (req: AuthorizedRequest, res, next) => {
+	const appId = checkInt(req.params.appId);
+	eventTracker.track('GET app (v1)', { appId });
+	if (appId == null) {
+		return res.status(400).send('Missing app id');
+	}
+
+	try {
+		const apps = await applicationManager.getCurrentApps();
+		const app = apps[appId];
+		const service = app?.services?.[0];
+		if (service == null) {
+			return res.status(400).send('App not found');
 		}
 
 		// handle the case where the appId is out of scope
-		if (!req.auth.isScoped({ apps: [appId] })) {
+		if (!req.auth.isScoped({ apps: [app.appId] })) {
 			res.status(401).json({
 				status: 'failed',
 				message: 'Application is not available',
@@ -183,8 +142,72 @@ export function createV1Api(router: express.Router) {
 			return;
 		}
 
-		return doPurge(appId, force)
+		if (app.services.length > 1) {
-			.then(() => res.status(200).json({ Data: 'OK', Error: '' }))
+			return res
+				.status(400)
+				.send('Some v1 endpoints are only allowed on single-container apps');
+		}
+
+		// Because we only have a single app, we can fetch the commit for that
+		// app, and maintain backwards compatability
+		const commit = await commitStore.getCommitForApp(appId);
+
+		// Don't return data that will be of no use to the user
+		const appToSend = {
+			appId,
+			commit,
+			containerId: service.containerId,
+			env: _.omit(service.config.environment, constants.privateAppEnvVars),
+			imageId: service.config.image,
+			releaseId: service.releaseId,
+		};
+
+		return res.json(appToSend);
+	} catch (e) {
+		next(e);
+	}
+});
+
+router.post('/v1/purge', (req: AuthorizedRequest, res, next) => {
+	const appId = checkInt(req.body.appId);
+	const force = checkTruthy(req.body.force);
+	if (appId == null) {
+		const errMsg = 'Invalid or missing appId';
+		return res.status(400).send(errMsg);
+	}
+
+	// handle the case where the appId is out of scope
+	if (!req.auth.isScoped({ apps: [appId] })) {
+		res.status(401).json({
+			status: 'failed',
+			message: 'Application is not available',
+		});
+		return;
+	}
+
+	return doPurge(appId, force)
+		.then(() => res.status(200).json({ Data: 'OK', Error: '' }))
+		.catch(next);
+});
+
+router.post('/v1/update', (req, res, next) => {
+	eventTracker.track('Update notification');
+	if (isReadyForUpdates()) {
+		config
+			.get('instantUpdates')
+			.then((instantUpdates) => {
+				if (instantUpdates) {
+					TargetState.update(req.body.force, true).catch(_.noop);
+					res.sendStatus(204);
+				} else {
+					log.debug(
+						'Ignoring update notification because instant updates are disabled',
+					);
+					res.sendStatus(202);
+				}
+			})
 			.catch(next);
-	});
+	} else {
-}
+		res.sendStatus(202);
+	}
+});

src/supervisor.ts

@@ -2,16 +2,18 @@ import * as apiBinder from './api-binder';
 import * as db from './db';
 import * as config from './config';
 import * as deviceState from './device-state';
+import * as logger from './logger';
+import SupervisorAPI from './device-api';
+import * as v1 from './device-api/v1';
+import logMonitor from './logging/monitor';
+
 import { intialiseContractRequirements } from './lib/contracts';
 import { normaliseLegacyDatabase } from './lib/legacy';
 import * as osRelease from './lib/os-release';
-import * as logger from './logger';
-import SupervisorAPI from './device-api';
 import log from './lib/supervisor-console';
 import version = require('./lib/supervisor-version');
 import * as avahi from './lib/avahi';
 import * as firewall from './lib/firewall';
-import logMonitor from './logging/monitor';
 
 const startupConfigFields: config.ConfigKey[] = [
 	'uuid',
@@ -66,7 +68,7 @@ export class Supervisor {
 			(() => {
 				log.info('Starting API server');
 				this.api = new SupervisorAPI({
-					routers: [apiBinder.router, deviceState.router],
+					routers: [v1.router, deviceState.router],
 					healthchecks: [apiBinder.healthcheck, deviceState.healthcheck],
 				});
 				this.api.listen(conf.listenPort, conf.apiTimeout);

test/legacy/41-device-api-v1.spec.ts

@@ -667,10 +667,12 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 	describe('POST /v1/update', () => {
 		let configStub: SinonStub;
 		let targetUpdateSpy: SinonSpy;
+		let readyForUpdatesStub: SinonStub;
 
 		before(() => {
 			configStub = stub(config, 'get');
 			targetUpdateSpy = spy(TargetState, 'update');
+			readyForUpdatesStub = stub(apiBinder, 'isReadyForUpdates').returns(true);
 		});
 
 		afterEach(() => {
@@ -680,6 +682,7 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 		after(() => {
 			configStub.restore();
 			targetUpdateSpy.restore();
+			readyForUpdatesStub.restore();
 		});
 
 		it('returns 204 with no parameters', async () => {

test/lib/mocked-device-api.ts

@@ -1,16 +1,13 @@
 import * as _ from 'lodash';
-import { Router } from 'express';
 import rewire = require('rewire');
 
 import { unlinkAll } from '~/lib/fs-utils';
-import * as applicationManager from '~/src/compose/application-manager';
 import * as serviceManager from '~/src/compose/service-manager';
 import * as volumeManager from '~/src/compose/volume-manager';
 import * as commitStore from '~/src/compose/commit';
 import * as config from '~/src/config';
 import * as db from '~/src/db';
-import { createV1Api } from '~/src/device-api/v1';
+import * as v1 from '~/src/device-api/v1';
-import { createV2Api } from '~/src/device-api/v2';
 import * as deviceState from '~/src/device-state';
 import SupervisorAPI from '~/src/device-api';
 import { Service } from '~/src/compose/service';
@@ -135,7 +132,7 @@ async function create(
 
 	// Create SupervisorAPI
 	const api = new SupervisorAPI({
-		routers: [deviceState.router, buildRoutes()],
+		routers: [v1.router, deviceState.router],
 		healthchecks,
 	});
 
@@ -173,21 +170,10 @@ async function initConfig(): Promise<void> {
 	}
 }
 
-function buildRoutes(): Router {
-	// Add to existing apiBinder router (it contains additional middleware and endpoints)
-	const router = apiBinder.router;
-	// Add V1 routes
-	createV1Api(applicationManager.router);
-	// Add V2 routes
-	createV2Api(applicationManager.router);
-	// Return modified Router
-	return router;
-}
-
 // TO-DO: Create a cleaner way to restore previous values.
 const originalVolGetAll = volumeManager.getAllByAppId;
 const originalSvcGetStatus = serviceManager.getState;
-const originalReadyForUpdates = apiBinder.__get__('readyForUpdates');
+const originalReadyForUpdates = apiBinder.isReadyForUpdates();
 
 function setupStubs() {
 	apiBinder.__set__('readyForUpdates', true);