From c9c0e650cb0f6655ac5a6c545e6b009c6fe6fa1c Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Tue, 7 Apr 2020 12:22:56 +0100 Subject: [PATCH] Support matching on device type within contracts Closes: #1191 Change-type: minor Signed-off-by: Cameron Diver --- src/application-manager.js | 64 ++++++++++++------------- src/lib/contracts.ts | 90 ++++++++++++++++++------------------ src/supervisor.ts | 8 ++++ test/05-device-state.spec.ts | 6 +++ test/24-contracts.ts | 87 ++++++++++++++++++---------------- 5 files changed, 138 insertions(+), 117 deletions(-) diff --git a/src/application-manager.js b/src/application-manager.js index 9aa65ea1..3f5fee48 100644 --- a/src/application-manager.js +++ b/src/application-manager.js @@ -1165,9 +1165,9 @@ export class ApplicationManager extends EventEmitter { } setTarget(apps, dependent, source, maybeTrx) { - const setInTransaction = (filteredApps, trx) => { + const setInTransaction = (filtered, trx) => { return Promise.try(() => { - const appsArray = _.map(filteredApps, function(app, appId) { + const appsArray = _.map(filtered, function(app, appId) { const appClone = _.clone(app); appClone.appId = checkInt(appId); appClone.source = source; @@ -1208,38 +1208,38 @@ export class ApplicationManager extends EventEmitter { // filter those out and add the target state to the database /** @type { { [appName: string]: string[]; } } */ const contractViolators = {}; - return Promise.resolve(validateTargetContracts(apps)) - .then(fulfilledContracts => { - const filteredApps = _.cloneDeep(apps); - _.each( - fulfilledContracts, - ( - { valid, unmetServices, fulfilledServices, unmetAndOptional }, - appId, - ) => { - if (!valid) { - contractViolators[apps[appId].name] = unmetServices; - return delete filteredApps[appId]; - } else { - // valid is true, but we could still be missing - // some optional containers, and need to filter - // these out of the target state - filteredApps[appId].services = _.pickBy( - filteredApps[appId].services, - ({ serviceName }) => fulfilledServices.includes(serviceName), - ); - if (unmetAndOptional.length !== 0) { - return this.reportOptionalContainers(unmetAndOptional); - } - } - }, - ); - if (maybeTrx != null) { - return setInTransaction(filteredApps, maybeTrx); + const fulfilledContracts = validateTargetContracts(apps); + const filteredApps = _.cloneDeep(apps); + _.each( + fulfilledContracts, + ( + { valid, unmetServices, fulfilledServices, unmetAndOptional }, + appId, + ) => { + if (!valid) { + contractViolators[apps[appId].name] = unmetServices; + return delete filteredApps[appId]; } else { - return this.db.transaction(setInTransaction); + // valid is true, but we could still be missing + // some optional containers, and need to filter + // these out of the target state + filteredApps[appId].services = _.pickBy( + filteredApps[appId].services, + ({ serviceName }) => fulfilledServices.includes(serviceName), + ); + if (unmetAndOptional.length !== 0) { + return this.reportOptionalContainers(unmetAndOptional); + } } - }) + }, + ); + let promise; + if (maybeTrx != null) { + promise = setInTransaction(filteredApps, maybeTrx); + } else { + promise = this.db.transaction(setInTransaction); + } + return promise .then(() => { this._targetVolatilePerImageId = {}; }) diff --git a/src/lib/contracts.ts b/src/lib/contracts.ts index 66c00fc3..582875bd 100644 --- a/src/lib/contracts.ts +++ b/src/lib/contracts.ts @@ -6,8 +6,6 @@ import * as _ from 'lodash'; import { Blueprint, Contract, ContractObject } from '@balena/contrato'; import { ContractValidationError, InternalInconsistencyError } from './errors'; -import * as osRelease from './os-release'; -import supervisorVersion = require('./supervisor-version'); import { checkTruthy } from './validation'; export { ContractObject }; @@ -35,50 +33,43 @@ export interface ServiceContracts { [serviceName: string]: { contract?: ContractObject; optional: boolean }; } -type PotentialContractRequirements = 'sw.supervisor' | 'sw.l4t'; -type ContractRequirementVersions = { +type PotentialContractRequirements = + | 'sw.supervisor' + | 'sw.l4t' + | 'hw.device-type'; +type ContractRequirements = { [key in PotentialContractRequirements]?: string; }; -let contractRequirementVersions = async () => { - const versions: ContractRequirementVersions = { - 'sw.supervisor': supervisorVersion, - }; - // We add a mock l4t version if one doesn't exist. This - // means that contracts used on mixed device fleets will - // still work when only a subset of the devices have an - // l4t string (for example a mixed fleet of rpi4 and tx2) - versions['sw.l4t'] = (await osRelease.getL4tVersion()) || '0'; +const contractRequirementVersions: ContractRequirements = {}; - return versions; -}; - -// When running in tests, we need this function to be -// repeatedly executed, but on-device, this should only be -// executed once per run -if (process.env.TEST !== '1') { - contractRequirementVersions = _.once(contractRequirementVersions); +export function intialiseContractRequirements(opts: { + supervisorVersion: string; + deviceType: string; + l4tVersion?: string; +}) { + contractRequirementVersions['sw.supervisor'] = opts.supervisorVersion; + contractRequirementVersions['sw.l4t'] = opts.l4tVersion; + contractRequirementVersions['hw.device-type'] = opts.deviceType; } function isValidRequirementType( - requirementVersions: ContractRequirementVersions, + requirementVersions: ContractRequirements, requirement: string, ) { return requirement in requirementVersions; } -export async function containerContractsFulfilled( +export function containerContractsFulfilled( serviceContracts: ServiceContracts, -): Promise { +): ApplicationContractResult { const containers = _(serviceContracts) .map('contract') .compact() .value(); - const versions = await contractRequirementVersions(); - const blueprintMembership: Dictionary = {}; - for (const component of _.keys(versions)) { + for (const component of _.keys(contractRequirementVersions)) { blueprintMembership[component] = 1; } const blueprint = new Blueprint( @@ -97,9 +88,10 @@ export async function containerContractsFulfilled( }); universe.addChildren( - [...getContractsFromVersions(versions), ...containers].map( - c => new Contract(c), - ), + [ + ...getContractsFromVersions(contractRequirementVersions), + ...containers, + ].map(c => new Contract(c)), ); const solution = blueprint.reproduce(universe); @@ -182,23 +174,33 @@ const contractObjectValidator = t.type({ ]), }); -function getContractsFromVersions(versions: ContractRequirementVersions) { - return _.map(versions, (version, component) => ({ - type: component, - slug: component, - name: component, - version, - })); +function getContractsFromVersions(components: ContractRequirements) { + return _.map(components, (value, component) => { + if (component === 'hw.device-type') { + return { + type: component, + slug: component, + name: value, + }; + } else { + return { + type: component, + slug: component, + name: component, + version: value, + }; + } + }); } -export async function validateContract(contract: unknown): Promise { +export function validateContract(contract: unknown): boolean { const result = contractObjectValidator.decode(contract); if (isLeft(result)) { throw new Error(reporter(result).join('\n')); } - const requirementVersions = await contractRequirementVersions(); + const requirementVersions = contractRequirementVersions; for (const { type } of result.right.requires || []) { if (!isValidRequirementType(requirementVersions, type)) { @@ -208,9 +210,9 @@ export async function validateContract(contract: unknown): Promise { return true; } -export async function validateTargetContracts( +export function validateTargetContracts( apps: Dictionary, -): Promise> { +): Dictionary { const appsFulfilled: Dictionary = {}; for (const appId of _.keys(apps)) { @@ -222,7 +224,7 @@ export async function validateTargetContracts( if (svc.contract) { try { - await validateContract(svc.contract); + validateContract(svc.contract); serviceContracts[svc.serviceName] = { contract: svc.contract, @@ -240,9 +242,7 @@ export async function validateTargetContracts( } if (!_.isEmpty(serviceContracts)) { - appsFulfilled[appId] = await containerContractsFulfilled( - serviceContracts, - ); + appsFulfilled[appId] = containerContractsFulfilled(serviceContracts); } else { appsFulfilled[appId] = { valid: true, diff --git a/src/supervisor.ts b/src/supervisor.ts index b84ec8de..2dc9edae 100644 --- a/src/supervisor.ts +++ b/src/supervisor.ts @@ -3,7 +3,9 @@ import Config, { ConfigKey } from './config'; import Database from './db'; import DeviceState from './device-state'; import EventTracker from './event-tracker'; +import { intialiseContractRequirements } from './lib/contracts'; import { normaliseLegacyDatabase } from './lib/migration'; +import * as osRelease from './lib/os-release'; import Logger from './logger'; import SupervisorAPI from './supervisor-api'; @@ -93,6 +95,12 @@ export class Supervisor { ...conf, }); + intialiseContractRequirements({ + supervisorVersion: version, + deviceType: await this.config.get('deviceType'), + l4tVersion: await osRelease.getL4tVersion(), + }); + log.debug('Starting api binder'); await this.apiBinder.initClient(); diff --git a/test/05-device-state.spec.ts b/test/05-device-state.spec.ts index 77150588..65227aa3 100644 --- a/test/05-device-state.spec.ts +++ b/test/05-device-state.spec.ts @@ -17,6 +17,7 @@ import DeviceState from '../src/device-state'; import { loadTargetFromFile } from '../src/device-state/preload'; import Service from '../src/compose/service'; +import { intialiseContractRequirements } from '../src/lib/contracts'; const mockedInitialConfig = { RESIN_SUPERVISOR_CONNECTIVITY_CHECK: 'true', @@ -227,6 +228,11 @@ describe('deviceState', () => { return env; }); + intialiseContractRequirements({ + supervisorVersion: '11.0.0', + deviceType: 'intel-nuc', + }); + deviceState = new DeviceState({ db, config, diff --git a/test/24-contracts.ts b/test/24-contracts.ts index 3dbfc3cb..20fa8f01 100644 --- a/test/24-contracts.ts +++ b/test/24-contracts.ts @@ -7,53 +7,48 @@ import * as semver from 'semver'; import * as constants from '../src/lib/constants'; import { containerContractsFulfilled, + intialiseContractRequirements, validateContract, } from '../src/lib/contracts'; import * as osRelease from '../src/lib/os-release'; import supervisorVersion = require('../src/lib/supervisor-version'); describe('Container contracts', () => { - let execStub: SinonStub; before(() => { - execStub = stub(child_process, 'exec').returns( - Promise.resolve([ - Buffer.from('4.9.140-l4t-r32.2+g3dcbed5'), - Buffer.from(''), - ]), - ); - }); - - after(() => { - execStub.restore(); + intialiseContractRequirements({ + supervisorVersion: '11.0.0', + deviceType: 'intel-nuc', + l4tVersion: '32.2', + }); }); describe('Contract validation', () => { it('should correctly validate a contract with no requirements', () => - expect( + expect(() => validateContract({ slug: 'user-container', }), - ).to.be.fulfilled); + ).to.be.not.throw()); it('should correctly validate a contract with extra fields', () => - expect( + expect(() => validateContract({ slug: 'user-container', name: 'user-container', version: '3.0.0', }), - ).to.be.fulfilled); + ).to.be.not.throw()); it('should not validate a contract without the minimum required fields', () => { return Promise.all([ - expect(validateContract({})).to.be.rejected, - expect(validateContract({ name: 'test' })).to.be.rejected, - expect(validateContract({ requires: [] })).to.be.rejected, + expect(() => validateContract({})).to.throw(), + expect(() => validateContract({ name: 'test' })).to.throw(), + expect(() => validateContract({ requires: [] })).to.throw(), ]); }); it('should correctly validate a contract with requirements', () => - expect( + expect(() => validateContract({ slug: 'user-container', requires: [ @@ -66,10 +61,10 @@ describe('Container contracts', () => { }, ], }), - ).to.be.fulfilled); + ).to.not.throw()); it('should not validate a contract with requirements without the minimum required fields', () => { - return expect( + return expect(() => validateContract({ slug: 'user-container', requires: [ @@ -78,7 +73,7 @@ describe('Container contracts', () => { }, ], }), - ).to.be.rejected; + ).to.throw(); }); }); @@ -105,7 +100,7 @@ describe('Container contracts', () => { it('Should correctly run containers with no requirements', async () => { expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -118,7 +113,7 @@ describe('Container contracts', () => { .to.have.property('valid') .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -141,7 +136,7 @@ describe('Container contracts', () => { it('should correctly run containers whose requirements are satisfied', async () => { expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -162,7 +157,7 @@ describe('Container contracts', () => { .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -183,7 +178,7 @@ describe('Container contracts', () => { .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -204,7 +199,7 @@ describe('Container contracts', () => { .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -228,7 +223,7 @@ describe('Container contracts', () => { .to.have.property('valid') .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -264,7 +259,7 @@ describe('Container contracts', () => { }); it('Should refuse to run containers whose requirements are not satisfied', async () => { - let fulfilled = await containerContractsFulfilled({ + let fulfilled = containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -287,7 +282,7 @@ describe('Container contracts', () => { .to.have.property('unmetServices') .that.deep.equals(['service']); - fulfilled = await containerContractsFulfilled({ + fulfilled = containerContractsFulfilled({ service2: { contract: { type: 'sw.container', @@ -310,7 +305,7 @@ describe('Container contracts', () => { .to.have.property('unmetServices') .that.deep.equals(['service2']); - fulfilled = await containerContractsFulfilled({ + fulfilled = containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -354,7 +349,7 @@ describe('Container contracts', () => { valid, unmetServices, fulfilledServices, - } = await containerContractsFulfilled({ + } = containerContractsFulfilled({ service1: { contract: { type: 'sw.container', @@ -379,7 +374,7 @@ describe('Container contracts', () => { valid, unmetServices, fulfilledServices, - } = await containerContractsFulfilled({ + } = containerContractsFulfilled({ service1: { contract: { type: 'sw.container', @@ -445,11 +440,23 @@ describe('L4T version detection', () => { }); describe('L4T comparison', () => { + const seedEngine = async (version: string) => { + if (execStub != null) { + execStub.restore(); + } + seedExec(version); + intialiseContractRequirements({ + supervisorVersion: '11.0.0', + deviceType: 'intel-nuc', + l4tVersion: await osRelease.getL4tVersion(), + }); + }; + it('should allow semver matching even when l4t does not fulfill semver', async () => { - seedExec('4.4.38-l4t-r31.0'); + await seedEngine('4.4.38-l4t-r31.0'); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -469,7 +476,7 @@ describe('L4T version detection', () => { .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -490,10 +497,10 @@ describe('L4T version detection', () => { }); it('should allow semver matching when l4t does fulfill semver', async () => { - seedExec('4.4.38-l4t-r31.0.1'); + await seedEngine('4.4.38-l4t-r31.0.1'); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container', @@ -513,7 +520,7 @@ describe('L4T version detection', () => { .that.equals(true); expect( - await containerContractsFulfilled({ + containerContractsFulfilled({ service: { contract: { type: 'sw.container',