Write update action and tests, remove isReadyForUpdate check

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

src/device-api/actions.ts

@@ -2,6 +2,7 @@ import * as Bluebird from 'bluebird';
 import * as _ from 'lodash';
 
 import { getGlobalApiKey, refreshKey } from '.';
+import * as messages from './messages';
 import * as eventTracker from '../event-tracker';
 import * as deviceState from '../device-state';
 import * as logger from '../logger';
@@ -14,6 +15,7 @@ import {
 	generateStep,
 } from '../compose/composition-steps';
 import { getApp } from '../device-state/db-format';
+import * as TargetState from '../device-state/target-state';
 import log from '../lib/supervisor-console';
 import blink = require('../lib/blink');
 import { lock } from '../lib/update-lock';
@@ -23,8 +25,7 @@ import {
 	BadRequestError,
 } from '../lib/errors';
 
-import type { InstancedDeviceState } from '../types';
-import * as messages from './messages';
+import { InstancedDeviceState } from '../types';
 
 /**
  * Run an array of healthchecks, outputting whether all passed or not
@@ -396,3 +397,22 @@ export const executeServiceAction = async ({
 		force,
 	);
 };
+
+/**
+ * Updates the target state cache of the Supervisor, which triggers an apply if applicable.
+ * Used by:
+ * - POST /v1/update
+ */
+export const updateTarget = async (force: boolean = false) => {
+	eventTracker.track('Update notification');
+
+	if (force || (await config.get('instantUpdates'))) {
+		TargetState.update(force, true).catch(_.noop);
+		return true;
+	}
+
+	log.debug(
+		'Ignoring update notification because instant updates are disabled or force not specified',
+	);
+	return false;
+};

src/device-api/v1.ts

@@ -5,7 +5,6 @@ import type { Response } from 'express';
 import * as actions from './actions';
 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';
 
@@ -21,7 +20,6 @@ import * as hostConfig from '../host-config';
 import * as applicationManager from '../compose/application-manager';
 import { CompositionStepAction } from '../compose/composition-steps';
 import * as commitStore from '../compose/commit';
-import * as TargetState from '../device-state/target-state';
 
 const disallowedHostConfigPatchFields = ['local_ip', 'local_port'];
 
@@ -178,25 +176,13 @@ router.post('/v1/purge', (req: AuthorizedRequest, res, next) => {
 		.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);
+router.post('/v1/update', async (req, res, next) => {
+	const force = checkTruthy(req.body.force);
+	try {
+		const result = await actions.updateTarget(force);
+		return res.sendStatus(result ? 204 : 202);
+	} catch (e: unknown) {
+		next(e);
 	}
 });
 

test/integration/device-api/actions.spec.ts

@@ -7,6 +7,7 @@ import { setTimeout } from 'timers/promises';
 import * as deviceState from '~/src/device-state';
 import * as deviceApi from '~/src/device-api';
 import * as actions from '~/src/device-api/actions';
+import * as TargetState from '~/src/device-state/target-state';
 import * as dbus from '~/lib/dbus';
 import { cleanupDocker } from '~/test-lib/docker-helper';
 
@@ -639,3 +640,40 @@ describe('reboots or shuts down device', () => {
 		expect(dbus.shutdown as SinonSpy).to.have.been.called;
 	});
 });
+
+describe('updates target state cache', () => {
+	let updateStub: SinonStub;
+	// Stub external dependencies. TargetState.update and api-binder methods
+	// should be tested separately.
+	before(async () => {
+		updateStub = stub(TargetState, 'update').resolves();
+		// updateTarget reads instantUpdates from the db
+		await config.initialized();
+	});
+
+	after(() => {
+		updateStub.restore();
+	});
+
+	afterEach(() => {
+		updateStub.resetHistory();
+	});
+
+	it('updates target state cache if instant updates are enabled', async () => {
+		await config.set({ instantUpdates: true });
+		await actions.updateTarget();
+		expect(updateStub).to.have.been.calledWith(false);
+	});
+
+	it('updates target state cache if force is specified', async () => {
+		await config.set({ instantUpdates: false });
+		await actions.updateTarget(true);
+		expect(updateStub).to.have.been.calledWith(true);
+	});
+
+	it("doesn't update target state cache if instantUpdates and force are false", async () => {
+		await config.set({ instantUpdates: false });
+		await actions.updateTarget(false);
+		expect(updateStub).to.not.have.been.called;
+	});
+});

