ExternalVendorCode/balena-supervisor
1
0
Fork 0
mirror of https://github.com/balena-os/balena-supervisor.git synced 2024-12-24 15:56:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #1881 from balena-os/1876-add-api-lock-checks

Browse Source 
Add update lock checks to PATCH /v1/device/host-config endpoint
This commit is contained in:
bulldozer-balena[bot] 2022-02-15 15:40:04 +00:00 committed by GitHub
commit 4e2b959481
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 156 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
docs/API.md
View File

@ -548,16 +548,16 @@ $ curl -X POST --header "Content-Type:application/json" \
> **Note:** on devices with supervisor version lower than 7.22.0, replace all `BALENA_` variables with `RESIN_`, e.g. `RESIN_SUPERVISOR_ADDRESS` instead of `BALENA_SUPERVISOR_ADDRESS`.
This endpoint allows setting some configuration values for the host OS. Currently it supports
proxy and hostname configuration.
This endpoint allows setting some configuration values for the host OS. Currently it supports proxy and hostname configuration.
For proxy configuration, balenaOS 2.0.7 and higher provides a transparent proxy redirector (redsocks) that makes all connections be routed to a SOCKS or HTTP proxy. This endpoint allows services to modify these proxy settings at runtime.
#### Request body
Is a JSON object with several optional fields. Proxy and hostname configuration go under a "network" key. If "proxy" or "hostname" are not present (undefined), those values will not be modified, so that a request can modify hostname
without changing proxy settings and viceversa.
A JSON object with several optional fields. Proxy and hostname configuration go under a `network` key. If `proxy` or `hostname` are not present (undefined), those values will not be modified, so that a request can modify hostname without changing proxy settings and vice versa.
By default, with [balenaOS 2.82.6](https://github.com/balena-os/meta-balena/blob/master/CHANGELOG.md#v2826) or newer, host config PATCH requests will cause a balenaEngine restart. Therefore, with Supervisor v12.11.34 and newer, the PATCH request will respect the presence of update locks. Specify the `force` boolean (`false` by default) property in the request body to ignore this.
```json
{
@ -570,7 +570,8 @@ without changing proxy settings and viceversa.
"password": "password",
"noProxy": [ "152.10.30.4", "253.1.1.0/16" ]
},
"hostname": "mynewhostname"
"hostname": "mynewhostname",
"force": true
}
}
```
@ -581,10 +582,10 @@ guaranteed to work, especially if they block connections that the balena service
Keep in mind that, even if transparent proxy redirection will take effect immediately after the API call (i.e. all new connections will go through the proxy), open connections will not be closed. So, if for example, the device has managed to connect to the balenaCloud VPN without the proxy, it will stay connected directly without trying to reconnect through the proxy, unless the connection breaks - any reconnection attempts will then go through the proxy. To force *all* connections to go through the proxy, the best way is to reboot the device (see the /v1/reboot endpoint). In most networks were no connections to the Internet can be made if not through a proxy, this should not be necessary (as there will be no open connections before configuring the proxy settings).
The "noProxy" setting for the proxy is an optional array of IP addresses/subnets that should not be routed through the
The `noProxy` setting for the proxy is an optional array of IP addresses/subnets that should not be routed through the
proxy. Keep in mind that local/reserved subnets are already [excluded by balenaOS automatically](https://github.com/balena-os/meta-balena/blob/master/meta-balena-common/recipes-connectivity/balena-proxy-config/balena-proxy-config/balena-proxy-config).
If either "proxy" or "hostname" are null or empty values (i.e. `{}` for proxy or an empty string for hostname), they will be cleared to their default values (i.e. not using a proxy, and a hostname equal to the first 7 characters of the device's uuid, respectively).
If either `proxy` or `hostname` are null or empty values (i.e. `{}` for proxy or an empty string for hostname), they will be cleared to their default values (i.e. not using a proxy, and a hostname equal to the first 7 characters of the device's uuid, respectively).
#### Examples:
From an app container:

24
src/compose/application-manager.ts
View File

@ -1,5 +1,6 @@
import * as express from 'express';
import * as _ from 'lodash';
import StrictEventEmitter from 'strict-event-emitter-types';
import * as config from '../config';
import { transaction, Transaction } from '../db';
@ -14,7 +15,7 @@ import {
ContractViolationError,
InternalInconsistencyError,
} from '../lib/errors';
import StrictEventEmitter from 'strict-event-emitter-types';
import { lock } from '../lib/update-lock';
import App from './app';
import * as volumeManager from './volume-manager';
@ -39,7 +40,6 @@ import {
} from '../types/state';
import { checkTruthy, checkInt } from '../lib/validation';
import { Proxyvisor } from '../proxyvisor';
import * as updateLock from '../lib/update-lock';
import { EventEmitter } from 'events';
type ApplicationManagerEventEmitter = StrictEventEmitter<
@ -90,7 +90,7 @@ export function resetTimeSpentFetching(value: number = 0) {
}
const actionExecutors = getExecutors({
lockFn: lockingIfNecessary,
lockFn: lock,
callbacks: {
containerStarted: (id: string) => {
containerStarted[id] = true;
@ -157,22 +157,6 @@ function reportCurrentState(data?: Partial<InstancedAppState>) {
events.emit('change', data ?? {});
}
export async function lockingIfNecessary<T extends unknown>(
appId: number,
{ force = false, skipLock = false } = {},
fn: () => Resolvable<T>,
) {
if (skipLock) {
return fn();
}
const lockOverride = (await config.get('lockOverride')) || force;
return updateLock.lock(
appId,
{ force: lockOverride },
fn as () => PromiseLike<void>,
);
}
export async function getRequiredSteps(
targetApps: InstancedAppState,
ignoreImages: boolean = false,
@ -359,7 +343,7 @@ export async function stopAll({ force = false, skipLock = false } = {}) {
const services = await serviceManager.getAll();
await Promise.all(
services.map(async (s) => {
return lockingIfNecessary(s.appId, { force, skipLock }, async () => {
return lock(s.appId, { force, skipLock }, async () => {
await serviceManager.kill(s, { removeContainer: false, wait: true });
if (s.containerId) {
delete containerStarted[s.containerId];

13
src/device-api/common.js
View File

@ -1,21 +1,20 @@
import * as Bluebird from 'bluebird';
import * as _ from 'lodash';
import { appNotFoundMessage } from '../lib/messages';
import * as logger from '../logger';
import * as logger from '../logger';
import * as deviceState from '../device-state';
import * as applicationManager from '../compose/application-manager';
import * as serviceManager from '../compose/service-manager';
import * as volumeManager from '../compose/volume-manager';
import { InternalInconsistencyError } from '../lib/errors';
import { lock } from '../lib/update-lock';
import { appNotFoundMessage } from '../lib/messages';
export async function doRestart(appId, force) {
await deviceState.initialized;
await applicationManager.initialized;
const { lockingIfNecessary } = applicationManager;
return lockingIfNecessary(appId, { force }, () =>
return lock(appId, { force }, () =>
deviceState.getCurrentState().then(function (currentState) {
if (currentState.local.apps?.[appId] == null) {
throw new InternalInconsistencyError(
@ -42,14 +41,12 @@ export async function doPurge(appId, force) {
await deviceState.initialized;
await applicationManager.initialized;
const { lockingIfNecessary } = applicationManager;
logger.logSystemMessage(
`Purging data for app ${appId}`,
{ appId },
'Purge data',
);
return lockingIfNecessary(appId, { force }, () =>
return lock(appId, { force }, () =>
deviceState.getCurrentState().then(function (currentState) {
const allApps = currentState.local.apps;

13
src/device-state.ts
View File

@ -147,10 +147,19 @@ function createDeviceStateRouter() {
const uuid = await config.get('uuid');
req.body.network.hostname = uuid?.slice(0, 7);
}
await hostConfig.patch(req.body);
const lockOverride = await config.get('lockOverride');
await hostConfig.patch(
req.body,
validation.checkTruthy(req.body.force) || lockOverride,
);
res.status(200).send('OK');
} catch (err) {
// 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');
}
});

12
src/host-config.ts
View File

@ -7,8 +7,9 @@ import * as path from 'path';
import * as config from './config';
import * as constants from './lib/constants';
import * as dbus from './lib/dbus';
import { ENOENT } from './lib/errors';
import { ENOENT, InternalInconsistencyError } from './lib/errors';
import { writeFileAtomic, mkdirp, unlinkAll } from './lib/fs-utils';
import * as updateLock from './lib/update-lock';
const redsocksHeader = stripIndent`
base {
@ -214,7 +215,13 @@ export function get(): Bluebird<HostConfig> {
});
}
export function patch(conf: HostConfig): Bluebird<void> {
export async function patch(conf: HostConfig, force: boolean): Promise<void> {
const appId = await config.get('applicationId');
if (!appId) {
throw new InternalInconsistencyError('Could not find an appId');
}
return updateLock.lock(appId, { force }, () => {
const promises: Array<Promise<void>> = [];
if (conf != null && conf.network != null) {
if (conf.network.proxy != null) {
@ -225,4 +232,5 @@ export function patch(conf: HostConfig): Bluebird<void> {
}
}
return Bluebird.all(promises).return();
});
}

38
src/lib/update-lock.ts
View File

@ -6,8 +6,14 @@ import * as path from 'path';
import * as Lock from 'rwlock';
import * as constants from './constants';
import { ENOENT, EEXIST, UpdatesLockedError } from './errors';
import {
ENOENT,
EEXIST,
UpdatesLockedError,
InternalInconsistencyError,
} from './errors';
import { getPathOnHost, pathExistsOnHost } from './fs-utils';
import * as config from '../config';
type asyncLockFile = typeof lockFileLib & {
unlockAsync(path: string): Bluebird<void>;
@ -115,16 +121,28 @@ const lockExistsErrHandler = (err: Error, release: () => void) => {
/**
* Try to take the locks for an application. If force is set, it will remove
* all existing lockfiles before performing the operation
*
* TODO: convert to native Promises. May require native implementation of Bluebird's dispose / using
*
* TODO: Remove skipLock as it's not a good interface. If lock is called it should try to take the lock
* without an option to skip.
*/
export function lock(
export function lock<T extends unknown>(
appId: number | null,
{ force = false }: { force: boolean },
fn: () => PromiseLike<void>,
): Bluebird<void> {
{ force = false, skipLock = false }: { force: boolean; skipLock?: boolean },
fn: () => Resolvable<T>,
): Bluebird<T> {
if (skipLock) {
return Bluebird.resolve(fn());
}
const takeTheLock = () => {
if (appId == null) {
return;
}
return config
.get('lockOverride')
.then((lockOverride) => {
return writeLock(appId)
.tap((release: () => void) => {
const [lockDir] = getPathOnHost(lockPath(appId));
@ -136,7 +154,7 @@ export function lock(
lockFilesOnHost(appId, serviceName),
(tmpLockName) => {
return Bluebird.try(() => {
if (force) {
if (force || lockOverride) {
return lockFile.unlockAsync(tmpLockName);
}
})
@ -151,11 +169,17 @@ export function lock(
.catch((err) => lockExistsErrHandler(err, release));
})
.disposer(dispose);
})
.catch((err) => {
throw new InternalInconsistencyError(
`Error getting lockOverride config value: ${err?.message ?? err}`,
);
});
};
const disposer = takeTheLock();
if (disposer) {
return Bluebird.using(disposer, fn);
return Bluebird.using(disposer, fn as () => PromiseLike<T>);
} else {
return Bluebird.resolve(fn());
}

56
test/41-device-api-v1.spec.ts
View File

@ -900,6 +900,62 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
restartServiceSpy.restore();
});
it('prevents patch if update locks are present', async () => {
stub(updateLock, 'lock').callsFake((__, opts, fn) => {
if (opts.force) {
return Bluebird.resolve(fn());
}
throw new UpdatesLockedError('Updates locked');
});
await request
.patch('/v1/device/host-config')
.send({ network: { hostname: 'foobaz' } })
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.expect(423);
expect(updateLock.lock).to.be.calledOnce;
(updateLock.lock as SinonStub).restore();
await request
.get('/v1/device/host-config')
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.then((response) => {
expect(response.body.network.hostname).to.deep.equal(
'foobardevice',
);
});
});
it('allows patch while update locks are present if force is in req.body', async () => {
stub(updateLock, 'lock').callsFake((__, opts, fn) => {
if (opts.force) {
return Bluebird.resolve(fn());
}
throw new UpdatesLockedError('Updates locked');
});
await request
.patch('/v1/device/host-config')
.send({ network: { hostname: 'foobaz' }, force: true })
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.expect(200);
expect(updateLock.lock).to.be.calledOnce;
(updateLock.lock as SinonStub).restore();
await request
.get('/v1/device/host-config')
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.then((response) => {
expect(response.body.network.hostname).to.deep.equal('foobaz');
});
});
it('updates the hostname with provided string if string is not empty', async () => {
// stub servicePartOf to throw exceptions for the new service names
stub(dbus, 'servicePartOf').callsFake(