refactor: Remove unnecessary validations and casts

Change-type: patch
Signed-off-by: Cameron Diver <cameron@balena.io>
This commit is contained in:
Cameron Diver 2019-01-09 10:04:04 +00:00
parent db74e748a1
commit 6a3148ff80
No known key found for this signature in database
GPG Key ID: 49690ED87032539F
10 changed files with 107 additions and 144 deletions

View File

@ -20,11 +20,11 @@ import {
} from './lib/errors';
import { pathExistsOnHost } from './lib/fs-utils';
import { request, requestOpts } from './lib/request';
import { ConfigValue } from './lib/types';
import { writeLock } from './lib/update-lock';
import { checkInt, checkTruthy } from './lib/validation';
import { DeviceApplicationState } from './types/state';
import { SchemaReturn as ConfigSchemaType } from './config/schema-type';
const REPORT_SUCCESS_DELAY = 1000;
const MAX_REPORT_RETRY_DELAY = 60000;
@ -46,16 +46,8 @@ interface APIBinderConstructOpts {
eventTracker: EventTracker;
}
interface KeyExchangeOpts {
uuid: ConfigValue;
deviceApiKey: ConfigValue;
apiTimeout: ConfigValue;
apiEndpoint: ConfigValue;
provisioningApiKey: ConfigValue;
}
interface Device {
id: string;
id: number;
[key: string]: unknown;
}
@ -65,6 +57,8 @@ interface DevicePinInfo {
commit: string;
}
type KeyExchangeOpts = ConfigSchemaType<'provisioningOptions'>;
export class APIBinder {
public router: express.Router;
@ -126,8 +120,7 @@ export class APIBinder {
const timeSinceLastFetch = process.hrtime(this.lastTargetStateFetch);
const timeSinceLastFetchMs =
timeSinceLastFetch[0] * 1000 + timeSinceLastFetch[1] / 1e6;
const stateFetchHealthy =
timeSinceLastFetchMs < 2 * (appUpdatePollInterval as number);
const stateFetchHealthy = timeSinceLastFetchMs < 2 * appUpdatePollInterval;
const stateReportHealthy =
!connectivityCheckEnabled ||
!this.deviceState.connected ||
@ -146,7 +139,7 @@ export class APIBinder {
return;
}
const baseUrl = url.resolve(apiEndpoint as string, '/v5/');
const baseUrl = url.resolve(apiEndpoint, '/v5/');
const passthrough = _.cloneDeep(requestOpts);
passthrough.headers =
passthrough.headers != null ? passthrough.headers : {};
@ -210,16 +203,13 @@ export class APIBinder {
// Either we haven't reported our initial config or we've been re-provisioned
if (apiEndpoint !== initialConfigReported) {
console.log('Reporting initial configuration');
await this.reportInitialConfig(
apiEndpoint as string,
bootstrapRetryDelay as number,
);
await this.reportInitialConfig(apiEndpoint, bootstrapRetryDelay);
}
console.log('Starting current state report');
await this.startCurrentStateReport();
await this.loadBackupFromMigration(bootstrapRetryDelay as number);
await this.loadBackupFromMigration(bootstrapRetryDelay);
this.readyForUpdates = true;
console.log('Starting target state poll');
@ -288,7 +278,7 @@ export class APIBinder {
id,
body: updatedFields,
})
.timeout(conf.apiTimeout as number);
.timeout(conf.apiTimeout);
}
public async provisionDependentDevice(device: Device): Promise<Device> {
@ -325,7 +315,7 @@ export class APIBinder {
return (await this.balenaApi
.post({ resource: 'device', body: device })
// TODO: Remove the `as number` when we fix the config typings
.timeout(conf.apiTimeout as number)) as Device;
.timeout(conf.apiTimeout)) as Device;
}
public async getTargetState(): Promise<DeviceApplicationState> {
@ -354,7 +344,7 @@ export class APIBinder {
return await this.cachedBalenaApi
._request(requestParams)
.timeout(apiTimeout as number);
.timeout(apiTimeout);
}
// TODO: Once 100% typescript, change this to a native promise
@ -451,7 +441,7 @@ export class APIBinder {
'localMode',
]);
if (checkTruthy(conf.localMode || false)) {
if (conf.localMode) {
return;
}
@ -460,12 +450,17 @@ export class APIBinder {
return 0;
}
const apiEndpoint = conf.apiEndpoint;
const uuid = conf.uuid;
if (uuid == null || apiEndpoint == null) {
throw new InternalInconsistencyError(
'No uuid or apiEndpoint provided to ApiBinder.report',
);
}
await Bluebird.resolve(
this.sendReportPatch(stateDiff, conf as {
uuid: string;
apiEndpoint: string;
}),
).timeout(conf.apiTimeout as number);
this.sendReportPatch(stateDiff, { apiEndpoint, uuid }),
).timeout(conf.apiTimeout);
this.stateReportErrors = 0;
_.assign(this.lastReportedState.local, stateDiff.local);
@ -523,14 +518,7 @@ export class APIBinder {
private async pollTargetState(): Promise<void> {
// TODO: Remove the checkInt here with the config changes
let pollInterval = checkInt((await this.config.get(
'appUpdatePollInterval',
)) as string);
if (!_.isNumber(pollInterval)) {
throw new InternalInconsistencyError(
'appUpdatePollInterval not a number in ApiBinder.pollTargetState',
);
}
let pollInterval = await this.config.get('appUpdatePollInterval');
try {
await this.getAndSetTargetState(false);
@ -556,6 +544,13 @@ export class APIBinder {
try {
const deviceId = await this.config.get('deviceId');
if (deviceId == null) {
throw new InternalInconsistencyError(
'Device ID not defined in ApiBinder.pinDevice',
);
}
const release = await this.balenaApi.get({
resource: 'release',
options: {
@ -577,7 +572,7 @@ export class APIBinder {
await this.balenaApi.patch({
resource: 'device',
id: deviceId as number,
id: deviceId,
body: {
should_be_running__release: releaseId,
},
@ -665,15 +660,11 @@ export class APIBinder {
opts?: KeyExchangeOpts,
): Promise<Device> {
if (opts == null) {
// FIXME: This casting shouldn't be necessary and stems from the
// meta-option provioningOptions not returning a ConfigValue
opts = ((await this.config.get(
'provisioningOptions',
)) as any) as KeyExchangeOpts;
opts = await this.config.get('provisioningOptions');
}
const uuid = opts.uuid as string;
const apiTimeout = opts.apiTimeout as number;
const uuid = opts.uuid;
const apiTimeout = opts.apiTimeout;
if (!(uuid && apiTimeout)) {
throw new InternalInconsistencyError(
'UUID and apiTimeout should be defined in exchangeKeyAndGetDevice',
@ -685,7 +676,7 @@ export class APIBinder {
if (opts.deviceApiKey != null) {
const device = await this.fetchDevice(
uuid,
opts.deviceApiKey as string,
opts.deviceApiKey,
apiTimeout,
);
if (device != null) {
@ -695,9 +686,14 @@ export class APIBinder {
// If it's not valid or doesn't exist then we try to use the
// user/provisioning api key for the exchange
if (!opts.provisioningApiKey) {
throw new InternalInconsistencyError(
'Required a provisioning key in exchangeKeyAndGetDevice',
);
}
const device = await this.fetchDevice(
uuid,
opts.provisioningApiKey as string,
opts.provisioningApiKey,
apiTimeout,
);
@ -746,10 +742,7 @@ export class APIBinder {
private async provision() {
let device: Device | null = null;
// FIXME: Config typing
const opts = ((await this.config.get(
'provisioningOptions',
)) as any) as Dictionary<any>;
const opts = await this.config.get('provisioningOptions');
if (
opts.registered_at != null &&
opts.deviceId != null &&
@ -762,10 +755,7 @@ export class APIBinder {
console.log(
'Device is registered but no device id available, attempting key exchange',
);
device =
(await this.exchangeKeyAndGetDeviceOrRegenerate(
opts as KeyExchangeOpts,
)) || null;
device = (await this.exchangeKeyAndGetDeviceOrRegenerate(opts)) || null;
} else if (opts.registered_at == null) {
console.log('New device detected. Provisioning...');
try {
@ -774,9 +764,7 @@ export class APIBinder {
} catch (err) {
if (DuplicateUuidError(err)) {
console.log('UUID already registered, trying a key exchange');
await this.exchangeKeyAndGetDeviceOrRegenerate(
opts as KeyExchangeOpts,
);
await this.exchangeKeyAndGetDeviceOrRegenerate(opts);
} else {
throw err;
}
@ -785,7 +773,7 @@ export class APIBinder {
console.log(
'Device is registered but we still have an apiKey, attempting key exchange',
);
device = await this.exchangeKeyAndGetDevice(opts as KeyExchangeOpts);
device = await this.exchangeKeyAndGetDevice(opts);
}
if (!device) {
@ -811,13 +799,7 @@ export class APIBinder {
this.eventTracker.track('Device bootstrap success');
// Now check if we need to pin the device
const toPin = await this.config.get('pinDevice');
let pinValue: DevicePinInfo | null = null;
try {
pinValue = JSON.parse(toPin as string);
} catch (e) {
console.log('Warning: Malformed pinDevice value in supervisor database');
}
const pinValue = await this.config.get('pinDevice');
if (pinValue != null) {
if (pinValue.app == null || pinValue.commit == null) {

View File

@ -140,7 +140,7 @@ module.exports = class ApplicationManager extends EventEmitter
@images.save(step.image)
cleanup: (step) =>
@config.get('localMode').then (localMode) =>
if !checkTruthy(localMode)
if !localMode
@images.cleanup()
createNetworkOrVolume: (step) =>
if step.model is 'network'
@ -762,7 +762,7 @@ module.exports = class ApplicationManager extends EventEmitter
getTargetApps: =>
@config.getMany(['apiEndpoint', 'localMode']). then ({ apiEndpoint, localMode }) =>
source = apiEndpoint
if checkTruthy(localMode)
if localMode
source = 'local'
Promise.map(@db.models('app').where({ source }), @normaliseAndExtendAppFromDB)
.map (app) =>
@ -847,8 +847,6 @@ module.exports = class ApplicationManager extends EventEmitter
return { imagesToSave, imagesToRemove }
_inferNextSteps: (cleanupNeeded, availableImages, downloading, supervisorNetworkReady, current, target, ignoreImages, { localMode, delta }) =>
localMode = checkTruthy(localMode)
delta = checkTruthy(delta)
Promise.try =>
if localMode
ignoreImages = true
@ -895,7 +893,7 @@ module.exports = class ApplicationManager extends EventEmitter
return Promise.try(fn)
@config.get('lockOverride')
.then (lockOverride) ->
return checkTruthy(lockOverride) or force
return lockOverride or force
.then (force) ->
updateLock.lock(appId, { force }, fn)
@ -919,7 +917,7 @@ module.exports = class ApplicationManager extends EventEmitter
getRequiredSteps: (currentState, targetState, extraState, ignoreImages = false) =>
{ cleanupNeeded, availableImages, downloading, supervisorNetworkReady, delta, localMode } = extraState
conf = _.mapValues({ delta, localMode }, (v) -> checkTruthy(v))
conf = { delta, localMode }
if conf.localMode
cleanupNeeded = false
@_inferNextSteps(cleanupNeeded, availableImages, downloading, supervisorNetworkReady, currentState, targetState, ignoreImages, conf)

View File

@ -59,7 +59,7 @@ module.exports = class Images extends EventEmitter
.catch =>
@reportChange(image.imageId, _.merge(_.clone(image), { status: 'Downloading', downloadProgress: 0 }))
Promise.try =>
if validation.checkTruthy(opts.delta) and opts.deltaSource?
if opts.delta and opts.deltaSource?
@logger.logSystemEvent(logTypes.downloadImageDelta, { image })
@inspectByName(opts.deltaSource)
.then (srcImage) =>

View File

@ -10,7 +10,6 @@ import {
serviceNotFoundMessage,
v2ServiceEndpointInputErrorMessage,
} from '../lib/messages';
import { checkTruthy } from '../lib/validation';
import { doPurge, doRestart, serviceAction } from './common';
import supervisorVersion = require('../lib/supervisor-version');
@ -232,7 +231,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) {
router.get('/v2/local/target-state', async (_req, res) => {
try {
const localMode = checkTruthy(await deviceState.config.get('localMode'));
const localMode = await deviceState.config.get('localMode');
if (!localMode) {
return res.status(400).json({
status: 'failed',
@ -258,7 +257,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) {
// TODO: We really should refactor the config module to provide bools
// as bools etc
try {
const localMode = checkTruthy(await deviceState.config.get('localMode'));
const localMode = await deviceState.config.get('localMode');
if (!localMode) {
return res.status(400).json({
status: 'failed',

View File

@ -1,6 +1,7 @@
import * as _ from 'lodash';
import Config from './config';
import { SchemaTypeKey } from './config/schema-type';
import Database, { Transaction } from './db';
import Logger from './logger';
@ -60,7 +61,7 @@ export class DeviceConfig {
private actionExecutors: DeviceActionExecutors;
private configBackend: DeviceConfigBackend | null = null;
private static configKeys: Dictionary<ConfigOption> = {
private static readonly configKeys: Dictionary<ConfigOption> = {
appUpdatePollInterval: {
envVarName: 'SUPERVISOR_POLL_INTERVAL',
varType: 'int',
@ -144,7 +145,9 @@ export class DeviceConfig {
if (!_.isObject(step.target)) {
throw new Error('Non-dictionary value passed to changeConfig');
}
await this.config.set(step.target as Dictionary<string>);
// TODO: Change the typing of step so that the types automatically
// work out and we don't need this cast to any
await this.config.set(step.target as { [key in SchemaTypeKey]: any });
if (step.humanReadableTarget) {
this.logger.logConfigChange(step.humanReadableTarget, {
success: true,
@ -200,9 +203,6 @@ export class DeviceConfig {
return this.configBackend;
}
const dt = await this.config.get('deviceType');
if (!_.isString(dt)) {
throw new Error('Could not detect device type');
}
this.configBackend = configUtils.getConfigBackend(dt) || null;
@ -253,9 +253,9 @@ export class DeviceConfig {
}
public async getCurrent() {
const conf = await this.config.getMany(
['deviceType'].concat(_.keys(DeviceConfig.configKeys)),
);
const conf = await this.config.getMany(['deviceType'].concat(
_.keys(DeviceConfig.configKeys),
) as SchemaTypeKey[]);
const configBackend = await this.getConfigBackend();
@ -271,7 +271,7 @@ export class DeviceConfig {
for (const key in DeviceConfig.configKeys) {
const { envVarName } = DeviceConfig.configKeys[key];
const confValue = conf[key];
const confValue = conf[key as SchemaTypeKey];
currentConf[envVarName] = confValue != null ? confValue.toString() : '';
}
@ -391,7 +391,7 @@ export class DeviceConfig {
// Check for special case actions for the VPN
if (
!checkTruthy(unmanaged || false) &&
!unmanaged &&
!_.isEmpty(target['SUPERVISOR_VPN_CONTROL']) &&
DeviceConfig.checkBoolChanged(current, target, 'SUPERVISOR_VPN_CONTROL')
) {

View File

@ -54,7 +54,7 @@ createDeviceStateRouter = (deviceState) ->
rebootOrShutdown = (req, res, action) ->
deviceState.config.get('lockOverride')
.then (lockOverride) ->
force = validation.checkTruthy(req.body.force) or validation.checkTruthy(lockOverride)
force = validation.checkTruthy(req.body.force) or lockOverride
deviceState.executeStepAction({ action }, { force })
.then (response) ->
res.status(202).json(response)
@ -246,7 +246,7 @@ module.exports = class DeviceState extends EventEmitter
init: ->
@config.on 'change', (changedConfig) =>
if changedConfig.loggingEnabled?
@logger.enable(validation.checkTruthy(changedConfig.loggingEnabled))
@logger.enable(changedConfig.loggingEnabled)
if changedConfig.apiSecret?
@reportCurrentState(api_secret: changedConfig.apiSecret)
@ -258,7 +258,7 @@ module.exports = class DeviceState extends EventEmitter
.then (conf) =>
@applications.init()
.then =>
if !validation.checkTruthy(conf.initialConfigSaved)
if !conf.initialConfigSaved
@saveInitialConfig()
.then =>
@initNetworkChecks(conf)
@ -280,7 +280,7 @@ module.exports = class DeviceState extends EventEmitter
.then =>
@applications.getTargetApps()
.then (targetApps) =>
if !conf.provisioned or (_.isEmpty(targetApps) and !validation.checkTruthy(conf.targetStateSet))
if !conf.provisioned or (_.isEmpty(targetApps) and !conf.targetStateSet)
@loadTargetFromFile()
.finally =>
@config.set({ targetStateSet: 'true' })
@ -296,7 +296,7 @@ module.exports = class DeviceState extends EventEmitter
@triggerApplyTarget({ initial: true })
initNetworkChecks: ({ apiEndpoint, connectivityCheckEnabled, unmanaged }) =>
return if validation.checkTruthy(unmanaged)
return if unmanaged
network.startConnectivityCheck apiEndpoint, connectivityCheckEnabled, (connected) =>
@connected = connected
@config.on 'change', (changedConfig) ->
@ -499,9 +499,9 @@ module.exports = class DeviceState extends EventEmitter
console.log('Device will be pinned')
if commitToPin? and appToPin?
@config.set
pinDevice: JSON.stringify {
pinDevice: {
commit: commitToPin,
app: appToPin,
app: parseInt(appToPin, 10),
}
# Ensure that this is actually a file, and not an empty path
# It can be an empty path because if the file does not exist

View File

@ -5,20 +5,3 @@ export interface EnvVarObject {
export interface LabelObject {
[name: string]: string;
}
// For backwards compatibility we need to use export = Config in config.ts
// so to export these types they have been moved here
export type ConfigValue = string | number | boolean | null;
export interface ConfigMap {
[key: string]: ConfigValue;
}
export interface ConfigSchema {
[key: string]: {
source: string;
default?: any;
mutable?: boolean;
removeIfNull?: boolean;
};
}

View File

@ -3,8 +3,8 @@ import * as Docker from 'dockerode';
import * as _ from 'lodash';
import Config from './config';
import { SchemaReturn, SchemaTypeKey } from './config/schema-type';
import Database from './db';
import { checkTruthy } from './lib/validation';
import { Logger } from './logger';
/**
@ -25,25 +25,28 @@ export class LocalModeManager {
public async init() {
// Setup a listener to catch state changes relating to local mode
this.config.on('change', changed => {
if (changed.localMode != null) {
const localMode = checkTruthy(changed.localMode) || false;
this.config.on(
'change',
(changed: { [key in SchemaTypeKey]: SchemaReturn<key> }) => {
if (changed.localMode != null) {
const localMode = changed.localMode || false;
// First switch the logger to it's correct state
this.logger.switchBackend(localMode);
// First switch the logger to it's correct state
this.logger.switchBackend(localMode);
// If we're leaving local mode, make sure to remove all of the
// leftover artifacts
if (!localMode) {
this.removeLocalModeArtifacts();
// If we're leaving local mode, make sure to remove all of the
// leftover artifacts
if (!localMode) {
this.removeLocalModeArtifacts();
}
}
}
});
},
);
// On startup, check if we're in unmanaged mode,
// as local mode needs to be set
let unmanagedLocalMode = false;
if (checkTruthy((await this.config.get('unmanaged')) || false)) {
if (await this.config.get('unmanaged')) {
console.log('Starting up in unmanaged mode, activating local mode');
await this.config.set({ localMode: true });
unmanagedLocalMode = true;
@ -51,8 +54,7 @@ export class LocalModeManager {
const localMode =
// short circuit the next get if we know we're in local mode
unmanagedLocalMode ||
checkTruthy((await this.config.get('localMode')) || false);
unmanagedLocalMode || (await this.config.get('localMode'));
if (!localMode) {
// Remove any leftovers if necessary

View File

@ -4,6 +4,7 @@ import * as _ from 'lodash';
import * as morgan from 'morgan';
import Config from './config';
import { SchemaReturn, SchemaTypeKey } from './config/schema-type';
import { EventTracker } from './event-tracker';
import blink = require('./lib/blink');
import * as iptables from './lib/iptables';
@ -141,24 +142,23 @@ export class SupervisorAPI {
port: number,
apiTimeout: number,
): Promise<void> {
const localMode = (await this.config.get('localMode')) || false;
await this.applyListeningRules(
checkTruthy(localMode) || false,
port,
allowedInterfaces,
);
const localMode = await this.config.get('localMode');
await this.applyListeningRules(localMode || false, port, allowedInterfaces);
// Monitor the switching of local mode, and change which interfaces will
// be listened to based on that
this.config.on('change', (changedConfig: Dictionary<string>) => {
if (changedConfig.localMode != null) {
this.applyListeningRules(
checkTruthy(changedConfig.localMode || false) || false,
port,
allowedInterfaces,
);
}
});
this.config.on(
'change',
(changedConfig: { [key in SchemaTypeKey]: SchemaReturn<key> }) => {
if (changedConfig.localMode != null) {
this.applyListeningRules(
changedConfig.localMode || false,
port,
allowedInterfaces,
);
}
},
);
this.server = this.api.listen(port);
this.server.timeout = apiTimeout;

View File

@ -7,7 +7,6 @@ EventEmitter = require 'events'
DeviceState = require './device-state'
{ SupervisorAPI } = require './supervisor-api'
{ Logger } = require './logger'
{ checkTruthy } = require './lib/validation'
constants = require './lib/constants'
@ -57,12 +56,12 @@ module.exports = class Supervisor extends EventEmitter
apiEndpoint: conf.apiEndpoint,
uuid: conf.uuid,
deviceApiKey: conf.deviceApiKey,
unmanaged: checkTruthy(conf.unmanaged),
enableLogs: checkTruthy(conf.loggingEnabled),
localMode: checkTruthy(conf.localMode)
unmanaged: conf.unmanaged,
enableLogs: conf.loggingEnabled,
localMode: conf.localMode
})
.then =>
if checkTruthy(conf.legacyAppsPresent)
if conf.legacyAppsPresent
console.log('Legacy app detected, running migration')
@deviceState.normaliseLegacy(@apiBinder.balenaApi)
.then =>