From 8f17c30de68f62477a52dd16a268e1d003c666ea Mon Sep 17 00:00:00 2001 From: Felipe Lalanne <1822826+pipex@users.noreply.github.com> Date: Wed, 2 Aug 2023 11:21:11 -0400 Subject: [PATCH] Replace dbus test service with mock-systemd-bus This avoids unnecessary mocking and tests against the real systemd API Change-type: patch --- docker-compose.dev.yml | 7 - docker-compose.test.yml | 40 ++--- test/integration/device-api/actions.spec.ts | 70 ++++++-- test/lib/dbus/.npmrc | 1 - test/lib/dbus/Dockerfile | 10 -- test/lib/dbus/entry.sh | 13 -- test/lib/dbus/login.ts | 66 -------- test/lib/dbus/package.json | 20 --- test/lib/dbus/systemd.ts | 173 -------------------- test/lib/dbus/tsconfig.json | 14 -- test/lib/dbus/utils.ts | 17 -- test/lib/legacy-mocha-hooks.ts | 70 -------- 12 files changed, 70 insertions(+), 431 deletions(-) delete mode 100644 test/lib/dbus/.npmrc delete mode 100644 test/lib/dbus/Dockerfile delete mode 100755 test/lib/dbus/entry.sh delete mode 100644 test/lib/dbus/login.ts delete mode 100644 test/lib/dbus/package.json delete mode 100644 test/lib/dbus/systemd.ts delete mode 100644 test/lib/dbus/tsconfig.json delete mode 100644 test/lib/dbus/utils.ts delete mode 100644 test/lib/legacy-mocha-hooks.ts diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 29c703d3..cdc3c60f 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -1,13 +1,6 @@ version: '2.3' services: - dbus-services: - environment: - DEVELOPMENT: 1 - volumes: - - './test/lib/dbus/systemd.ts:/usr/src/app/systemd.ts' - - './test/lib/dbus/login.ts:/usr/src/app/login.ts' - sut: command: sleep infinity volumes: diff --git a/docker-compose.test.yml b/docker-compose.test.yml index 88cc7b99..fbe966e2 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -12,41 +12,30 @@ services: # Use bridge networking for the tests environment: DOCKER_HOST: tcp://docker:2375 - DBUS_SYSTEM_BUS_ADDRESS: unix:path=/run/dbus/system_bus_socket + DBUS_SYSTEM_BUS_ADDRESS: unix:path=/shared/dbus/system_bus_socket # Required to skip device mounting in test env TEST: 1 depends_on: - docker - - dbus - - dbus-services + - mock-systemd volumes: - - dbus:/run/dbus + - dbus:/shared/dbus - ./test/data/root:/mnt/root - ./test/data/root/mnt/boot:/mnt/boot - ./test/lib/wait-for-it.sh:/wait-for-it.sh tmpfs: - /data # sqlite3 database - dbus: - image: balenablocks/dbus - stop_grace_period: 3s - environment: - DBUS_CONFIG: session.conf - DBUS_ADDRESS: unix:path=/run/dbus/system_bus_socket + # The service setup + mock-systemd: + image: ghcr.io/balena-os/mock-systemd-bus + # Necessary to run systemd in a container + privileged: true volumes: - - dbus:/run/dbus - - # Fake system service to listen for supervisor - # requests - dbus-services: - build: ./test/lib/dbus - stop_grace_period: 3s - depends_on: - - dbus - volumes: - - dbus:/run/dbus + - dbus:/shared/dbus environment: - DBUS_SYSTEM_BUS_ADDRESS: unix:path=/run/dbus/system_bus_socket + DBUS_SYSTEM_BUS_ADDRESS: unix:path=/shared/dbus/system_bus_socket + MOCK_SYSTEMD_UNITS: openvpn.service avahi.socket docker: image: docker:dind @@ -77,15 +66,14 @@ services: depends_on: - balena-supervisor-sut - docker - - dbus - - dbus-services + - mock-systemd stop_grace_period: 3s volumes: - - dbus:/run/dbus + - dbus:/shared/dbus # Set required supervisor configuration variables here environment: DOCKER_HOST: tcp://docker:2375 - DBUS_SYSTEM_BUS_ADDRESS: unix:path=/run/dbus/system_bus_socket + DBUS_SYSTEM_BUS_ADDRESS: unix:path=/shared/dbus/system_bus_socket BALENA_SUPERVISOR_ADDRESS: http://balena-supervisor-sut:48484 # Required by migrations CONFIG_MOUNT_POINT: /mnt/boot/config.json diff --git a/test/integration/device-api/actions.spec.ts b/test/integration/device-api/actions.spec.ts index 21469575..13ec6c20 100644 --- a/test/integration/device-api/actions.spec.ts +++ b/test/integration/device-api/actions.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import { stub, SinonStub, spy, SinonSpy } from 'sinon'; +import { stub, SinonStub } from 'sinon'; import * as Docker from 'dockerode'; import * as request from 'supertest'; import { setTimeout } from 'timers/promises'; @@ -10,9 +10,41 @@ import * as hostConfig from '~/src/host-config'; 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'; +import { exec } from '~/src/lib/fs-utils'; + +export async function dbusSend( + dest: string, + path: string, + message: string, + ...contents: string[] +) { + const { stdout, stderr } = await exec( + [ + 'dbus-send', + '--system', + `--dest=${dest}`, + '--print-reply', + path, + message, + ...contents, + ].join(' '), + { encoding: 'utf8' }, + ); + + if (stderr) { + throw new Error(stderr); + } + + // Remove first line, trim each line, and join them back together + return stdout + .split(/\r?\n/) + .slice(1) + .map((s) => s.trim()) + .join(''); +} + describe('regenerates API keys', () => { // Stub external dependency - current state report should be tested separately. // API key related methods are tested in api-keys.spec.ts. @@ -692,24 +724,34 @@ describe('manages application lifecycle', () => { }); describe('reboots or shuts down device', () => { - before(async () => { - spy(dbus, 'reboot'); - spy(dbus, 'shutdown'); - }); - - after(() => { - (dbus.reboot as SinonSpy).restore(); - (dbus.shutdown as SinonSpy).restore(); - }); - it('reboots device', async () => { await actions.executeDeviceAction({ action: 'reboot' }); - expect(dbus.reboot as SinonSpy).to.have.been.called; + // The reboot method delays the call by one second + await setTimeout(1500); + await expect( + dbusSend( + 'org.freedesktop.login1', + '/org/freedesktop/login1', + 'org.freedesktop.DBus.Properties.Get', + 'string:org.freedesktop.login1.Manager', + 'string:MockState', + ), + ).to.eventually.equal('variant string "rebooting"'); }); it('shuts down device', async () => { await actions.executeDeviceAction({ action: 'shutdown' }); - expect(dbus.shutdown as SinonSpy).to.have.been.called; + // The shutdown method delays the call by one second + await setTimeout(1500); + await expect( + dbusSend( + 'org.freedesktop.login1', + '/org/freedesktop/login1', + 'org.freedesktop.DBus.Properties.Get', + 'string:org.freedesktop.login1.Manager', + 'string:MockState', + ), + ).to.eventually.equal('variant string "off"'); }); }); diff --git a/test/lib/dbus/.npmrc b/test/lib/dbus/.npmrc deleted file mode 100644 index 43c97e71..00000000 --- a/test/lib/dbus/.npmrc +++ /dev/null @@ -1 +0,0 @@ -package-lock=false diff --git a/test/lib/dbus/Dockerfile b/test/lib/dbus/Dockerfile deleted file mode 100644 index 216cd497..00000000 --- a/test/lib/dbus/Dockerfile +++ /dev/null @@ -1,10 +0,0 @@ -FROM node:16-alpine - -RUN apk add --update python3 dbus-dev make g++ libgcc - -WORKDIR /usr/src/app -COPY package.json *.ts tsconfig.json entry.sh ./ - -RUN npm install && npm run build - -CMD ["./entry.sh"] diff --git a/test/lib/dbus/entry.sh b/test/lib/dbus/entry.sh deleted file mode 100755 index 2c3819c1..00000000 --- a/test/lib/dbus/entry.sh +++ /dev/null @@ -1,13 +0,0 @@ -#!/bin/sh - -if [ "${DEVELOPMENT}" = "1" ]; then - # Use nodemon in development mode - npx nodemon -w systemd.ts systemd.ts & - npx nodemon -w login.ts login.ts -else - # Launch services in separate processes. node-dbus for some - # reason blocks when trying to register multiple services - # on the same process - node systemd.js & - node login.js -fi diff --git a/test/lib/dbus/login.ts b/test/lib/dbus/login.ts deleted file mode 100644 index b23baf8b..00000000 --- a/test/lib/dbus/login.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { createSystemInterface } from './utils'; - -// Create login interface -const login = createSystemInterface( - 'org.freedesktop.login1', - '/org/freedesktop/login1', - 'org.freedesktop.login1.Manager', -); - -type SystemState = { status: 'ready' | 'rebooting' | 'off' }; -const systemState: SystemState = { status: 'ready' }; -login.addMethod( - 'Reboot', - { in: [{ type: 'b', name: 'interactive' }] } as any, - function (_interactive, callback) { - // Wait a bit before changing the runtime state - setTimeout(() => { - console.log('Rebooting'); - systemState.status = 'rebooting'; - }, 500); - - callback(null); - }, -); - -login.addMethod( - 'PowerOff', - { in: [{ type: 'b', name: 'interactive' }] } as any, - function (_interactive, callback) { - // Wait a bit before changing the runtime state - setTimeout(() => { - console.log('Powering off'); - systemState.status = 'off'; - }, 500); - - callback(null); - }, -); - -// This is not a real login interface method, but it will help for -// testing -login.addMethod( - 'PowerOn', - { in: [{ type: 'b', name: 'interactive' }] } as any, - function (_interactive, callback) { - // Wait a bit before changing the runtime state - setTimeout(() => { - console.log('Starting up'); - systemState.status = 'ready'; - }, 500); - - callback(null); - }, -); - -// This is not a real login interface method, but it will help for -// testing -login.addMethod( - 'GetState', - { out: { type: 's', name: 'state' } } as any, - function (callback: any) { - callback(null, systemState.status); - }, -); - -login.update(); diff --git a/test/lib/dbus/package.json b/test/lib/dbus/package.json deleted file mode 100644 index 27052203..00000000 --- a/test/lib/dbus/package.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "name": "dbus", - "version": "0.1.0", - "description": "OS dbus service spoofing", - "scripts": { - "test": "echo \"Error: no test specified\" && exit 1", - "build": "tsc" - }, - "author": "", - "license": "Apache-2.0", - "dependencies": { - "dbus": "^1.0.7" - }, - "devDependencies": { - "@types/dbus": "^1.0.3", - "nodemon": "^2.0.20", - "ts-node": "^10.9.1", - "typescript": "^4.8.4" - } -} diff --git a/test/lib/dbus/systemd.ts b/test/lib/dbus/systemd.ts deleted file mode 100644 index b12f93e4..00000000 --- a/test/lib/dbus/systemd.ts +++ /dev/null @@ -1,173 +0,0 @@ -import * as DBus from 'dbus'; -import { createSystemInterface } from './utils'; - -const systemdService = DBus.registerService( - 'system', - 'org.freedesktop.systemd1', -); - -// Create the systemd -const systemd = createSystemInterface( - systemdService, - '/org/freedesktop/systemd1', - 'org.freedesktop.systemd1.Manager', -); - -type Unit = { - running: boolean; - path: string; - partOf?: string; -}; - -// Maintain the state of created units in memory -const units: { [key: string]: Unit } = {}; -function createUnit(name: string, path: string, partOf?: string) { - // Each unit needs an object and a properties interface - const obj = systemdService.createObject(path); - const iface = obj.createInterface('org.freedesktop.DBus.Properties'); - - units[name] = { running: false, path, partOf }; - - // org.freedesktop.DBus.Properties needs a Get method to get the - // unit properties - iface.addMethod( - 'Get', - { - in: [ - { type: 's', name: 'interface_name' }, - { type: 's', name: 'property_name' }, - ], - out: { type: 'v' }, - } as any, - function (interfaceName, propertyName, callback: any) { - if (interfaceName !== 'org.freedesktop.systemd1.Unit') { - callback(`Unkown interface: ${interfaceName}`); - } - - switch (propertyName) { - case 'ActiveState': - callback(null, units[name].running ? 'active' : 'inactive'); - break; - case 'PartOf': - callback(partOf ?? 'none'); - break; - default: - callback(`Unknown property: ${propertyName}`); - } - }, - ); - - iface.update(); -} - -systemd.addMethod( - 'StopUnit', - { - in: [ - { type: 's', name: 'unit_name' }, - { type: 's', name: 'mode' }, - ], - out: { type: 'o' }, - } as any, - function (unitName, _mode, callback: any) { - if (!units[unitName]) { - callback(`Unit not found: ${unitName}`); - return; - } - - // Wait a bit before changing the runtime state - setTimeout(() => { - units[unitName] = { ...units[unitName], running: false }; - }, 500); - - callback( - null, - `/org/freedesktop/systemd1/job/${String( - Math.floor(Math.random() * 10000), - )}`, - ); - }, -); - -systemd.addMethod( - 'StartUnit', - { - in: [ - { type: 's', name: 'unit_name' }, - { type: 's', name: 'mode' }, - ], - out: { type: 'o' }, - } as any, - function (unitName, _mode, callback: any) { - if (!units[unitName]) { - callback(`Unit not found: ${unitName}`); - return; - } - - // Wait a bit before changing the runtime state - setTimeout(() => { - units[unitName] = { ...units[unitName], running: true }; - }, 500); - - callback( - null, - // Make up a job number - `/org/freedesktop/systemd1/job/${String( - Math.floor(Math.random() * 10000), - )}`, - ); - }, -); - -systemd.addMethod( - 'RestartUnit', - { - in: [ - { type: 's', name: 'unit_name' }, - { type: 's', name: 'mode' }, - ], - out: { type: 'o' }, - } as any, - function (unitName, _mode, callback: any) { - if (!units[unitName]) { - callback(`Unit not found: ${unitName}`); - return; - } - - // Wait a bit before changing the runtime state - setTimeout(() => { - units[unitName] = { ...units[unitName], running: false }; - }, 500); - - setTimeout(() => { - units[unitName] = { ...units[unitName], running: true }; - }, 1000); - - callback( - null, - `/org/freedesktop/systemd1/job/${String( - Math.floor(Math.random() * 10000), - )}`, - ); - }, -); - -systemd.addMethod( - 'GetUnit', - { in: [{ type: 's', name: 'unit_name' }], out: { type: 'o' } } as any, - function (unitName, callback) { - if (!units[unitName]) { - callback(`Unit not found: ${unitName}`); - return; - } - - const { path } = units[unitName]; - callback(null, path); - }, -); - -// Simulate OS units -createUnit('openvpn.service', '/org/freedesktop/systemd1/unit/openvpn'); -createUnit('avahi.socket', '/org/freedesktop/systemd1/unit/avahi'); - -systemd.update(); diff --git a/test/lib/dbus/tsconfig.json b/test/lib/dbus/tsconfig.json deleted file mode 100644 index f6532c51..00000000 --- a/test/lib/dbus/tsconfig.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "compilerOptions": { - "module": "commonjs", - "noUnusedParameters": true, - "noUnusedLocals": true, - "removeComments": true, - "sourceMap": true, - "strict": true, - "target": "es2019", - "declaration": true, - "skipLibCheck": true - }, - "include": ["*.ts"] -} diff --git a/test/lib/dbus/utils.ts b/test/lib/dbus/utils.ts deleted file mode 100644 index e7714620..00000000 --- a/test/lib/dbus/utils.ts +++ /dev/null @@ -1,17 +0,0 @@ -import * as DBus from 'dbus'; - -export function createSystemInterface( - svc: DBus.DBusService | string, - objName: string, - ifaceName: string, -) { - const service = ((s: DBus.DBusService | string) => { - if (typeof s === 'string') { - return DBus.registerService('system', s); - } - return s; - })(svc); - - const obj = service.createObject(objName); - return obj.createInterface(ifaceName); -} diff --git a/test/lib/legacy-mocha-hooks.ts b/test/lib/legacy-mocha-hooks.ts deleted file mode 100644 index 72946c68..00000000 --- a/test/lib/legacy-mocha-hooks.ts +++ /dev/null @@ -1,70 +0,0 @@ -// TODO: Remove this file when all legacy tests have migrated to unit/integration. - -import { stub, SinonStub } from 'sinon'; -import * as dbus from 'dbus'; -import { Error as DBusError, DBusInterface } from 'dbus'; -import { initialized } from '~/lib/dbus'; - -let getBusStub: SinonStub; - -export const mochaHooks = { - async beforeAll() { - getBusStub = stub(dbus, 'getBus').returns({ - getInterface: ( - serviceName: string, - _objectPath: string, - _interfaceName: string, - interfaceCb: (err: null | DBusError, iface: DBusInterface) => void, - ) => { - if (/systemd/.test(serviceName)) { - interfaceCb(null, { - StartUnit: () => { - // noop - }, - RestartUnit: () => { - // noop - }, - StopUnit: () => { - // noop - }, - EnableUnitFiles: () => { - // noop - }, - DisableUnitFiles: () => { - // noop - }, - GetUnit: ( - _unitName: string, - getUnitCb: (err: null | Error, unitPath: string) => void, - ) => { - getUnitCb(null, 'this is the unit path'); - }, - Get: ( - _unitName: string, - _property: string, - getCb: (err: null | Error, value: unknown) => void, - ) => { - getCb(null, 'this is the value'); - }, - } as any); - } else { - interfaceCb(null, { - Reboot: () => { - // noop - }, - PowerOff: () => { - // noop - }, - } as any); - } - }, - } as dbus.DBusConnection); - - // Initialize dbus module before any tests are run so any further tests - // that interface with lib/dbus use the stubbed busses above. - await initialized(); - }, - afterAll() { - getBusStub.restore(); - }, -};