test/integration/device-api/v1.spec.ts

@@ -636,4 +636,52 @@ describe('device-api/v1', () => {
 				.expect(500);
 		});
 	});
+
+	describe('POST /v1/update', () => {
+		let updateTargetStub: SinonStub;
+		beforeEach(() => {
+			updateTargetStub = stub(actions, 'updateTarget');
+		});
+		afterEach(async () => updateTargetStub.restore());
+
+		it('validates data from request body', async () => {
+			// Parses force: false
+			await request(api)
+				.post('/v1/update')
+				.send({ force: false })
+				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`);
+			expect(updateTargetStub.lastCall.firstArg).to.be.false;
+			updateTargetStub.resetHistory();
+
+			// Parses force: true
+			await request(api)
+				.post('/v1/update')
+				.send({ force: true })
+				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`);
+			expect(updateTargetStub.lastCall.firstArg).to.be.true;
+			updateTargetStub.resetHistory();
+
+			// Defaults to force: false
+			await request(api)
+				.post('/v1/update')
+				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`);
+			expect(updateTargetStub.lastCall.firstArg).to.be.false;
+		});
+
+		it('responds with 204 if update triggered', async () => {
+			updateTargetStub.returns(true);
+			await request(api)
+				.post('/v1/update')
+				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
+				.expect(204);
+		});
+
+		it('responds with 202 if update not triggered', async () => {
+			updateTargetStub.returns(false);
+			await request(api)
+				.post('/v1/update')
+				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
+				.expect(202);
+		});
+	});
 });

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

@@ -17,7 +17,6 @@ import * as apiBinder from '~/src/api-binder';
 import * as deviceState from '~/src/device-state';
 import * as dbus from '~/lib/dbus';
 import * as updateLock from '~/lib/update-lock';
-import * as TargetState from '~/src/device-state/target-state';
 import * as targetStateCache from '~/src/device-state/target-state-cache';
 import constants = require('~/lib/constants');
 import { UpdatesLockedError } from '~/lib/errors';
@@ -164,70 +163,6 @@ 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(() => {
-			targetUpdateSpy.resetHistory();
-		});
-
-		after(() => {
-			configStub.restore();
-			targetUpdateSpy.restore();
-			readyForUpdatesStub.restore();
-		});
-
-		it('returns 204 with no parameters', async () => {
-			// Stub response for getting instantUpdates
-			configStub.resolves(true);
-			// Make request
-			await request
-				.post('/v1/update')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
-				.expect(sampleResponses.V1.POST['/update [204 Response]'].statusCode);
-			// Check that TargetState.update was called
-			expect(targetUpdateSpy).to.be.called;
-			expect(targetUpdateSpy).to.be.calledWith(undefined, true);
-		});
-
-		it('returns 204 with force: true in body', async () => {
-			// Stub response for getting instantUpdates
-			configStub.resolves(true);
-			// Make request with force: true in the body
-			await request
-				.post('/v1/update')
-				.send({ force: true })
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
-				.expect(sampleResponses.V1.POST['/update [204 Response]'].statusCode);
-			// Check that TargetState.update was called
-			expect(targetUpdateSpy).to.be.called;
-			expect(targetUpdateSpy).to.be.calledWith(true, true);
-		});
-
-		it('returns 202 when instantUpdates are disabled', async () => {
-			// Stub response for getting instantUpdates
-			configStub.resolves(false);
-			// Make request
-			await request
-				.post('/v1/update')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
-				.expect(sampleResponses.V1.POST['/update [202 Response]'].statusCode);
-			// Check that TargetState.update was not called
-			expect(targetUpdateSpy).to.not.be.called;
-		});
-	});
-
 	describe('/v1/device/host-config', () => {
 		// Wrap GET and PATCH /v1/device/host-config tests in the same block to share
 		// common scoped variables, namely file paths and file content