From 63afeed60b2ea2ae8500571460bb5ed9327c50e4 Mon Sep 17 00:00:00 2001 From: Robert Gingras Date: Tue, 29 Sep 2026 11:35:43 -0400 Subject: [PATCH 1/5] volumes: add the Volume model and host-path helpers (#421) Introduce the data layer for user-defined container volumes, replacing the hardcoded quick_and_dirty mp0 stopgap (removed in the next commit). - Volume model + Volumes table: per-container name, hostPath, mountPath, ro/rw mode, scope, a `builtin` flag (legacy quick_and_dirty rows only), and a pending/ready/failed status with statusMessage/appliedAt that the create job will block on. Unique per container by name and by mount path. - Validation: names are single safe path segments; mount paths are absolute, canonicalized (duplicate/trailing slashes), and may not contain provider delimiters (',' ':'), backslashes, whitespace, control characters, or '.'/ '..' segments, since they are interpolated into Proxmox mpN and Docker bind syntax. - Volume.buildMountConfig renders rows to mpN. Legacy builtin rows are never rendered, but their low indices are reserved so user volumes added to a pre-existing container never overwrite its still-live mp0. - utils/volumes.js: - resolveVolumesRoot derives /volumes from the node storage's ACTUAL configured path (new ProxmoxApi.storageConfig), never assuming the /mnt/pve/ layout, and rejects block storage without a path. - Host paths are /site-///: site-scoped because hostnames are only unique per site, owner-scoped so retained data is only ever reattached for the same owner. Unsafe owner segments are refused, not rewritten. - isAgentlessNodeType: docker and dummy volumes don't use the site agent and are marked ready locally. - DockerApi maps mpN values to HostConfig.Binds on recreate; DummyApi gains a simulated storageConfig. --- .../20260722120000-create-volumes.js | 99 ++++++++ create-a-container/models/container.js | 6 + create-a-container/models/volume.js | 224 ++++++++++++++++++ .../utils/__tests__/docker-volumes.test.js | 29 +++ .../utils/__tests__/volumes.test.js | 198 ++++++++++++++++ create-a-container/utils/docker-api.js | 41 +++- create-a-container/utils/dummy-api.js | 15 ++ create-a-container/utils/proxmox-api.js | 16 ++ create-a-container/utils/volumes.js | 167 +++++++++++++ 9 files changed, 794 insertions(+), 1 deletion(-) create mode 100644 create-a-container/migrations/20260722120000-create-volumes.js create mode 100644 create-a-container/models/volume.js create mode 100644 create-a-container/utils/__tests__/docker-volumes.test.js create mode 100644 create-a-container/utils/__tests__/volumes.test.js create mode 100644 create-a-container/utils/volumes.js diff --git a/create-a-container/migrations/20260722120000-create-volumes.js b/create-a-container/migrations/20260722120000-create-volumes.js new file mode 100644 index 00000000..8fc8d2ad --- /dev/null +++ b/create-a-container/migrations/20260722120000-create-volumes.js @@ -0,0 +1,99 @@ +'use strict'; +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.createTable('Volumes', { + id: { + allowNull: false, + autoIncrement: true, + primaryKey: true, + type: Sequelize.INTEGER, + }, + containerId: { + type: Sequelize.INTEGER, + allowNull: false, + references: { model: 'Containers', key: 'id' }, + onDelete: 'CASCADE', + }, + name: { + type: Sequelize.STRING(255), + allowNull: false, + }, + // Host directory bind-mounted into the container. Derived from the node + // storage's actual configured path (never assumed to be /mnt/pve/). + // Null until the create job derives it. + hostPath: { + type: Sequelize.STRING(1024), + allowNull: true, + }, + // Guest mount point (e.g. /mnt/data). + mountPath: { + type: Sequelize.STRING(1024), + allowNull: false, + }, + mode: { + type: Sequelize.ENUM('ro', 'rw'), + allowNull: false, + defaultValue: 'rw', + }, + // Anticipates per-user/site/node volumes; v1 is per-container only. + scope: { + type: Sequelize.STRING(50), + allowNull: false, + defaultValue: 'container', + }, + // Whether this is the built-in shared read-only volume (formerly the + // hardcoded quick_and_dirty mp0). Built-ins are never rendered as a + // separate agent-provisioned directory and are seeded ready. + builtin: { + type: Sequelize.BOOLEAN, + allowNull: false, + defaultValue: false, + }, + // Directory-readiness lifecycle owned by the agent (pending -> ready|failed). + // The create job blocks on this before setting the bind mount. + status: { + type: Sequelize.ENUM('pending', 'ready', 'failed'), + allowNull: false, + defaultValue: 'pending', + }, + statusMessage: { + type: Sequelize.STRING(2000), + allowNull: true, + }, + appliedAt: { + type: Sequelize.DATE, + allowNull: true, + }, + createdAt: { + allowNull: false, + type: Sequelize.DATE, + }, + updatedAt: { + allowNull: false, + type: Sequelize.DATE, + }, + }); + + // A container can only mount one volume per host path and one per mount + // point; both are enforced so a reconfigure can't double-attach. + await queryInterface.addIndex('Volumes', ['containerId', 'name'], { + unique: true, + name: 'volumes_container_id_name_unique', + }); + await queryInterface.addIndex('Volumes', ['containerId', 'mountPath'], { + unique: true, + name: 'volumes_container_id_mount_path_unique', + }); + }, + + async down(queryInterface, Sequelize) { + await queryInterface.dropTable('Volumes'); + // Postgres materializes ENUMs as types that dropTable does not remove. + const dialect = queryInterface.sequelize.getDialect(); + if (dialect === 'postgres') { + await queryInterface.sequelize.query('DROP TYPE IF EXISTS "enum_Volumes_mode";'); + await queryInterface.sequelize.query('DROP TYPE IF EXISTS "enum_Volumes_status";'); + } + }, +}; diff --git a/create-a-container/models/container.js b/create-a-container/models/container.js index c5b496fa..f5a666f6 100644 --- a/create-a-container/models/container.js +++ b/create-a-container/models/container.js @@ -24,6 +24,12 @@ module.exports = (sequelize, DataTypes) => { as: 'collaborators', onDelete: 'CASCADE', }); + // a container has zero or more volumes (bind mounts) + Container.hasMany(models.Volume, { + foreignKey: 'containerId', + as: 'volumes', + onDelete: 'CASCADE', + }); } /** diff --git a/create-a-container/models/volume.js b/create-a-container/models/volume.js new file mode 100644 index 00000000..962bf535 --- /dev/null +++ b/create-a-container/models/volume.js @@ -0,0 +1,224 @@ +'use strict'; +const { Model } = require('sequelize'); + +// Name/mount of the retired hardcoded shared mount. The stopgap is fully +// removed: no new container is auto-attached this volume. The name/mount are +// retained only so (a) the backfill migration can record the already-live mount +// on pre-#421 containers as a `builtin` row, and (b) the API can reject a user +// volume that would collide with such a row. +const QUICK_AND_DIRTY_NAME = 'quick_and_dirty'; +const QUICK_AND_DIRTY_MOUNT = '/mnt/quick_and_dirty'; + +// A volume name must be a safe single path segment: no traversal (`..`), no +// path separators, no NUL, and no leading dot. This is the single source of +// truth for name validation, used at ingest and when deriving the host path. +const VALID_NAME = /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/; + +/** + * Validate a user-supplied volume name. Rejects traversal, separators, and + * anything that isn't a conventional safe path segment. + * @param {*} name + * @returns {boolean} + */ +function isValidVolumeName(name) { + if (typeof name !== 'string') return false; + if (name === '.' || name === '..') return false; + if (name.includes('/') || name.includes('\\') || name.includes('\0')) return false; + return VALID_NAME.test(name); +} + +/** + * Canonicalize an absolute guest mount path to a single normal form: + * collapse duplicate slashes and strip a trailing slash (except root). This is + * what the reserved-path and per-container uniqueness checks compare against, + * so equivalent spellings (`/mnt/x/`, `/mnt//x`) can't slip past them. + * Returns null if the input is not a usable absolute path. + * @param {*} mountPath + * @returns {string|null} + */ +function canonicalizeMountPath(mountPath) { + if (typeof mountPath !== 'string' || !mountPath.startsWith('/')) return null; + const collapsed = mountPath.replace(/\/{2,}/g, '/').replace(/\/+$/, ''); + return collapsed === '' ? '/' : collapsed; +} + +/** + * Validate a guest mount path. The value is interpolated into comma-delimited + * Proxmox `mpN` syntax (`,mp=,ro=`) and colon-delimited Docker + * bind syntax (`:[:ro]`), so beyond "absolute" it must contain no + * provider delimiters (`,` `:`), no backslash, no whitespace, no control + * characters (incl. NUL/newline), and no `.`/`..` traversal segments — any of + * which could corrupt the provider config or override mount options. + * @param {*} mountPath + * @returns {boolean} + */ +function isValidMountPath(mountPath) { + const canon = canonicalizeMountPath(mountPath); + if (!canon) return false; + // No provider delimiters, backslash, whitespace, or control chars anywhere. + // eslint-disable-next-line no-control-regex + if (/[,:\\\s\u0000-\u001f\u007f]/.test(canon)) return false; + // Reject traversal / relative segments. + const segments = canon.split('/').slice(1); // drop leading '' from root + for (const seg of segments) { + if (seg === '.' || seg === '..') return false; + } + return canon === '/' ? false : true; // a bare '/' mount is not meaningful +} + +module.exports = (sequelize, DataTypes) => { + class Volume extends Model { + static associate(models) { + Volume.belongsTo(models.Container, { foreignKey: 'containerId', as: 'container' }); + } + + /** + * Render this volume to a Proxmox bind-mount `mpN` config value: + * `,mp=,ro=<0|1>`. + * @returns {string} + */ + toMpValue() { + const ro = this.mode === 'ro' ? 1 : 0; + return `${this.hostPath},mp=${this.mountPath},ro=${ro}`; + } + + /** + * Build the `{ mp1, mp2, ... }` config fragment for a set of volumes. + * + * This is the single place that maps Volume records to Proxmox mount-point + * keys, so both the create and reconfigure paths render identical config. + * + * Legacy `builtin` rows (a backfill artifact for pre-#421 containers whose + * `quick_and_dirty` mount is still live at `mp0`) are NOT rendered — their + * host path is unknown and the live mount must be preserved. Crucially, the + * indices they occupy are RESERVED: user volumes are numbered starting AFTER + * the built-in rows, so the first user volume added to a pre-#421 container + * lands on `mp` (e.g. `mp1`) and never overwrites the live + * `mp0`. Because updateLxcConfig is a partial update, omitting the reserved + * low indices leaves the legacy mount untouched. Rows without a derived host + * path are also skipped. + * + * @param {Volume[]} volumes + * @returns {object} e.g. { mp1: '...', mp2: '...' } (mp0 reserved for a + * pre-existing built-in mount when present) + */ + static buildMountConfig(volumes) { + const config = {}; + // Reserve one low index per legacy built-in mount so user volumes never + // collide with a still-live mp0 on a pre-#421 container. + const reserved = volumes.filter((v) => v.builtin).length; + const mountable = volumes.filter((v) => !v.builtin && v.hostPath); + const sorted = [...mountable].sort((a, b) => a.id - b.id); + sorted.forEach((v, i) => { + config[`mp${reserved + i}`] = v.toMpValue(); + }); + return config; + } + + static get QUICK_AND_DIRTY_NAME() { + return QUICK_AND_DIRTY_NAME; + } + + static get QUICK_AND_DIRTY_MOUNT() { + return QUICK_AND_DIRTY_MOUNT; + } + + static isValidName(name) { + return isValidVolumeName(name); + } + + static isValidMountPath(mountPath) { + return isValidMountPath(mountPath); + } + + static canonicalizeMountPath(mountPath) { + return canonicalizeMountPath(mountPath); + } + } + + Volume.init( + { + containerId: { + type: DataTypes.INTEGER, + allowNull: false, + references: { model: 'Containers', key: 'id' }, + }, + name: { + type: DataTypes.STRING(255), + allowNull: false, + validate: { + isSafeName(value) { + if (!isValidVolumeName(value)) { + throw new Error( + 'Volume name must be a safe path segment (letters, digits, dot, dash, underscore; no traversal or separators)', + ); + } + }, + }, + }, + hostPath: { + type: DataTypes.STRING(1024), + allowNull: true, + }, + mountPath: { + type: DataTypes.STRING(1024), + allowNull: false, + validate: { + isSafeMountPath(value) { + if (!isValidMountPath(value)) { + throw new Error( + 'Volume mountPath must be an absolute path with no provider delimiters ' + + "(',' ':'), backslashes, whitespace, control characters, or '.'/'..' segments", + ); + } + }, + }, + }, + mode: { + type: DataTypes.ENUM('ro', 'rw'), + allowNull: false, + defaultValue: 'rw', + }, + scope: { + type: DataTypes.STRING(50), + allowNull: false, + defaultValue: 'container', + }, + builtin: { + type: DataTypes.BOOLEAN, + allowNull: false, + defaultValue: false, + }, + status: { + type: DataTypes.ENUM('pending', 'ready', 'failed'), + allowNull: false, + defaultValue: 'pending', + }, + statusMessage: { + type: DataTypes.STRING(2000), + allowNull: true, + }, + appliedAt: { + type: DataTypes.DATE, + allowNull: true, + }, + }, + { + sequelize, + modelName: 'Volume', + indexes: [ + { unique: true, fields: ['containerId', 'name'] }, + { unique: true, fields: ['containerId', 'mountPath'] }, + ], + }, + ); + + return Volume; +}; + +// Constants/validator exported on the factory for non-ORM consumers +// (utils/volumes.js, routers). models/index.js only invokes the factory and +// keys the result by `model.name`, so these attachments don't affect the ORM. +module.exports.isValidVolumeName = isValidVolumeName; +module.exports.QUICK_AND_DIRTY_NAME = QUICK_AND_DIRTY_NAME; +module.exports.QUICK_AND_DIRTY_MOUNT = QUICK_AND_DIRTY_MOUNT; diff --git a/create-a-container/utils/__tests__/docker-volumes.test.js b/create-a-container/utils/__tests__/docker-volumes.test.js new file mode 100644 index 00000000..ebd43614 --- /dev/null +++ b/create-a-container/utils/__tests__/docker-volumes.test.js @@ -0,0 +1,29 @@ +/** + * Docker volume mapping (issue #421): Proxmox mpN bind-mount config values are + * translated to Docker HostConfig.Binds (`:[:ro]`). + */ + +const { mpConfigToDockerBinds } = require('../docker-api'); + +describe('mpConfigToDockerBinds', () => { + test('maps rw and ro mounts, ignores non-mp keys', () => { + const binds = mpConfigToDockerBinds({ + cores: 4, + mp0: '/srv/volumes/web/data,mp=/mnt/data,ro=0', + mp1: '/srv/volumes/shared/quick_and_dirty,mp=/mnt/quick_and_dirty,ro=1', + env: 'FOO=bar', + }); + expect(binds).toEqual([ + '/srv/volumes/web/data:/mnt/data', + '/srv/volumes/shared/quick_and_dirty:/mnt/quick_and_dirty:ro', + ]); + }); + + test('returns empty for config without mp keys', () => { + expect(mpConfigToDockerBinds({ cores: 2, memory: 512 })).toEqual([]); + }); + + test('skips malformed mp entries missing mp=', () => { + expect(mpConfigToDockerBinds({ mp0: '/only/host/path' })).toEqual([]); + }); +}); diff --git a/create-a-container/utils/__tests__/volumes.test.js b/create-a-container/utils/__tests__/volumes.test.js new file mode 100644 index 00000000..24ba3a4a --- /dev/null +++ b/create-a-container/utils/__tests__/volumes.test.js @@ -0,0 +1,198 @@ +/** + * Volume model + utils/volumes pure-logic tests (issue #421): name validation, + * mpN rendering (including that legacy built-in / underived rows are skipped), + * and host-path derivation from the node storage's ACTUAL configured path. + */ + +const { resetDb, closeDb } = require('../../tests/helpers/db'); +const { Volume } = require('../../models'); +const volumeModule = require('../../models/volume'); +const { + resolveVolumesRoot, + containerVolumeHostPath, +} = require('../volumes'); + +describe('Volume name validation', () => { + test.each([ + ['data', true], + ['my-volume_1', true], + ['a.b.c', true], + ['', false], + ['.', false], + ['..', false], + ['../etc', false], + ['a/b', false], + ['a\\b', false], + ['-leading-dash-ok?', true], // starts with dash is disallowed by regex? see below + ])('%s -> %s', (name, expected) => { + // Adjust expectation for the one intentionally-tricky case: names must start + // with an alphanumeric. + const actual = volumeModule.isValidVolumeName(name); + if (name === '-leading-dash-ok?') { + expect(actual).toBe(false); + return; + } + expect(actual).toBe(expected); + }); + + test('rejects NUL and traversal', () => { + expect(volumeModule.isValidVolumeName('a\0b')).toBe(false); + expect(volumeModule.isValidVolumeName('..')).toBe(false); + expect(volumeModule.isValidVolumeName('foo/../bar')).toBe(false); + }); +}); + +describe('Volume.buildMountConfig', () => { + beforeAll(async () => { + await resetDb(); + }); + + test('renders contiguous mpN in id order with ro flag', () => { + // Build unsaved instances so toMpValue/id behave like real rows. + const a = Volume.build({ id: 1, containerId: 1, hostPath: '/data/c/a', mountPath: '/mnt/a', mode: 'ro' }); + const b = Volume.build({ id: 3, containerId: 1, hostPath: '/data/c/b', mountPath: '/mnt/b', mode: 'rw' }); + const cfg = Volume.buildMountConfig([b, a]); + expect(cfg).toEqual({ + mp0: '/data/c/a,mp=/mnt/a,ro=1', + mp1: '/data/c/b,mp=/mnt/b,ro=0', + }); + }); + + test('empty set renders no keys', () => { + expect(Volume.buildMountConfig([])).toEqual({}); + }); + + test('reserves the legacy mp0 slot: user volumes render after built-in rows', () => { + const user = Volume.build({ id: 1, containerId: 1, hostPath: '/data/c/data', mountPath: '/mnt/data', mode: 'rw' }); + const builtin = Volume.build({ + id: 2, + containerId: 1, + name: 'quick_and_dirty', + hostPath: null, + mountPath: '/mnt/quick_and_dirty', + mode: 'ro', + builtin: true, + }); + const underived = Volume.build({ id: 3, containerId: 1, hostPath: null, mountPath: '/mnt/x', mode: 'rw' }); + // The built-in reserves mp0 (its live mount), so the user volume lands on + // mp1 and never overwrites the pre-#421 quick_and_dirty mount. The + // host-path-less user row is skipped. + expect(Volume.buildMountConfig([user, builtin, underived])).toEqual({ + mp1: '/data/c/data,mp=/mnt/data,ro=0', + }); + }); + + test('no reserved slot when there is no built-in row (starts at mp0)', () => { + const a = Volume.build({ id: 1, containerId: 1, hostPath: '/data/c/a', mountPath: '/mnt/a', mode: 'rw' }); + expect(Volume.buildMountConfig([a])).toEqual({ mp0: '/data/c/a,mp=/mnt/a,ro=0' }); + }); +}); + +describe('containerVolumeHostPath', () => { + test('scopes user volumes by site, owner, and hostname under the volumes root', () => { + expect(containerVolumeHostPath('/srv/volumes', 7, 'alice', 'web01', 'data')).toBe( + '/srv/volumes/site-7/alice/web01/data', + ); + }); + + test('a different owner reusing the same hostname gets a different directory', () => { + const alice = containerVolumeHostPath('/srv/volumes', 7, 'alice', 'web01', 'data'); + const bob = containerVolumeHostPath('/srv/volumes', 7, 'bob', 'web01', 'data'); + expect(bob).not.toBe(alice); + }); + + test.each(['', '../evil', 'a/b', '.hidden', '-dash', 'has space', undefined])( + 'refuses an unsafe owner segment %p', + (owner) => { + expect(() => containerVolumeHostPath('/srv/volumes', 7, owner, 'web01', 'data')).toThrow( + /not a safe path segment/, + ); + }, + ); +}); + +describe('Volume.isValidMountPath / canonicalizeMountPath', () => { + test.each(['/mnt/data', '/mnt/a/b/c', '/srv/vol'])('accepts %s', (p) => { + expect(Volume.isValidMountPath(p)).toBe(true); + }); + + test.each([ + 'rel/ative', + '/', + '/mnt/a,ro=0', + '/mnt/a:b', + '/mnt/a\\b', + '/mnt/a b', + '/mnt/a\nb', + '/mnt/../etc', + '/mnt/./x', + ])('rejects %s', (p) => { + expect(Volume.isValidMountPath(p)).toBe(false); + }); + + test('canonicalizes duplicate and trailing slashes', () => { + expect(Volume.canonicalizeMountPath('/mnt//data/')).toBe('/mnt/data'); + expect(Volume.canonicalizeMountPath('/mnt/quick_and_dirty/')).toBe('/mnt/quick_and_dirty'); + }); +}); + +describe('resolveVolumesRoot', () => { + const node = { name: 'pve1', nodeType: 'proxmox', volumeStorage: 'cephfs', imageStorage: 'local' }; + + test('derives the root from the storage config path (not /mnt/pve assumption)', async () => { + const client = { + async storageConfig(storage) { + expect(storage).toBe('cephfs'); + return { storage, type: 'cephfs', path: '/mnt/pve/cephfs/', shared: 1 }; + }, + }; + const res = await resolveVolumesRoot(client, node); + expect(res).toEqual({ root: '/mnt/pve/cephfs/volumes', storage: 'cephfs', shared: true }); + }); + + test('throws when the storage has no host path (block storage)', async () => { + const client = { + async storageConfig() { + return { storage: 'local-lvm', type: 'lvmthin', shared: 0 }; + }, + }; + await expect(resolveVolumesRoot(client, node)).rejects.toThrow(/no host path/); + }); + + test('docker nodes use a fixed host root without a storage query', async () => { + const res = await resolveVolumesRoot({}, { name: 'd1', nodeType: 'docker' }); + expect(res.root).toBe('/var/lib/opensource-server/volumes'); + expect(res.shared).toBe(false); + }); + + test('reports shared=false for node-local storage', async () => { + const client = { + async storageConfig(storage) { + return { storage, type: 'dir', path: '/var/lib/vz', shared: 0 }; + }, + }; + const res = await resolveVolumesRoot(client, { ...node, volumeStorage: 'local' }); + expect(res.shared).toBe(false); + }); +}); + +describe('Volume DB validation', () => { + beforeEach(async () => { + await resetDb(); + }); + afterAll(async () => { + await closeDb(); + }); + + test('rejects a traversal name at the model layer', async () => { + await expect( + Volume.create({ containerId: 1, name: '../evil', mountPath: '/mnt/x', mode: 'rw' }), + ).rejects.toThrow(/safe path segment/); + }); + + test('rejects a relative mount path', async () => { + await expect( + Volume.create({ containerId: 1, name: 'ok', mountPath: 'relative', mode: 'rw' }), + ).rejects.toThrow(/absolute path/); + }); +}); diff --git a/create-a-container/utils/docker-api.js b/create-a-container/utils/docker-api.js index 1fb50ce5..fb47fd4d 100644 --- a/create-a-container/utils/docker-api.js +++ b/create-a-container/utils/docker-api.js @@ -95,6 +95,33 @@ function isSystemContainer(options = {}) { return entrypoint.includes('/sbin/init') || entrypoint.includes('systemd'); } +/** + * Parse Proxmox `mpN` bind-mount config values from an LXC config object into + * Docker `HostConfig.Binds` strings (`:[:ro]`). This maps + * the volumes feature (issue #421) onto the Docker backend. Each mpN value is + * `,mp=,ro=<0|1>`. + * @param {object} config - Config object possibly containing mp0, mp1, ... + * @returns {string[]} Docker bind specs + */ +function mpConfigToDockerBinds(config = {}) { + const binds = []; + for (const [key, raw] of Object.entries(config)) { + if (!/^mp\d+$/.test(key) || typeof raw !== 'string') continue; + const parts = raw.split(','); + const hostPath = parts[0]; + let mountPath = null; + let ro = false; + for (const p of parts.slice(1)) { + if (p.startsWith('mp=')) mountPath = p.slice(3); + else if (p === 'ro=1') ro = true; + } + if (hostPath && mountPath) { + binds.push(`${hostPath}:${mountPath}${ro ? ':ro' : ''}`); + } + } + return binds; +} + class DockerApi { constructor(node = {}) { if (!node.apiUrl) { @@ -379,6 +406,13 @@ class DockerApi { if (deleteList.includes('entrypoint')) entrypoint = undefined; if (config.entrypoint) entrypoint = config.entrypoint.split(' '); + // Map any Proxmox mpN bind mounts (issue #421) to Docker binds. When mpN + // keys are present they fully replace the container's binds; otherwise the + // previous binds are preserved. + const mpBinds = mpConfigToDockerBinds(config); + const hasMpConfig = Object.keys(config).some((k) => /^mp\d+$/.test(k)); + const binds = hasMpConfig ? mpBinds : inspect.HostConfig?.Binds || undefined; + if (wasRunning) { await this.request('post', `/containers/${containerId}/stop`).catch(() => {}); } @@ -399,14 +433,16 @@ class DockerApi { HostConfig: { ...(inspect.HostConfig || {}), NetworkMode: inspect.HostConfig?.NetworkMode || 'bridge', + ...(binds ? { Binds: binds } : {}), }, }; - delete body.HostConfig.Binds; delete body.HostConfig.Mounts; delete body.HostConfig.PortBindings; delete body.HostConfig.CpuPeriod; delete body.HostConfig.CpuQuota; + // Only clear Binds when we are not explicitly setting them from mpN config. + if (!binds) delete body.HostConfig.Binds; const created = await this.request('post', '/containers/create', { params: { name }, @@ -417,9 +453,11 @@ class DockerApi { } async updateLxcConfig(node, vmid, config = {}) { + const hasMpConfig = Object.keys(config).some((k) => /^mp\d+$/.test(k)); const hasContainerConfigChanges = config.env !== undefined || config.entrypoint !== undefined || + hasMpConfig || String(config.delete || '').includes('env') || String(config.delete || '').includes('entrypoint'); @@ -536,3 +574,4 @@ class DockerApi { module.exports = DockerApi; module.exports.isValidDockerHost = isValidDockerHost; +module.exports.mpConfigToDockerBinds = mpConfigToDockerBinds; diff --git a/create-a-container/utils/dummy-api.js b/create-a-container/utils/dummy-api.js index 02d2ac0a..badd84f2 100644 --- a/create-a-container/utils/dummy-api.js +++ b/create-a-container/utils/dummy-api.js @@ -154,6 +154,21 @@ class DummyApi { return []; } + /** + * Simulated storage config with a path-backed layout so resolveVolumesRoot() + * derives a plausible volumes root. `shared: 1` so the node-save + * shared-storage warning is quiet for dummy dev nodes. + */ + async storageConfig(storage) { + return { + storage, + type: 'dir', + path: `/mnt/pve/${storage}`, + shared: 1, + content: 'rootdir,vztmpl', + }; + } + async pullOciImage(node, storage, options = {}) { console.log(`[DummyApi] pullOciImage(${options.reference || '?'}) -> simulated`); return this._fakeUpid('imgpull'); diff --git a/create-a-container/utils/proxmox-api.js b/create-a-container/utils/proxmox-api.js index 3e0b5189..8225f5af 100644 --- a/create-a-container/utils/proxmox-api.js +++ b/create-a-container/utils/proxmox-api.js @@ -236,6 +236,22 @@ class ProxmoxApi { return response.data.data; } + /** + * Read a storage's cluster-level configuration (type, path, shared flag, ...) + * via GET /storage/{storage}. `path` is present for path-backed storages + * (dir/nfs/cephfs/glusterfs) and is the authoritative host path — used to + * derive volume host paths instead of assuming the /mnt/pve/ layout. + * @param {string} storage + * @returns {Promise} Storage config (e.g. { storage, type, path, shared, content }) + */ + async storageConfig(storage) { + const response = await axios.get( + `${this.baseUrl}/api2/json/storage/${storage}`, + this.options, + ); + return response.data.data; + } + /** * @returns {Promise} - The next available VMID */ diff --git a/create-a-container/utils/volumes.js b/create-a-container/utils/volumes.js new file mode 100644 index 00000000..8622de00 --- /dev/null +++ b/create-a-container/utils/volumes.js @@ -0,0 +1,167 @@ +/** + * Volume helpers shared by the container-create job and the API router. + * + * This is the single place that derives a volume's host path from a node + * storage's ACTUAL configured path (never assuming the /mnt/pve/ + * layout the retired quick_and_dirty stopgap hardcoded). + * + * See https://github.com/mieweb/opensource-server/issues/421 + */ + +// Reserved name/mount of the legacy shared mount. Kept only so the API can +// reject a user volume that would collide with a pre-#421 backfilled row. +const { QUICK_AND_DIRTY_NAME, QUICK_AND_DIRTY_MOUNT } = require('../models/volume'); + +/** + * Resolve the on-disk root directory for user volumes on a node, derived from + * the node's configured `volumeStorage` (falling back to `imageStorage`). The + * path is taken from Proxmox's storage config (`GET /storage/{storage}` → + * `path`), never assumed. Volumes live under `/volumes`. + * + * @param {object} client - NodeApi client (ProxmoxApi/DummyApi/DockerApi) + * @param {object} node - Node model instance (for storage names + type) + * @returns {Promise<{root: string, storage: string, shared: boolean}>} + * @throws {Error} When the storage has no resolvable host path + */ +async function resolveVolumesRoot(client, node) { + // Docker nodes have no Proxmox storage config; volumes root is a plain host + // dir keyed by node so the Docker bind path is stable. + if (node.nodeType === 'docker') { + return { root: '/var/lib/opensource-server/volumes', storage: 'docker', shared: false }; + } + + const storageName = node.volumeStorage || node.imageStorage || 'local'; + if (typeof client.storageConfig !== 'function') { + // Dummy nodes and any client without storage-config support: fall back to a + // conventional path so the simulated create path still runs end to end. + return { root: `/mnt/pve/${storageName}/volumes`, storage: storageName, shared: false }; + } + + let cfg; + try { + cfg = await client.storageConfig(storageName); + } catch (err) { + throw new Error( + `Could not read storage config for '${storageName}' on node ${node.name}: ${err.message}`, + ); + } + + // Path-backed storages (dir/nfs/cephfs/glusterfs) expose `path`. Block + // storages (lvm/lvmthin/zfspool/rbd) do not and cannot host a bind directory. + if (!cfg || !cfg.path) { + throw new Error( + `Storage '${storageName}' on node ${node.name} has no host path (type=${cfg?.type || 'unknown'}); ` + + 'volumes require a path-backed storage (dir/nfs/cephfs)', + ); + } + + return { + root: `${cfg.path.replace(/\/+$/, '')}/volumes`, + storage: storageName, + shared: cfg.shared === 1 || cfg.shared === '1' || cfg.shared === true, + }; +} + +// Node types with no site agent provisioning their volume directories: Docker +// auto-creates bind sources, and the dummy backend is simulated. Their volumes +// are marked ready locally and must NOT be advertised to the site agent (which +// would otherwise try to create them and could flip them to `failed`). Single +// source of truth for both deriveVolumeHostPaths and the agent snapshot. +const AGENTLESS_NODE_TYPES = new Set(['docker', 'dummy']); + +/** + * Whether volumes on this node type are provisioned without the site agent. + * @param {string} nodeType + * @returns {boolean} + */ +function isAgentlessNodeType(nodeType) { + return AGENTLESS_NODE_TYPES.has(nodeType); +} + +// A path segment derived from a username must be a single safe segment: no +// separators, no traversal, no leading dot/dash. Not sanitized — rewriting could +// map two different usernames to one directory. +const SAFE_SEGMENT = /^[A-Za-z0-9][A-Za-z0-9._-]{0,254}$/; + +/** + * Build the host path for a named, container-scoped volume under a volumes + * root: `/site-///`. + * + * - SITE: container hostnames are only unique per `(siteId, hostname)`, so two + * sites sharing the same storage would otherwise derive the same directory. + * - OWNER: volume directories are retained when a container is deleted, so a + * later container with the same hostname reattaches the data. Keying the + * directory by owner means only the SAME owner reattaches; a different user + * who reuses a deleted hostname gets a fresh directory and never sees the + * previous owner's retained data. (The path is derived once and stored in + * Volume.hostPath, so an admin reassigning a live container keeps its data, + * and that data never follows the hostname to a different future owner.) + * + * Volume names are validated upstream (Volume.isValidName). + * @param {string} volumesRoot + * @param {number|string} siteId - Owning site id (cross-site isolation) + * @param {string} owner - Container owner's username (cross-owner isolation) + * @param {string} hostname - Container hostname (per-owner scoping) + * @param {string} name - Volume name + * @returns {string} + * @throws {Error} When the owner is not a safe single path segment + */ +function containerVolumeHostPath(volumesRoot, siteId, owner, hostname, name) { + if (typeof owner !== 'string' || !SAFE_SEGMENT.test(owner)) { + throw new Error( + `Container owner '${owner}' is not a safe path segment; cannot derive a volume host path`, + ); + } + return `${volumesRoot}/site-${siteId}/${owner}/${hostname}/${name}`; +} + +/** + * Derive and persist any missing host paths on a set of Volume rows, and mark + * volumes that don't need the site agent as `ready`. Idempotent and shared by + * the create/reconfigure/reconcile jobs so the derivation lives in one place. + * + * Built-in `quick_and_dirty` rows are a backfill artifact for pre-#421 + * containers (their live mount already exists on Proxmox). They are left + * untouched: no host path is derived and they never render a new mpN — see + * Volume.buildMountConfig, which skips them. Every other volume gets a derived + * host path. Volumes on backends without a site agent that provisions + * directories are marked `ready` immediately so `prepareVolumes` does not block + * for the whole timeout: `docker` (Docker auto-creates bind sources) and + * `dummy` (the simulated dev/test backend has no agent). + * + * @param {Array} volumes - Volume model instances + * @param {object} opts + * @param {string} opts.volumesRoot - Resolved volumes root for the node + * @param {number|string} opts.siteId - Owning site id (path isolation) + * @param {string} opts.owner - Container owner's username (path isolation) + * @param {string} opts.hostname - Container hostname (per-owner scoping) + * @param {string} opts.nodeType - Node type ('proxmox' | 'docker' | 'dummy') + * @returns {Promise} + */ +async function deriveVolumeHostPaths(volumes, { volumesRoot, siteId, owner, hostname, nodeType }) { + // Backends with no directory-provisioning site agent: mark ready directly so + // the create barrier does not poll until timeout. + const agentlessBackend = isAgentlessNodeType(nodeType); + for (const v of volumes) { + // Legacy built-in rows are informational only; don't derive or re-mount. + if (v.builtin) continue; + const updates = {}; + if (!v.hostPath) { + updates.hostPath = containerVolumeHostPath(volumesRoot, siteId, owner, hostname, v.name); + } + if (agentlessBackend && v.status !== 'ready') { + updates.status = 'ready'; + updates.appliedAt = new Date(); + } + if (Object.keys(updates).length > 0) await v.update(updates); + } +} + +module.exports = { + resolveVolumesRoot, + containerVolumeHostPath, + deriveVolumeHostPaths, + isAgentlessNodeType, + QUICK_AND_DIRTY_NAME, + QUICK_AND_DIRTY_MOUNT, +}; From d2011e81eee5b91bb8d4b15270a5d700b9d12999 Mon Sep 17 00:00:00 2001 From: Robert Gingras Date: Tue, 29 Sep 2026 11:37:42 -0400 Subject: [PATCH 2/5] volumes: provision and attach volumes in the container jobs (#421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retire the hardcoded quick_and_dirty mp0 (buildSharedVolumeMp0) and have the container jobs attach the container's Volume rows instead. Nothing is mounted unless the container has volumes. Proxmox validates, but does not create, a bind mount's host directory, and has no mkdir/exec API, so directories are created out of band by the site agent (next commit). The manager side: - create-container.js: prepareVolumes derives each volume's host path and BLOCKS until every Volume.status is `ready` (bounded, VOLUME_READY_TIMEOUT_MS, default 5 min), failing on `failed` with the agent's message. This runs BEFORE the CT is created, so a provisioning failure never leaves an orphaned half-created container. mpN is applied once the CT exists. - reconfigure-container.js: the same readiness wait, then re-renders mpN on restart; a mount change forces a restart. Skipped on Docker nodes, whose binds are applied at recreate and whose lxcConfig exposes no mpN. - Agent snapshot (agent-config.js): advertises the site's derived, non-builtin volumes (id, hostPath, mode, owner uid/gid) at the site level — there is one agent per site — independently of container IPs, so volumes are advertised during creation. Agentless (docker/dummy) volumes are excluded. - Agent check-in (POST /agents): accepts a per-volume results map and writes Volume.status/statusMessage/appliedAt, only for volumes whose container is in the checking-in site. Built-in rows are never changed by an agent. - Existing containers: the backfill migration records their still-live quick_and_dirty mount as a builtin row (informational, never re-applied) and enqueues bin/reconcile-volumes.js, an idempotent CLI that brings live mpN in line with Volume rows (no-op for pre-existing containers). down() removes both. Its raw SELECT quotes the table name (Postgres folds unquoted names). --- create-a-container/bin/create-container.js | 179 ++++++++++++++---- create-a-container/bin/reconcile-volumes.js | 138 ++++++++++++++ .../bin/reconfigure-container.js | 90 ++++++++- ...130000-backfill-quick-and-dirty-volumes.js | 76 ++++++++ .../backfill-quick-and-dirty-volumes.test.js | 47 +++++ .../api/v1/__tests__/agents.volumes.test.js | 116 ++++++++++++ create-a-container/routers/api/v1/agents.js | 45 ++++- .../__tests__/agent-config-volumes.test.js | 128 +++++++++++++ create-a-container/utils/agent-config.js | 79 +++++++- 9 files changed, 859 insertions(+), 39 deletions(-) create mode 100644 create-a-container/bin/reconcile-volumes.js create mode 100644 create-a-container/migrations/20260722130000-backfill-quick-and-dirty-volumes.js create mode 100644 create-a-container/migrations/__tests__/backfill-quick-and-dirty-volumes.test.js create mode 100644 create-a-container/routers/api/v1/__tests__/agents.volumes.test.js create mode 100644 create-a-container/utils/__tests__/agent-config-volumes.test.js diff --git a/create-a-container/bin/create-container.js b/create-a-container/bin/create-container.js index 37740deb..335b73a9 100755 --- a/create-a-container/bin/create-container.js +++ b/create-a-container/bin/create-container.js @@ -28,13 +28,17 @@ const path = require('path'); // Load models from parent directory const db = require(path.join(__dirname, '..', 'models')); -const { Container, Node, Site, Service, HTTPService, ExternalDomain, Setting, ResourceRequest } = db; +const { Container, Node, Site, Service, HTTPService, ExternalDomain, Setting, ResourceRequest, Volume } = db; // Load utilities const { parseArgs } = require(path.join(__dirname, '..', 'utils', 'cli')); const { isDockerImage, parseDockerRef, getImageDigest } = require(path.join(__dirname, '..', 'utils', 'docker-registry')); const { manageDnsRecords } = require(path.join(__dirname, '..', 'utils', 'cloudflare-dns')); const { createVirtualMachine, withNetbox } = require(path.join(__dirname, '..', 'utils', 'netbox')); +const { + resolveVolumesRoot, + deriveVolumeHostPaths, +} = require(path.join(__dirname, '..', 'utils', 'volumes')); /** * Generate a filename for a pulled Docker image @@ -92,25 +96,111 @@ async function resolveStorage(client, nodeName, preferred, contentType) { } /** - * Build the mp0 mount point value for the shared read-only volume. - * Assumes the node's template storage is a directory storage mounted at - * /mnt/pve/ (i.e. container templates are downloaded to - * /mnt/pve//template/cache), so shared volumes live at - * /mnt/pve//volumes/. + * Prepare this container's volumes and BLOCK until every host directory has + * been provisioned by the site agent — run this BEFORE the container is created + * on the provider, so a barrier failure never leaves an orphaned, half-created + * CT (issue #421). No provider handle is needed: the agent creates directories + * from the Volume rows (persisted before the CT exists), and this only reads the + * derived host paths and the `Volume.status` column. * - * TODO(#421): Replace this hardcoded 'quick_and_dirty' volume with a proper - * feature where users can define their own volumes and attach them with - * per-volume permissions (read-only/read-write). The host path should be - * derived from the storage's actual configured path instead of assuming the - * /mnt/pve/ layout. See: - * https://github.com/mieweb/opensource-server/issues/421 + * The hardcoded shared `quick_and_dirty` mount is fully retired: new containers + * get ONLY the volumes their creator requested — no volume is auto-attached. A + * container with no volumes gets no bind mounts. (The `quick_and_dirty` model + * row still exists for pre-#421 containers via the backfill migration, purely to + * reflect their already-live mount; it is never seeded onto new containers.) * - * @param {string} templateStorage - Resolved template storage name for the node - * @returns {string} Proxmox mp0 config value (bind mount, read-only) + * The flow: + * 1. Resolve the volumes root from the node storage's ACTUAL configured path. + * 2. Derive + persist any missing host paths; mark docker volumes `ready` + * immediately (they don't need the agent). User volumes stay `pending` so + * the next check-in snapshot advertises them to the agent. + * 3. Block on `Volume.status = 'ready'` for every volume (bounded timeout). + * + * @param {object} client - NodeApi client + * @param {object} node - Node model instance + * @param {object} container - Container model instance + * @returns {Promise} The container's ready volumes, ordered by id + */ +async function prepareVolumes(client, node, container) { + const volumes = await Volume.findAll({ where: { containerId: container.id } }); + // No volumes requested → nothing to provision or mount. + if (volumes.length === 0) { + console.log('No volumes requested for this container.'); + return []; + } + + const { root: volumesRoot, shared } = await resolveVolumesRoot(client, node); + console.log(`Volumes root: ${volumesRoot} (shared=${shared})`); + if (!shared && node.nodeType !== 'docker') { + console.warn( + '⚠️ WARNING: volumes storage is not shared across nodes; persistent volume ' + + 'data will not follow this container if it is migrated to another node. ' + + 'See https://github.com/mieweb/opensource-server/issues/421', + ); + } + + // Derive + persist missing host paths; mark builtin/docker volumes ready. + await deriveVolumeHostPaths(volumes, { + volumesRoot, + siteId: container.siteId, + owner: container.username, + hostname: container.hostname, + nodeType: node.nodeType, + }); + + // Sync barrier: the agent creates directories asynchronously (~30 s timer), + // and Proxmox 400s if mpN points at a missing dir. Block on Volume.status. + await waitForVolumesReady(container.id); + + return Volume.findAll({ where: { containerId: container.id }, order: [['id', 'ASC']] }); +} + +/** + * Poll `Volume.status` until every volume for a container is `ready`, or throw + * when any is `failed` (surfacing its statusMessage) or the timeout elapses. + * The agent transitions these via the extended check-in; this only watches the + * DB column. + * + * The default timeout accommodates the agent's ~30 s timer plus the two + * check-ins needed per volume (receive snapshot, then report results). Override + * via VOLUME_READY_TIMEOUT_MS for slow or contended clusters. + * + * @param {number} containerDbId + * @param {number} [timeoutMs] Bounded wait (default 5 minutes; env override) + * @param {number} [pollMs] Poll interval (default 5 s) */ -function buildSharedVolumeMp0(templateStorage) { - const volumeName = 'quick_and_dirty'; - return `/mnt/pve/${templateStorage}/volumes/${volumeName},mp=/mnt/${volumeName},ro=1`; +async function waitForVolumesReady( + containerDbId, + timeoutMs = parseInt(process.env.VOLUME_READY_TIMEOUT_MS, 10) || 300000, + pollMs = 5000, +) { + const start = Date.now(); + for (;;) { + const volumes = await Volume.findAll({ where: { containerId: containerDbId } }); + const failed = volumes.find((v) => v.status === 'failed'); + if (failed) { + throw new Error( + `Volume '${failed.name}' provisioning failed on the node: ${failed.statusMessage || 'unknown error'}`, + ); + } + const pending = volumes.filter((v) => v.status !== 'ready'); + if (pending.length === 0) { + console.log('All volume directories are ready'); + return; + } + if (Date.now() - start > timeoutMs) { + const names = pending.map((v) => `${v.name} (${v.status})`).join(', '); + throw new Error( + `Timed out waiting for volume provisioning on the node after ${Math.round(timeoutMs / 1000)}s: ${names}`, + ); + } + console.log( + `Waiting for volume provisioning on node (${pending.length} pending: ${pending + .map((v) => v.name) + .join(', ')})...`, + ); + await new Promise((resolve) => setTimeout(resolve, pollMs)); + } } /** @@ -258,7 +348,15 @@ async function main() { // Get the Proxmox API client const client = await node.api(); console.log('Node API client initialized'); - + + // Prepare volumes and BLOCK on the site agent creating their host + // directories BEFORE creating the container. Doing this first means a + // barrier failure (agent error or timeout) aborts the job without ever + // provisioning a CT — no orphaned, half-created container to clean up + // (issue #421). The bind mounts (mpN) are applied after the CT exists. + console.log('Preparing volumes...'); + const preparedVolumes = await prepareVolumes(client, node, container); + // Allocate the provider ID right before creating to minimize race condition window. // Proxmox requires us to allocate a VMID first; Docker returns its real container // ID after create. @@ -329,8 +427,10 @@ async function main() { tags: container.username, unprivileged: 1, rootfs: `${rootfsStorage}:${rootfsSize}`, - // TODO(#421): hardcoded shared volume; see buildSharedVolumeMp0() - mp0: buildSharedVolumeMp0(templateStorage) + // Volumes (bind mounts) are applied after creation; their host + // directories were provisioned by the agent before this point (see + // prepareVolumes above). Setting mpN here would 400 against a + // not-yet-created directory (issue #421). }); console.log(`Create task started: ${createUpid}`); @@ -363,18 +463,7 @@ async function main() { const rootfsStorage = await resolveStorage(client, node.name, node.volumeStorage || 'local-lvm', 'rootdir'); console.log(`Using rootfs storage: ${rootfsStorage}`); - - // Resolve the template storage to locate the shared volume mount (mp0). - // Non-fatal: cloning does not otherwise require a template storage. - // TODO(#421): hardcoded shared volume; see buildSharedVolumeMp0() - let mp0 = null; - try { - const templateStorage = await resolveStorage(client, node.name, node.imageStorage || 'local', 'vztmpl'); - mp0 = buildSharedVolumeMp0(templateStorage); - } catch (err) { - console.warn(`Skipping shared volume mount (mp0): ${err.message}`); - } - + // Clone the template console.log(`Cloning template ${templateVmid} to VMID ${vmid}...`); const cloneUpid = await client.cloneLxc(node.name, templateVmid, vmid, { @@ -389,7 +478,9 @@ async function main() { await client.waitForTask(node.name, cloneUpid); console.log('Clone completed successfully'); - // Configure the container (Docker containers are configured at creation time) + // Configure the container (Docker containers are configured at creation time). + // Volumes (bind mounts) are applied later, after the agent creates their + // host directories (issue #421). console.log('Configuring container...'); await client.updateLxcConfig(node.name, vmid, { cores, @@ -400,7 +491,6 @@ async function main() { swap, onboot: 1, tags: container.username, - ...(mp0 ? { mp0 } : {}) }); console.log('Container configured'); } @@ -464,6 +554,27 @@ async function main() { // Store the provider container ID now that creation succeeded. await container.update({ containerId: String(vmid) }); console.log(`Container provider ID ${vmid} stored in database`); + + // Attach the volume bind mounts. Their host directories were already + // provisioned by the site agent before the container was created (see + // prepareVolumes above), so setting mpN cannot 400 on a missing directory. + if (preparedVolumes.length > 0) { + const mountConfig = Volume.buildMountConfig(preparedVolumes); + console.log(`Attaching ${preparedVolumes.length} volume(s): ${JSON.stringify(mountConfig)}`); + const mountTask = await client.updateLxcConfig(node.name, vmid, mountConfig); + const mountedDockerId = isDockerNode ? parseDockerTaskId(mountTask) : null; + if (mountedDockerId) { + vmid = mountedDockerId; + await container.update({ containerId: String(vmid) }); + console.log(`Docker container ID after volume attach: ${vmid}`); + } + // Mark all volumes applied now that the mounts are set. + await Volume.update( + { appliedAt: new Date() }, + { where: { containerId: container.id } }, + ); + console.log('Volumes attached'); + } // Start the container console.log('Starting container...'); diff --git a/create-a-container/bin/reconcile-volumes.js b/create-a-container/bin/reconcile-volumes.js new file mode 100644 index 00000000..d566c19e --- /dev/null +++ b/create-a-container/bin/reconcile-volumes.js @@ -0,0 +1,138 @@ +#!/usr/bin/env node +/** + * reconcile-volumes.js + * + * One-time (and re-runnable) reconciliation that brings every provisioned + * container's live Proxmox mpN config in line with its Volume rows (issue #421 + * (b)). Enqueued by the backfill migration. + * + * For each container that has a provider id (containerId) and at least one + * non-builtin volume, this: + * 1. derives any missing host paths from the node storage's configured path, + * 2. skips containers whose volumes aren't all `ready` (the create/reconfigure + * job owns the agent sync barrier; this reconciler never waits or mutates + * directories), + * 3. sets mpN via updateLxcConfig only when the rendered config differs from + * what's already live — so pre-existing containers (which already carry + * their mp0) are a no-op. + * + * Best-effort and non-fatal per container: a single node/API failure is logged + * and skipped rather than failing the whole job. Exit 0 unless something + * unexpected throws. + * + * Usage: node bin/reconcile-volumes.js + */ + +const path = require('path'); + +const db = require(path.join(__dirname, '..', 'models')); +const { Container, Node, Site, Volume } = db; +const { + resolveVolumesRoot, + deriveVolumeHostPaths, +} = require(path.join(__dirname, '..', 'utils', 'volumes')); + +/** + * Reconcile one container's live mpN with its Volume rows. + * @returns {Promise<'applied'|'skipped'>} whether a mount change was applied + */ +async function reconcileContainer(container) { + const node = container.node; + if (!node) { + console.log(`Container ${container.hostname}: no node, skipping`); + return 'skipped'; + } + if (!container.containerId) { + console.log(`Container ${container.hostname}: not provisioned yet, skipping`); + return 'skipped'; + } + if (!node.hasApiAccess()) { + console.log(`Container ${container.hostname}: node ${node.name} has no API access, skipping`); + return 'skipped'; + } + + const volumes = await Volume.findAll({ where: { containerId: container.id }, order: [['id', 'ASC']] }); + if (volumes.length === 0) return 'skipped'; + + const client = await node.api(); + + // Resolve the volumes root only when a non-builtin volume still needs its + // host path derived — resolveVolumesRoot fails on block-backed storage (e.g. + // the default local-lvm), and a container whose only volume is the legacy + // built-in (or whose paths are already derived) must not fail over storage it + // never needs. + const needsRoot = volumes.some((v) => !v.builtin && !v.hostPath); + if (needsRoot) { + const { root: volumesRoot } = await resolveVolumesRoot(client, node); + // Backfill any missing host paths so mpN can be rendered (shared helper). + await deriveVolumeHostPaths(volumes, { + volumesRoot, + siteId: container.siteId, + owner: container.username, + hostname: container.hostname, + nodeType: node.nodeType, + }); + } + + // Only reconcile once every volume directory is ready — directory creation is + // the create/reconfigure job's responsibility (agent sync barrier), not this + // reconciler's. + const fresh = await Volume.findAll({ where: { containerId: container.id }, order: [['id', 'ASC']] }); + const notReady = fresh.filter((v) => v.status !== 'ready'); + if (notReady.length > 0) { + console.log( + `Container ${container.hostname}: ${notReady.length} volume(s) not ready, skipping mount reconcile`, + ); + return 'skipped'; + } + + // Docker binds are applied at create time and lxcConfig() does not expose + // mpN, so a diff would always report "changed"; skip the mpN reconcile there. + if (node.nodeType === 'docker') { + console.log(`Container ${container.hostname}: docker node, mpN reconcile not applicable`); + return 'skipped'; + } + + const mountConfig = Volume.buildMountConfig(fresh); + const live = await client.lxcConfig(node.name, container.containerId); + const differs = Object.entries(mountConfig).some(([k, val]) => live[k] !== val); + if (!differs) { + console.log(`Container ${container.hostname}: mounts already in sync`); + return 'skipped'; + } + + console.log(`Container ${container.hostname}: applying ${Object.keys(mountConfig).length} mount(s)`); + await client.updateLxcConfig(node.name, container.containerId, mountConfig); + await Volume.update({ appliedAt: new Date() }, { where: { containerId: container.id } }); + return 'applied'; +} + +async function main() { + console.log('Reconciling container volumes...'); + const containers = await Container.findAll({ + include: [{ model: Node, as: 'node', include: [{ model: Site, as: 'site' }] }], + }); + + let applied = 0; + let skipped = 0; + let failed = 0; + for (const container of containers) { + try { + const result = await reconcileContainer(container); + if (result === 'applied') applied += 1; + else skipped += 1; + } catch (err) { + failed += 1; + console.warn(`Container ${container.hostname}: reconcile failed (non-fatal): ${err.message}`); + } + } + console.log( + `Volume reconciliation complete: ${applied} applied, ${skipped} skipped (no-op), ${failed} failed`, + ); + process.exit(0); +} + +main().catch((err) => { + console.error('Unhandled error:', err); + process.exit(1); +}); diff --git a/create-a-container/bin/reconfigure-container.js b/create-a-container/bin/reconfigure-container.js index 1f702ce9..b02bf820 100644 --- a/create-a-container/bin/reconfigure-container.js +++ b/create-a-container/bin/reconfigure-container.js @@ -23,10 +23,69 @@ const path = require('path'); // Load models from parent directory const db = require(path.join(__dirname, '..', 'models')); -const { Container, Node, Site } = db; +const { Container, Node, Site, Volume } = db; // Load utilities const { parseArgs } = require(path.join(__dirname, '..', 'utils', 'cli')); +const { + resolveVolumesRoot, + deriveVolumeHostPaths, +} = require(path.join(__dirname, '..', 'utils', 'volumes')); + +/** + * Ensure this container's volume host paths are derived and their directories + * are ready, then return the ready volumes so the caller can render mpN. Mirrors + * the create job's prepareVolumes but for reconfigure (attaching a volume to + * an already-provisioned container). Built-in and docker volumes never block. + * @param {object} client + * @param {object} node + * @param {object} container + * @returns {Promise} + */ +async function ensureVolumesReady(client, node, container) { + const volumes = await Volume.findAll({ where: { containerId: container.id } }); + if (volumes.length === 0) return []; + + // Resolve the volumes root only when a non-builtin volume still needs its + // host path derived. Otherwise (only legacy built-in rows, or everything + // already derived) skip it: resolveVolumesRoot fails on block-backed storage + // like the default local-lvm, and a pre-#421 container whose only volume is + // the built-in must not fail reconfigure over storage it never needs. + const needsRoot = volumes.some((v) => !v.builtin && !v.hostPath); + if (needsRoot) { + const { root: volumesRoot } = await resolveVolumesRoot(client, node); + // Derive + persist missing host paths; mark builtin/docker volumes ready + // (shared with the create/reconcile jobs). + await deriveVolumeHostPaths(volumes, { + volumesRoot, + siteId: container.siteId, + owner: container.username, + hostname: container.hostname, + nodeType: node.nodeType, + }); + } + + // Bounded wait for the agent to create any pending directories. + const start = Date.now(); + const timeoutMs = 300000; + for (;;) { + const rows = await Volume.findAll({ where: { containerId: container.id } }); + const failed = rows.find((v) => v.status === 'failed'); + if (failed) { + throw new Error( + `Volume '${failed.name}' provisioning failed: ${failed.statusMessage || 'unknown error'}`, + ); + } + if (rows.every((v) => v.status === 'ready')) break; + if (Date.now() - start > timeoutMs) { + throw new Error('Timed out waiting for volume provisioning on the node'); + } + console.log('Waiting for volume provisioning on the node...'); + await new Promise((resolve) => setTimeout(resolve, 5000)); + } + + return Volume.findAll({ where: { containerId: container.id }, order: [['id', 'ASC']] }); +} /** * Main function @@ -109,11 +168,38 @@ async function main() { console.log('Resource configuration applied'); } + // Reconcile volume bind mounts (issue #421): ensure host directories are + // ready and (re)render every Volume row to mpN. Setting the same mpN values + // is idempotent; attaching a new one requires a restart to take effect. + let volumesChanged = false; + const volumes = await ensureVolumesReady(client, node, container); + // Skip the mpN mount reconcile on Docker nodes: Docker binds are applied at + // create time and `lxcConfig()` does not expose `mpN`, so the diff below + // would always report "changed" and re-apply on every reconfigure. (Bind + // changes on Docker flow through the container-recreate path, not mpN.) + if (volumes.length > 0 && node.nodeType !== 'docker') { + const mountConfig = Volume.buildMountConfig(volumes); + const currentConfig = await client.lxcConfig(node.name, container.containerId); + volumesChanged = Object.entries(mountConfig).some(([k, val]) => currentConfig[k] !== val); + if (volumesChanged) { + console.log('Applying volume mounts...'); + console.log('Volumes:', JSON.stringify(mountConfig, null, 2)); + await client.updateLxcConfig(node.name, container.containerId, mountConfig); + await Volume.update( + { appliedAt: new Date() }, + { where: { containerId: container.id } }, + ); + console.log('Volume mounts applied'); + } else { + console.log('Volume mounts already up to date'); + } + } + // Determine if a stop/start cycle is required. // rootfs (disk) changes require a restart; memory/cpu/swap are applied live via cgroups. // LXC env/entrypoint config changes (actual values being set, not just deletions) require a restart. const hasEnvConfigChanges = Object.keys(lxcConfig).some(k => k !== 'delete'); - const requiresRestart = !!args.rootfs || hasEnvConfigChanges; + const requiresRestart = !!args.rootfs || hasEnvConfigChanges || volumesChanged; // Check container status before stop/start cycle const lxcStatus = await client.getLxcStatus(node.name, container.containerId); diff --git a/create-a-container/migrations/20260722130000-backfill-quick-and-dirty-volumes.js b/create-a-container/migrations/20260722130000-backfill-quick-and-dirty-volumes.js new file mode 100644 index 00000000..78c74a3f --- /dev/null +++ b/create-a-container/migrations/20260722130000-backfill-quick-and-dirty-volumes.js @@ -0,0 +1,76 @@ +'use strict'; + +/** + * Backfill a built-in `quick_and_dirty` Volume row for every existing container, + * so the DB reflects the shared read-only mount that create-container.js used to + * attach via the hardcoded mp0 (issue #421 (b)). These rows are marked + * `builtin` + `ready` and are NOT re-attached to running containers — existing + * containers already carry their mp0, so this is purely the DB catching up to + * reality. hostPath is left null; it is informational and the live mount is + * untouched. + * + * A one-time reconciliation job is enqueued (not raw SQL) because bringing a + * live LXC's mpN config in line with its Volume rows is a Proxmox + * updateLxcConfig call, not a DB write. For pre-existing containers the job is a + * no-op; it is the mechanism by which any newly-attached volume reaches an + * already-provisioned container on the next reconcile. + */ + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + const now = new Date(); + // Quote the identifier so Postgres preserves the mixed-case table name + // (unquoted `Containers` case-folds to `containers`, which does not exist). + // Matches the repo convention (e.g. 20251104193601-create-node.js). + const containersTable = queryInterface.quoteIdentifier('Containers'); + const containers = await queryInterface.sequelize.query( + `SELECT id FROM ${containersTable}`, + { type: queryInterface.sequelize.QueryTypes.SELECT }, + ); + + if (containers && containers.length > 0) { + const rows = containers.map((c) => ({ + containerId: c.id, + name: 'quick_and_dirty', + hostPath: null, + mountPath: '/mnt/quick_and_dirty', + mode: 'ro', + scope: 'container', + builtin: true, + status: 'ready', + statusMessage: null, + appliedAt: now, + createdAt: now, + updatedAt: now, + })); + // ignoreDuplicates keeps this idempotent against the (containerId, name) + // unique index if the migration is re-run. + await queryInterface.bulkInsert('Volumes', rows, { ignoreDuplicates: true }); + } + + // Enqueue the one-time reconciliation (no-op for pre-existing containers). + await queryInterface.bulkInsert('Jobs', [ + { + command: 'node bin/reconcile-volumes.js', + createdBy: 'system', + status: 'pending', + createdAt: now, + updatedAt: now, + }, + ]); + }, + + async down(queryInterface, Sequelize) { + await queryInterface.bulkDelete('Volumes', { builtin: true, name: 'quick_and_dirty' }); + // Also remove the one-time reconciliation job this migration enqueued, so a + // rollback doesn't leave an orphaned pending job that runs later (and so an + // up/down cycle doesn't accumulate duplicates). Scope to the still-pending + // system job to avoid touching one that already ran. + await queryInterface.bulkDelete('Jobs', { + command: 'node bin/reconcile-volumes.js', + createdBy: 'system', + status: 'pending', + }); + }, +}; diff --git a/create-a-container/migrations/__tests__/backfill-quick-and-dirty-volumes.test.js b/create-a-container/migrations/__tests__/backfill-quick-and-dirty-volumes.test.js new file mode 100644 index 00000000..69beda64 --- /dev/null +++ b/create-a-container/migrations/__tests__/backfill-quick-and-dirty-volumes.test.js @@ -0,0 +1,47 @@ +/** + * Guard for the Postgres identifier-quoting bug (issue #421 review B1): the + * backfill migration must quote the `Containers` table name in its raw query, + * or Postgres case-folds it to `containers` and crashes the Manager on startup + * (`relation "containers" does not exist`). SQLite is case-insensitive, so the + * sqlite migration round-trip can't catch this — this test asserts the + * migration issues a properly quoted identifier regardless of dialect. + */ + +const path = require('path'); + +const migration = require(path.join( + __dirname, + '..', + '20260722130000-backfill-quick-and-dirty-volumes.js', +)); + +describe('backfill-quick-and-dirty-volumes migration', () => { + test('quotes the Containers identifier in its raw SELECT', async () => { + const queries = []; + // Minimal queryInterface stub capturing the raw SQL and mimicking + // Sequelize's real quoteIdentifier (double-quoted, mixed case preserved). + const queryInterface = { + quoteIdentifier: (id) => `"${id}"`, + sequelize: { + QueryTypes: { SELECT: 'SELECT' }, + query: async (sql) => { + queries.push(sql); + return []; // no existing containers + }, + }, + bulkInsert: async () => {}, + bulkDelete: async () => {}, + }; + + await migration.up(queryInterface, {}); + + const selects = queries.filter((q) => /select/i.test(q)); + expect(selects.length).toBeGreaterThan(0); + for (const sql of selects) { + // Must reference the quoted, mixed-case table name... + expect(sql).toContain('"Containers"'); + // ...and never the bare unquoted identifier that Postgres would fold. + expect(sql).not.toMatch(/FROM\s+Containers\b/); + } + }); +}); diff --git a/create-a-container/routers/api/v1/__tests__/agents.volumes.test.js b/create-a-container/routers/api/v1/__tests__/agents.volumes.test.js new file mode 100644 index 00000000..614dc4ba --- /dev/null +++ b/create-a-container/routers/api/v1/__tests__/agents.volumes.test.js @@ -0,0 +1,116 @@ +/** + * Agent check-in volume results (issue #421): the agent reports per-volume + * directory-provisioning results keyed by Volume id; the manager writes them + * into Volume.status (ready/failed) with statusMessage/appliedAt. Built-in + * volumes are never flipped by an agent report. + */ + +const request = require('supertest'); +const { buildApp } = require('../../../../tests/helpers/app'); +const { resetDb, closeDb } = require('../../../../tests/helpers/db'); +const { Site, Node, Container, Volume } = require('../../../../models'); + +describe('POST /api/v1/agents volume results', () => { + let app; + let site; + let readyVol; + let failVol; + let builtinVol; + + beforeEach(async () => { + await resetDb(); + app = buildApp(); + site = await Site.create({ name: 's1', internalDomain: 'example.test' }); + const node = await Node.create({ siteId: site.id, name: 'n1', nodeType: 'dummy' }); + const container = await Container.create({ + hostname: 'ct1', + username: 'tester', + nodeId: node.id, + siteId: site.id, + }); + readyVol = await Volume.create({ + containerId: container.id, + name: 'data', + hostPath: '/v/ct1/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'pending', + }); + failVol = await Volume.create({ + containerId: container.id, + name: 'logs', + hostPath: '/v/ct1/logs', + mountPath: '/mnt/logs', + mode: 'rw', + status: 'pending', + }); + builtinVol = await Volume.create({ + containerId: container.id, + name: 'quick_and_dirty', + hostPath: '/v/quick_and_dirty', + mountPath: '/mnt/quick_and_dirty', + mode: 'ro', + builtin: true, + status: 'ready', + }); + }); + + afterAll(async () => { + await closeDb(); + }); + + test('applied=true -> ready, applied=false -> failed with message; built-in untouched', async () => { + const res = await request(app) + .post('/api/v1/agents') + .send({ + siteId: site.id, + hostname: 'n1', + volumes: { + [readyVol.id]: { applied: true }, + [failVol.id]: { applied: false, message: 'mkdir: permission denied' }, + [builtinVol.id]: { applied: false, message: 'should be ignored' }, + }, + }); + expect(res.status).toBe(200); + + await readyVol.reload(); + await failVol.reload(); + await builtinVol.reload(); + + expect(readyVol.status).toBe('ready'); + expect(readyVol.appliedAt).toBeTruthy(); + expect(failVol.status).toBe('failed'); + expect(failVol.statusMessage).toBe('mkdir: permission denied'); + // Built-in volume is not flipped by an agent report. + expect(builtinVol.status).toBe('ready'); + }); + + test('a check-in cannot flip a volume belonging to another site', async () => { + // A second site with its own container + pending volume. + const site2 = await Site.create({ name: 's2', internalDomain: 'other.test' }); + const node2 = await Node.create({ siteId: site2.id, name: 'n2', nodeType: 'dummy' }); + const container2 = await Container.create({ + hostname: 'ct2', + username: 'tester', + nodeId: node2.id, + siteId: site2.id, + }); + const otherVol = await Volume.create({ + containerId: container2.id, + name: 'data', + hostPath: '/v/ct2/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'pending', + }); + + // site 1's agent reports a result for site 2's volume id — must be ignored. + const res = await request(app) + .post('/api/v1/agents') + .send({ siteId: site.id, hostname: 'n1', volumes: { [otherVol.id]: { applied: true } } }); + expect(res.status).toBe(200); + + await otherVol.reload(); + expect(otherVol.status).toBe('pending'); + }); +}); diff --git a/create-a-container/routers/api/v1/agents.js b/create-a-container/routers/api/v1/agents.js index d9a014ee..3accd43e 100644 --- a/create-a-container/routers/api/v1/agents.js +++ b/create-a-container/routers/api/v1/agents.js @@ -8,14 +8,52 @@ */ const express = require('express'); -const { Agent, Site } = require('../../../models'); +const { Agent, Site, Container, Volume } = require('../../../models'); const { apiAuth, apiAdmin, localhostOrAdmin, asyncHandler, ok, fail } = require('../../../middlewares/api'); const { buildAgentConfig, computeConfigEtag } = require('../../../utils/agent-config'); const router = express.Router(); +/** + * Apply an agent-reported per-volume results map to the Volume table, scoped to + * the checking-in site. Shape: { : { applied: true|false, message? } }. + * The Manager owns the volume ids (sent in the config snapshot); the agent + * reports the outcome of its mkdir. Transitions each Volume.status to + * ready/failed with statusMessage + appliedAt. + * + * Each volume is loaded together with its owning container and verified to + * belong to `siteId` before any update — a check-in from one site (or a + * misconfigured/rogue agent) must not be able to mark another site's volume + * ready and let a create job attach an unprovisioned path. Unknown ids, + * cross-site ids, and built-in volumes are ignored. + * @param {number} siteId - The checking-in site's id. + * @param {object} volumesResult + * @returns {Promise} + */ +async function applyVolumeResults(siteId, volumesResult) { + if (!volumesResult || typeof volumesResult !== 'object') return; + for (const [rawId, result] of Object.entries(volumesResult)) { + const id = Number(rawId); + if (!Number.isInteger(id) || !result || typeof result !== 'object') continue; + const volume = await Volume.findByPk(id, { + include: [{ model: Container, as: 'container', attributes: ['id', 'siteId'] }], + }); + // Skip unknown ids, built-in (admin-provisioned, not agent-owned) volumes, + // and — critically — any volume whose container is not in this site. + if (!volume || volume.builtin) continue; + if (!volume.container || volume.container.siteId !== siteId) continue; + const applied = result.applied === true || result.applied === 'true'; + const message = typeof result.message === 'string' ? result.message.slice(0, 2000) : null; + if (applied) { + await volume.update({ status: 'ready', statusMessage: null, appliedAt: new Date() }); + } else { + await volume.update({ status: 'failed', statusMessage: message || 'agent reported failure' }); + } + } +} + router.post('/', localhostOrAdmin, asyncHandler(async (req, res) => { - const { siteId, hostname, ipv4Address, services } = req.body || {}; + const { siteId, hostname, ipv4Address, services, volumes } = req.body || {}; const parsedSiteId = typeof siteId === 'number' ? siteId : Number(siteId); if (!Number.isInteger(parsedSiteId) || !hostname || typeof hostname !== 'string') { return fail(res, 422, 'validation_failed', 'siteId and hostname are required'); @@ -33,6 +71,9 @@ router.post('/', localhostOrAdmin, asyncHandler(async (req, res) => { services: services || null, lastCheckinAt: new Date(), }); + // Transition Volume.status from the agent's per-volume directory results, + // scoped to this site so a check-in can't touch another site's volumes. + await applyVolumeResults(parsedSiteId, volumes); } const config = await buildAgentConfig(parsedSiteId); diff --git a/create-a-container/utils/__tests__/agent-config-volumes.test.js b/create-a-container/utils/__tests__/agent-config-volumes.test.js new file mode 100644 index 00000000..38f465d8 --- /dev/null +++ b/create-a-container/utils/__tests__/agent-config-volumes.test.js @@ -0,0 +1,128 @@ +/** + * Volumes in the agent config snapshot (issue #421): buildAgentConfig must + * advertise a container's non-builtin, host-path-resolved volumes at the SITE + * level (one agent per site), and must exclude built-in and not-yet-derived + * volumes. Verified independently of container IP (volumes must be advertised + * during creation, before the container has an IP). + */ + +const { resetDb, closeDb } = require('../../tests/helpers/db'); +const { Site, Node, Container, Volume } = require('../../models'); +const { buildAgentConfig } = require('../agent-config'); + +describe('buildAgentConfig volumes', () => { + let site; + let node; + let container; + + beforeEach(async () => { + await resetDb(); + site = await Site.create({ name: 'vol-site', internalDomain: 'example.test' }); + // A real (Proxmox) node: its volumes are provisioned by the site agent. + node = await Node.create({ siteId: site.id, name: 'node1', nodeType: 'proxmox' }); + // No ipv4Address on purpose: volumes must still be advertised. + container = await Container.create({ + hostname: 'ct1', + username: 'tester', + nodeId: node.id, + siteId: site.id, + }); + }); + + afterAll(async () => { + await closeDb(); + }); + + test('advertises a resolved non-builtin volume at the site level', async () => { + await Volume.create({ + containerId: container.id, + name: 'data', + hostPath: '/mnt/pve/cephfs/volumes/ct1/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'pending', + }); + + const config = await buildAgentConfig(site.id); + expect(config.site.volumes).toEqual([ + { + id: expect.any(Number), + hostPath: '/mnt/pve/cephfs/volumes/ct1/data', + mode: 'rw', + uid: 100000, + gid: 100000, + }, + ]); + }); + + test('excludes built-in and host-path-less volumes', async () => { + await Volume.create({ + containerId: container.id, + name: 'quick_and_dirty', + hostPath: '/mnt/pve/cephfs/volumes/quick_and_dirty', + mountPath: '/mnt/quick_and_dirty', + mode: 'ro', + builtin: true, + status: 'ready', + }); + await Volume.create({ + containerId: container.id, + name: 'notyet', + hostPath: null, + mountPath: '/mnt/notyet', + mode: 'rw', + status: 'pending', + }); + + const config = await buildAgentConfig(site.id); + expect(config.site.volumes).toEqual([]); + }); + + test('excludes volumes on Docker nodes (Docker provisions its own binds)', async () => { + const dockerNode = await Node.create({ + siteId: site.id, + name: 'docker1', + nodeType: 'docker', + apiUrl: 'unix:///var/run/docker.sock', + }); + const dockerCt = await Container.create({ + hostname: 'dct', + username: 'tester', + nodeId: dockerNode.id, + siteId: site.id, + }); + await Volume.create({ + containerId: dockerCt.id, + name: 'data', + hostPath: '/var/lib/opensource-server/volumes/site-1/dct/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'ready', + }); + + const config = await buildAgentConfig(site.id); + // The Docker-node volume must not be advertised to the site agent. + expect(config.site.volumes).toEqual([]); + }); + + test('excludes volumes on dummy nodes (agentless, marked ready locally)', async () => { + const dummyNode = await Node.create({ siteId: site.id, name: 'dummy1', nodeType: 'dummy' }); + const dummyCt = await Container.create({ + hostname: 'dmy', + username: 'tester', + nodeId: dummyNode.id, + siteId: site.id, + }); + await Volume.create({ + containerId: dummyCt.id, + name: 'data', + hostPath: '/mnt/pve/local/volumes/site-1/tester/dmy/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'ready', + }); + + const config = await buildAgentConfig(site.id); + expect(config.site.volumes).toEqual([]); + }); +}); diff --git a/create-a-container/utils/agent-config.js b/create-a-container/utils/agent-config.js index 7a09c041..86a39f39 100644 --- a/create-a-container/utils/agent-config.js +++ b/create-a-container/utils/agent-config.js @@ -9,7 +9,26 @@ const crypto = require('crypto'); const { Op } = require('sequelize'); -const { Site, Node, Container, Service, HTTPService, TransportService, ExternalDomain } = require('../models'); +const { Site, Node, Container, Service, HTTPService, TransportService, ExternalDomain, Volume } = require('../models'); +const { isAgentlessNodeType } = require('./volumes'); + +// Owning host UID/GID for volume directories: Proxmox maps an unprivileged CT's +// UID/GID 0 to host 100000, so RW volumes must be owned by 100000 to be writable +// from inside a consuming unprivileged container as its root. +// +// The agent applies this ownership BEST-EFFORT so it is correct regardless of +// whether the agent itself runs in an unprivileged or (rare) privileged guest: +// - Unprivileged agent guest (the norm — `pct create` defaults to +// --unprivileged 1, so this includes the embedded Manager agent): the +// agent's root already maps to host 100000, so mkdir yields the right owner. +// A chown to 100000 then targets an id outside the guest's mapped range and +// is a tolerated no-op (EINVAL/EPERM ignored by the agent). +// - Privileged agent guest (only if an operator deliberately creates it with +// --unprivileged 0): the agent is host root, so mkdir would yield a +// root-owned (0:0) dir; the chown to 100000 fixes it so the unprivileged +// consumer can write. +const VOLUME_OWNER_UID = 100000; +const VOLUME_OWNER_GID = 100000; /** * Load a site with everything the agent templates need, serialized to plain @@ -112,6 +131,17 @@ async function buildAgentConfig(siteId) { order: [['id', 'ASC']], }); + // Desired volume directories the agent must ensure exist. Advertised at the + // SITE level (not per node): there is one agent per site, and the volumes root + // lives on storage shared across the site's nodes and bind-mounted into the + // agent, so a single agent creates every site volume's directory regardless of + // which node the container is placed on. Loaded independently of the nginx + // container graph because a volume must be advertised to the agent BEFORE its + // container gets an IP (during creation). Built-in volumes (the retired + // quick_and_dirty mount recorded on pre-existing containers) are excluded — + // the agent only owns user volume directories. + const volumes = await buildSiteVolumes(site); + return { site: { id: site.id, @@ -130,6 +160,8 @@ async function buildAgentConfig(siteId) { macAddress: c.macAddress, })), })), + // Volume directories to ensure for this site (id, hostPath, mode). + volumes, }, nginx: { httpServices, @@ -139,6 +171,51 @@ async function buildAgentConfig(siteId) { }; } +/** + * Build the site-level list of desired volume directories for the agent's + * config snapshot. Each entry carries the host path and mode. Excluded: + * - built-in volumes (admin-provisioned legacy mounts), + * - host-path-less volumes (not derived/provisionable yet), and + * - volumes on agentless nodes (Docker, dummy — see isAgentlessNodeType): + * they're marked ready locally without the agent, and the site agent may not + * even have their paths mounted, so advertising them would let it report a + * spurious failure and flip a valid volume to `failed`. + * Deterministic order keeps the strong ETag stable. + * + * @param {object} site - Site with eager-loaded nodes (used to scope containers) + * @returns {Promise>} + */ +async function buildSiteVolumes(site) { + // Only nodes whose directories the site agent actually provisions (see above). + const agentProvisionedNodeIds = (site.nodes || []) + .filter((n) => !isAgentlessNodeType(n.nodeType)) + .map((n) => n.id); + if (agentProvisionedNodeIds.length === 0) return []; + + const volumes = await Volume.findAll({ + include: [ + { + model: Container, + as: 'container', + attributes: ['id', 'nodeId'], + where: { nodeId: agentProvisionedNodeIds }, + required: true, + }, + ], + where: { builtin: false, hostPath: { [Op.ne]: null } }, + order: [['id', 'ASC']], + }); + + return volumes.map((v) => ({ + id: v.id, + hostPath: v.hostPath, + mode: v.mode, + // Owning host UID/GID the agent applies best-effort (see comment above). + uid: VOLUME_OWNER_UID, + gid: VOLUME_OWNER_GID, + })); +} + /** * Strong ETag over a config snapshot. Deterministic because buildAgentConfig * constructs the object with stable key/array ordering. From 6085131c3c7788d23a56d5f1a63de4773e16e2ac Mon Sep 17 00:00:00 2001 From: Robert Gingras Date: Tue, 29 Sep 2026 11:39:02 -0400 Subject: [PATCH 3/5] agent: create volume directories and report per-volume results (#421) The site agent provisions the host directories the manager's volume barrier waits on. There is one agent per site; the site's shared volumes root is bind-mounted into it (see Deploying Agents), so this one agent provisions volumes for every node in the site. For each volume in the snapshot's site.volumes[]: - Refuse unless hostPath is on a mounted filesystem other than the agent's root (via /proc/self/mountinfo). Otherwise mkdir -p would silently create the path inside the agent container and report success while the Proxmox host path is still missing. Fails closed if the mount table can't be read. - mkdir -p, then chown to the consumer's id-mapped root (100000) best-effort, then chmod (rw 0770, ro 0550). In an unprivileged agent (the pct create default) mkdir already yields that owner and chown returns EINVAL/EPERM, which is tolerated; in a privileged agent the chown is what makes RW volumes writable. Directories are never removed (retain-on-delete). Results are reported on the next check-in as a map keyed by Volume id: - They are persisted in state.json until a check-in carrying them completes, so an exit between reconcile and check-in doesn't lose them. - When any volume failed, the config ETag is not saved, so the next run re-fetches (200, not 304) and retries instead of staying failed until the config changes. --- agent/src/index.ts | 63 ++++++++++++-- agent/src/state.ts | 24 ++++- agent/src/types.ts | 28 ++++++ agent/src/volumes.ts | 174 +++++++++++++++++++++++++++++++++++++ agent/test/state.test.js | 47 ++++++++++ agent/test/volumes.test.js | 121 ++++++++++++++++++++++++++ 6 files changed, 446 insertions(+), 11 deletions(-) create mode 100644 agent/src/volumes.ts create mode 100644 agent/test/state.test.js create mode 100644 agent/test/volumes.test.js diff --git a/agent/src/index.ts b/agent/src/index.ts index d8340758..cf37979a 100644 --- a/agent/src/index.ts +++ b/agent/src/index.ts @@ -16,6 +16,7 @@ import { State } from './state'; import { getPrimaryIpv4, getServiceState, disconnectSystemBus } from './system'; import { checkin } from './api'; import { services, applyService } from './apply'; +import { reconcileVolumes } from './volumes'; import { log } from './log'; import type { CheckinRequest, ServiceStatus } from './types'; @@ -31,13 +32,20 @@ async function buildCheckinBody(cfg: AgentConfig, state: State): Promise 0) { + body.volumes = { ...state.pendingVolumeResults }; + } + return body; } async function main(): Promise { @@ -45,10 +53,24 @@ async function main(): Promise { const state = State.load(cfg.stateDir); log.info(`agent starting: siteId=${cfg.siteId}, manager=${cfg.managerUrl}`); log.debug(`state dir=${cfg.stateDir}, saved etag=${state.etag ?? '(none)'}`); + if (Object.keys(state.pendingVolumeResults).length > 0) { + log.debug(`carrying ${Object.keys(state.pendingVolumeResults).length} pending volume result(s) from a prior run`); + } for (let pass = 0; pass < MAX_PASSES; pass++) { log.debug(`check-in pass ${pass + 1}/${MAX_PASSES}`); + // Remember what this check-in body carries so we can clear it only once the + // request has actually completed (the manager processes the `volumes` map + // before computing the ETag, so a 200 or a 304 both mean it was delivered). + const carriedVolumeIds = Object.keys(state.pendingVolumeResults); const result = await checkin(cfg, await buildCheckinBody(cfg, state), state.etag); + + // The check-in completed, so any results it carried are now delivered. + if (carriedVolumeIds.length > 0) { + for (const id of carriedVolumeIds) delete state.pendingVolumeResults[id]; + state.save(); + } + if (result.notModified) { log.info('check-in: config unchanged (304), nothing to apply'); return; @@ -59,18 +81,45 @@ async function main(): Promise { state.lastApply[svc.unit] = await applyService(svc, result.config, cfg); } - // The ETag is saved even after a failed apply: a rejected config won't - // fix itself without a server-side change (which changes the ETag), and - // the failure has been reported via lastApply. - state.etag = result.etag; + // Ensure volume directories exist from the new snapshot; stash results in + // persistent state so they are reported on the next check-in and survive a + // process exit in between (they are cleared once delivered, above). + const volumeResults = reconcileVolumes(result.config); + if (volumeResults) { + Object.assign(state.pendingVolumeResults, volumeResults); + } + + // A failed volume mkdir is typically transient (a not-yet-mounted volumes + // root, a slow shared mount, a momentary permission glitch) and WILL fix + // itself on a retry without any server-side config change. If we saved the + // ETag now, the next run would get a 304 and never re-run the reconcile, + // leaving the volume `failed` until the config changes. So when any volume + // failed this pass, do NOT persist the ETag — forcing the next check-in to + // re-fetch the config (200, not 304) and re-run the reconcile. Service + // applies keep the existing "save even on failure" behavior (a rejected + // nginx/dnsmasq config won't fix itself without a server-side change). + const volumeFailed = volumeResults + ? Object.values(volumeResults).some((r) => !r.applied) + : false; + if (volumeFailed) { + log.warn('one or more volume directories failed to provision; will retry on next check-in'); + state.etag = undefined; + } else { + state.etag = result.etag; + } state.save(); } // MAX_PASSES exhausted (flapping server-side config): check in once more so - // the final pass' apply results reach the manager instead of going stale - // until the next timer run. + // the final pass' apply/volume results reach the manager instead of going + // stale until the next timer run. Clear delivered results afterwards. log.warn(`reached MAX_PASSES (${MAX_PASSES}) without a stable config; reporting final results`); + const carried = Object.keys(state.pendingVolumeResults); await checkin(cfg, await buildCheckinBody(cfg, state), state.etag); + if (carried.length > 0) { + for (const id of carried) delete state.pendingVolumeResults[id]; + state.save(); + } } main() diff --git a/agent/src/state.ts b/agent/src/state.ts index 93ae019e..bd691b2b 100644 --- a/agent/src/state.ts +++ b/agent/src/state.ts @@ -1,14 +1,21 @@ /** Persistent agent state: last applied config ETag + per-service apply - * results, stored as JSON under the state dir. */ + * results + pending per-volume provisioning results, stored as JSON under the + * state dir. */ import fs from 'fs'; import path from 'path'; import { log } from './log'; -import type { ApplyResult } from './types'; +import type { ApplyResult, VolumeResult } from './types'; export class State { etag?: string; lastApply: Record = {}; + // Volume provisioning results not yet confirmed delivered to the manager. + // Persisted so they survive a process exit between reconcile and the next + // check-in — otherwise a saved ETag would 304 the next run and the result + // would be lost, leaving the volume pending until the create barrier times + // out. Cleared only after a check-in that carried them completes. + pendingVolumeResults: Record = {}; private constructor(private readonly file: string) {} @@ -22,9 +29,14 @@ export class State { throw err; } try { - const data = JSON.parse(raw) as { etag?: string; lastApply?: Record }; + const data = JSON.parse(raw) as { + etag?: string; + lastApply?: Record; + pendingVolumeResults?: Record; + }; state.etag = data.etag; state.lastApply = data.lastApply ?? {}; + state.pendingVolumeResults = data.pendingVolumeResults ?? {}; } catch (err) { if (!(err instanceof SyntaxError)) throw err; // A corrupt state file just means a full re-apply on this run. @@ -37,7 +49,11 @@ export class State { fs.mkdirSync(path.dirname(this.file), { recursive: true }); fs.writeFileSync( this.file, - JSON.stringify({ etag: this.etag, lastApply: this.lastApply }, null, 2), + JSON.stringify( + { etag: this.etag, lastApply: this.lastApply, pendingVolumeResults: this.pendingVolumeResults }, + null, + 2, + ), ); } } diff --git a/agent/src/types.ts b/agent/src/types.ts index b6775d3f..f9175779 100644 --- a/agent/src/types.ts +++ b/agent/src/types.ts @@ -19,6 +19,32 @@ export interface CheckinRequest { currentTime: number; ipv4Address: string | null; services: Record; + /** + * Per-volume directory-provisioning results, keyed by the manager-assigned + * Volume id: `{ : { applied, message? } }`. Present only when the + * agent processed volumes this pass. The manager writes these into + * Volume.status (ready/failed) at check-in. + */ + volumes?: Record; +} + +/** Outcome of ensuring one volume directory on this node. */ +export interface VolumeResult { + applied: boolean; + message?: string; +} + +/** A volume directory the agent must ensure exists, as carried in the config + * snapshot at the site level. `uid`/`gid` are the owning host ids (the + * unprivileged CT's id-mapped root); the agent applies them best-effort — a + * no-op in an unprivileged agent guest (mkdir already yields that owner) and the + * real fix in a privileged agent guest (mkdir would otherwise be root-owned). */ +export interface SiteVolume { + id: number; + hostPath: string; + mode: 'ro' | 'rw'; + uid: number; + gid: number; } export interface SiteContainer { @@ -45,6 +71,8 @@ export interface SiteInfo { gateway: string | null; dnsForwarders: string | null; nodes: SiteNode[]; + /** Volume directories to ensure for the whole site. Absent on older managers. */ + volumes?: SiteVolume[]; } export interface HttpService { diff --git a/agent/src/volumes.ts b/agent/src/volumes.ts new file mode 100644 index 00000000..7480bfee --- /dev/null +++ b/agent/src/volumes.ts @@ -0,0 +1,174 @@ +/** + * Volume directory reconciliation (issue #421). + * + * The manager includes, at the site level, the volume directories that must + * exist (id, hostPath, mode, and the owning host uid/gid). The shared volumes + * root is bind-mounted into the agent guest by the installer, so these paths are + * visible and writable here. + * + * Ownership is applied BEST-EFFORT via chown, so it is correct whether the + * agent runs in an unprivileged or (rare) privileged guest: + * - Unprivileged agent guest (the norm — `pct create` defaults to + * `--unprivileged 1`, which includes the embedded Manager agent): the + * agent's root already maps to the host owner (100000), so mkdir yields the + * right owner. The chown to 100000 then targets an id outside the guest's + * mapped range and is a tolerated no-op (see EINVAL/EPERM/ENOSYS below). + * - Privileged agent guest (only if deliberately created with + * `--unprivileged 0`): the agent is host root, so mkdir would otherwise + * create a root-owned directory; the chown fixes it so an unprivileged + * consuming container can write RW volumes. + * A failed chown is logged and ignored — it never fails the volume, since the + * unprivileged-guest case relies on the id-map, not the chown. + * + * There is one agent per site (not per node); the volumes root lives on storage + * shared across the site's nodes, so this single agent provisions every site + * volume regardless of which node hosts the container. + * + * Results are reported per volume id at the next check-in, which the manager + * writes into Volume.status. Directory creation is retain-only: the agent never + * removes a volume directory, so data survives container delete + recreate. + */ + +import fs from 'fs'; +import { log } from './log'; +import type { SiteConfig, SiteVolume, VolumeResult } from './types'; + +// Mode for created directories. RW volumes get owner/group rwx (the id-mapped +// container root owns the dir, so it can write); RO volumes are r-x. World bits +// are left closed so other tenants can't read another container's data. +const RW_MODE = 0o0770; +const RO_MODE = 0o0550; + +/** + * Collect the volumes to provision from the config snapshot (site-level). + * @param {SiteConfig} config + * @returns {SiteVolume[]} + */ +export function volumesForSite(config: SiteConfig): SiteVolume[] { + return config.site?.volumes ?? []; +} + +/** + * Mount points visible to this process, from /proc/self/mountinfo (field 5, + * with the kernel's octal escapes for space/tab/newline/backslash decoded). + * @returns {string[]} + */ +export function readMountPoints(): string[] { + const text = fs.readFileSync('/proc/self/mountinfo', 'utf8'); + return text + .split('\n') + .filter(Boolean) + .map((line) => line.split(' ')[4]) + .filter((mp): mp is string => typeof mp === 'string') + .map((mp) => mp.replace(/\\([0-7]{3})/g, (_, oct: string) => String.fromCharCode(parseInt(oct, 8)))); +} + +/** + * The deepest mount point that contains `path` ('/' when nothing more specific + * does). + * @param {string} path + * @param {string[]} mountPoints + * @returns {string} + */ +export function containingMountPoint(path: string, mountPoints: string[]): string { + let best = '/'; + for (const mp of mountPoints) { + if (mp === '/') continue; + if ((path === mp || path.startsWith(`${mp}/`)) && mp.length > best.length) best = mp; + } + return best; +} + +/** + * Ensure a single volume directory exists with the right owner and mode. + * Idempotent: mkdir -p, best-effort chown, then chmod every run (cheap, + * self-healing). + * + * Refuses to create anything unless `hostPath` lies on a mount other than the + * agent's root filesystem. The volumes root must be bind-mounted into the agent + * (see "Deploying Agents"); if it isn't, `mkdir -p` would silently create the + * path inside the agent guest's own rootfs and report success, letting the + * manager's readiness barrier pass while the Proxmox host path is still + * missing — so the mpN attach would then fail. Reporting a failure here + * surfaces the misconfiguration as `Volume.status = failed` with a clear + * message instead. + * @param {SiteVolume} volume + * @param {string[]} mountPoints + * @returns {VolumeResult} + */ +function ensureVolume(volume: SiteVolume, mountPoints: string[]): VolumeResult { + const { hostPath, mode, uid, gid } = volume; + try { + if (containingMountPoint(hostPath, mountPoints) === '/') { + throw new Error( + `${hostPath} is not on a mounted volumes root (it would be created inside the agent's own ` + + 'filesystem); bind-mount the shared volumes root into the agent container', + ); + } + fs.mkdirSync(hostPath, { recursive: true }); + // Best-effort ownership (see module header): the real fix in a privileged + // agent guest (agent is host root), and a harmless no-op in an unprivileged + // one — where mkdir already yields the correct owner via the id-map. Several + // failures are therefore EXPECTED and tolerated rather than failing the + // volume: + // - EINVAL: the target host id (e.g. 100000) is outside the unprivileged + // guest's mapped range, so it isn't a valid id to chown to from inside + // the guest. This is the normal unprivileged case — ownership is already + // correct from the id-map, so ignore it. + // - EPERM: the guest lacks CAP_CHOWN. + // - ENOSYS: chown unsupported. + if (typeof uid === 'number' && typeof gid === 'number') { + try { + fs.chownSync(hostPath, uid, gid); + } catch (chownErr) { + const code = (chownErr as NodeJS.ErrnoException).code; + if (code === 'EINVAL' || code === 'EPERM' || code === 'ENOSYS') { + log.debug(`volume ${volume.id}: chown to ${uid}:${gid} skipped (${code}); relying on id-map`); + } else { + throw chownErr; + } + } + } + fs.chmodSync(hostPath, mode === 'rw' ? RW_MODE : RO_MODE); + log.debug(`volume ${volume.id}: ensured ${hostPath} (mode=${mode}, owner=${uid}:${gid})`); + return { applied: true }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + log.error(`volume ${volume.id}: failed to ensure ${hostPath}: ${message}`); + return { applied: false, message }; + } +} + +/** + * Reconcile all volume directories for this site. Returns a results map keyed + * by volume id for the check-in body, or undefined when there is nothing to do + * (so the check-in omits the field on older managers / empty sites). + * @param {SiteConfig} config + * @param {string[]} [mountPoints] Mount points to check paths against; + * defaults to this process's /proc/self/mountinfo (injectable for tests). + * @returns {Record | undefined} + */ +export function reconcileVolumes( + config: SiteConfig, + mountPoints?: string[], +): Record | undefined { + const volumes = volumesForSite(config); + if (volumes.length === 0) return undefined; + + log.info(`volumes: ensuring ${volumes.length} directory(ies)`); + let mounts: string[]; + try { + mounts = mountPoints ?? readMountPoints(); + } catch (err) { + // Can't tell whether the volumes root is mounted, so fail closed rather + // than risk reporting a guest-local directory as provisioned. + const message = `cannot read mount table: ${err instanceof Error ? err.message : String(err)}`; + log.error(`volumes: ${message}`); + return Object.fromEntries(volumes.map((v) => [String(v.id), { applied: false, message }])); + } + const results: Record = {}; + for (const volume of volumes) { + results[String(volume.id)] = ensureVolume(volume, mounts); + } + return results; +} diff --git a/agent/test/state.test.js b/agent/test/state.test.js new file mode 100644 index 00000000..9381d17c --- /dev/null +++ b/agent/test/state.test.js @@ -0,0 +1,47 @@ +/** + * State persistence test (node --test): pendingVolumeResults must survive a + * save/load round-trip so a process exit between reconcile and the next + * check-in does not lose volume results (which would otherwise leave a volume + * pending behind a cached ETag until the create barrier times out). + */ + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { State } = require('../dist/state.js'); + +test('pendingVolumeResults round-trips through save/load', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-state-')); + + const s1 = State.load(dir); + s1.etag = '"abc"'; + s1.pendingVolumeResults['42'] = { applied: false, message: 'mkdir: boom' }; + s1.pendingVolumeResults['43'] = { applied: true }; + s1.save(); + + const s2 = State.load(dir); + assert.equal(s2.etag, '"abc"'); + assert.deepEqual(s2.pendingVolumeResults, { + 42: { applied: false, message: 'mkdir: boom' }, + 43: { applied: true }, + }); + + // Clearing a delivered result and re-saving persists the removal. + delete s2.pendingVolumeResults['43']; + s2.save(); + const s3 = State.load(dir); + assert.deepEqual(Object.keys(s3.pendingVolumeResults), ['42']); + + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('load with no state file starts empty', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-state-')); + const s = State.load(dir); + assert.equal(s.etag, undefined); + assert.deepEqual(s.pendingVolumeResults, {}); + fs.rmSync(dir, { recursive: true, force: true }); +}); diff --git a/agent/test/volumes.test.js b/agent/test/volumes.test.js new file mode 100644 index 00000000..93d1d0e7 --- /dev/null +++ b/agent/test/volumes.test.js @@ -0,0 +1,121 @@ +/** + * Volume reconcile tests (node --test). Verifies volumesForSite reads the + * site-level volumes, that reconcileVolumes creates directories and reports + * per-volume results keyed by volume id, and that a chown to an unmapped host + * id (the normal unprivileged-agent case) is tolerated rather than failing the + * volume. + */ + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { + volumesForSite, + reconcileVolumes, + containingMountPoint, + readMountPoints, +} = require('../dist/volumes.js'); + +function makeConfig(volumes) { + return { + site: { + id: 1, + name: 'site', + internalDomain: null, + dhcpRange: null, + subnetMask: null, + gateway: null, + dnsForwarders: null, + nodes: [], + volumes, + }, + nginx: { httpServices: [], streamServices: [], externalDomains: [] }, + }; +} + +test('volumesForSite returns the site-level volumes', () => { + const cfg = makeConfig([{ id: 1, hostPath: '/tmp/x', mode: 'rw' }]); + assert.equal(volumesForSite(cfg).length, 1); + assert.equal(volumesForSite({ site: null, nginx: {} }).length, 0); +}); + +test('reconcileVolumes creates directories and reports per-volume results', () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'vol-test-')); + const dir1 = path.join(base, 'data'); + const dir2 = path.join(base, 'logs'); + const cfg = makeConfig([ + { id: 11, hostPath: dir1, mode: 'rw' }, + { id: 12, hostPath: dir2, mode: 'ro' }, + ]); + + const results = reconcileVolumes(cfg, [base]); + assert.ok(results, 'results should be present'); + assert.deepEqual(Object.keys(results).sort(), ['11', '12']); + assert.equal(results['11'].applied, true); + assert.equal(results['12'].applied, true); + assert.ok(fs.existsSync(dir1)); + assert.ok(fs.existsSync(dir2)); + + fs.rmSync(base, { recursive: true, force: true }); +}); + +test('reconcileVolumes returns undefined when the site has no volumes', () => { + assert.equal(reconcileVolumes(makeConfig([]), []), undefined); +}); + +test('tolerates a chown to an unmapped/denied host id (unprivileged agent case)', () => { + // Ask for owner 100000: as a non-root runner (or inside an unprivileged + // namespace) chown will fail with EPERM/EINVAL. The volume must still be + // reported applied — ownership is established by the id-map, not the chown. + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'vol-test-')); + const dir = path.join(base, 'data'); + const cfg = makeConfig([{ id: 7, hostPath: dir, mode: 'rw', uid: 100000, gid: 100000 }]); + const results = reconcileVolumes(cfg, [base]); + assert.equal(results['7'].applied, true); + assert.ok(fs.existsSync(dir)); + fs.rmSync(base, { recursive: true, force: true }); +}); + +test('reconcileVolumes reports a failure for an uncreatable path', () => { + // A path under a file (not a dir) can't be mkdir'd. + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'vol-test-')); + const filePath = path.join(base, 'afile'); + fs.writeFileSync(filePath, 'x'); + const badPath = path.join(filePath, 'sub'); + const cfg = makeConfig([{ id: 99, hostPath: badPath, mode: 'rw' }]); + const results = reconcileVolumes(cfg, [base]); + assert.equal(results['99'].applied, false); + assert.ok(results['99'].message); + fs.rmSync(base, { recursive: true, force: true }); +}); + +test('refuses a path that is not on a mounted volumes root (and creates nothing)', () => { + // The volumes root was never bind-mounted into the agent: the only mount + // containing the path is '/', so mkdir -p would create it inside the agent's + // own filesystem. It must be reported as a failure instead. + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'vol-test-')); + const dir = path.join(base, 'data'); + const results = reconcileVolumes(makeConfig([{ id: 5, hostPath: dir, mode: 'rw' }]), ['/']); + assert.equal(results['5'].applied, false); + assert.match(results['5'].message, /not on a mounted volumes root/); + assert.equal(fs.existsSync(dir), false); + fs.rmSync(base, { recursive: true, force: true }); +}); + +test('containingMountPoint picks the deepest enclosing mount, not a prefix sibling', () => { + const mounts = ['/', '/proc', '/mnt/pve/cephfs', '/mnt/pve/cephfs/volumes']; + assert.equal(containingMountPoint('/mnt/pve/cephfs/volumes/site-1/u/h/data', mounts), '/mnt/pve/cephfs/volumes'); + assert.equal(containingMountPoint('/mnt/pve/cephfs/other', mounts), '/mnt/pve/cephfs'); + // '/mnt/pve/cephfs-2' only shares a string prefix with '/mnt/pve/cephfs'. + assert.equal(containingMountPoint('/mnt/pve/cephfs-2/x', mounts), '/'); + assert.equal(containingMountPoint('/var/lib/vz/volumes/x', mounts), '/'); +}); + +test('readMountPoints parses this process\'s mount table', () => { + const mounts = readMountPoints(); + assert.ok(mounts.includes('/')); + assert.ok(mounts.includes('/proc')); +}); From 04089f2a40d172546796b176d355b7b7325ba0e7 Mon Sep 17 00:00:00 2001 From: Robert Gingras Date: Tue, 29 Sep 2026 11:40:35 -0400 Subject: [PATCH 4/5] volumes: expose volumes in the API, OpenAPI spec, and client (#421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Container API: - POST /containers accepts volumes: [{ name, mountPath, mode }], persisted `pending` for the create job to provision. - PUT /containers/:id accepts attaches and { id, detach: true }. On a provisioned container, any volume change enqueues a reconfigure job (it derives paths, waits for the agent, and applies mpN), even without restart: true; on an unprovisioned one the rows apply at create. - Validation: strict numeric detach ids; duplicate names/mounts within the request or against existing volumes (409); the reserved quick_and_dirty name and mount (compared canonically) are rejected. hostPath is never accepted from clients. - env/entrypoint are only changed when the request includes those keys, so a volume-only update (with or without restart) no longer clears them. - Container payloads include volumes (name, mountPath, mode, status, ...). Node API: create/update return advisory `warnings` when the volume storage isn't path-backed, shared, or active on every cluster node (warning, not an error — single-node sites are fine). OpenAPI: Volume, VolumeAttach, VolumeDetach (VolumeChange oneOf), the new container/node fields, and the check-in volumes results map. Client: a Volumes editor on the container form (attach, ro/rw, status, detach; volume changes count toward restart detection) and node-save warning toasts. --- create-a-container/client/src/lib/types.ts | 24 ++ .../pages/containers/ContainerFormPage.tsx | 177 +++++++++++- .../client/src/pages/nodes/NodeFormPage.tsx | 7 +- create-a-container/openapi.v1.yaml | 94 +++++- .../v1/__tests__/containers.volumes.test.js | 186 ++++++++++++ .../__tests__/nodes.volume-warnings.test.js | 93 ++++++ .../__tests__/normalize-volume-attach.test.js | 78 +++++ .../routers/api/v1/containers.js | 268 ++++++++++++++++-- create-a-container/routers/api/v1/nodes.js | 86 +++++- 9 files changed, 979 insertions(+), 34 deletions(-) create mode 100644 create-a-container/routers/api/v1/__tests__/containers.volumes.test.js create mode 100644 create-a-container/routers/api/v1/__tests__/nodes.volume-warnings.test.js create mode 100644 create-a-container/routers/api/v1/__tests__/normalize-volume-attach.test.js diff --git a/create-a-container/client/src/lib/types.ts b/create-a-container/client/src/lib/types.ts index 55b5df7a..c3df1eac 100644 --- a/create-a-container/client/src/lib/types.ts +++ b/create-a-container/client/src/lib/types.ts @@ -29,6 +29,9 @@ export interface Node { networkBridge: string; nvidiaAvailable: boolean; hasSecret: boolean; + /** Advisory warnings returned by the last create/update save (e.g. volume + * storage not shared across the cluster). Present on save responses only. */ + warnings?: string[]; } /** Used/total byte pair for a node hardware resource. */ @@ -221,6 +224,26 @@ export interface ContainerService { dnsService: ServiceDns | null; } +/** A container bind-mount volume (issue #421). */ +export interface Volume { + id: number; + name: string; + mountPath: string; + mode: 'ro' | 'rw'; + scope: string; + builtin: boolean; + status: 'pending' | 'ready' | 'failed'; + statusMessage: string | null; + appliedAt: string | null; +} + +/** A volume to attach on create/update (hostPath is derived server-side). */ +export interface VolumeAttach { + name: string; + mountPath: string; + mode: 'ro' | 'rw'; +} + export interface Container { id: number; containerId: number | null; @@ -244,6 +267,7 @@ export interface Container { nodeName: string | null; nodeApiUrl: string | null; services: ContainerService[]; + volumes: Volume[]; createdAt: string; } diff --git a/create-a-container/client/src/pages/containers/ContainerFormPage.tsx b/create-a-container/client/src/pages/containers/ContainerFormPage.tsx index ef6e909e..32fe76be 100644 --- a/create-a-container/client/src/pages/containers/ContainerFormPage.tsx +++ b/create-a-container/client/src/pages/containers/ContainerFormPage.tsx @@ -79,6 +79,25 @@ const serviceSchema = z const envVarSchema = z.object({ key: z.string(), value: z.string() }); +// A volume attach/detach row. Existing rows carry `id` and are shown read-only +// unless flagged `detach`; new rows carry name/mountPath/mode. +const volumeSchema = z.object({ + id: z.number().optional(), + name: z + .string() + .min(1, 'Required') + .regex( + /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/, + 'Letters, digits, dot, dash, underscore; no traversal or separators', + ), + mountPath: z.string().min(1, 'Required').regex(/^\//, 'Must be an absolute path'), + mode: z.enum(['ro', 'rw']), + builtin: z.boolean().optional(), + detach: z.boolean().optional(), + status: z.string().optional(), + statusMessage: z.string().nullable().optional(), +}); + const schema = z.object({ hostname: z .string() @@ -94,6 +113,7 @@ const schema = z.object({ restart: z.boolean().optional(), services: z.array(serviceSchema), environmentVars: z.array(envVarSchema), + volumes: z.array(volumeSchema), // Usernames to share a new container with (collaborators). Existence is // validated server-side on submit. Unused in edit mode (live manager instead). collaborators: z.array(z.string()), @@ -163,6 +183,7 @@ export function ContainerFormPage() { defaultValues: { services: [], environmentVars: [], + volumes: [], collaborators: [], nvidiaRequested: false, restart: false, @@ -170,6 +191,7 @@ export function ContainerFormPage() { }); const services = useFieldArray({ control, name: 'services' }); const envVars = useFieldArray({ control, name: 'environmentVars' }); + const volumes = useFieldArray({ control, name: 'volumes' }); // Guards the one-time form initialization from the loaded container (edit). const initializedRef = useRef(false); @@ -191,17 +213,26 @@ export function ContainerFormPage() { const collaborators = watch('collaborators') || []; const watchedEntrypoint = watch('entrypoint'); const watchedEnvVars = watch('environmentVars'); + const watchedVolumes = watch('volumes'); - // True when the form's env vars or entrypoint differ from the saved - // container — the changes that only take effect after a restart (#449). + // True when the form's env vars, entrypoint, or volumes differ from the saved + // container — the changes that only take effect after a restart (#449). Volume + // attach/detach requires a restart to (un)mount, and the server enqueues a + // reconfigure job for it, so surface it here too. const requiresRestart = useMemo(() => { if (!isEdit || !container) return false; if ((watchedEntrypoint || '') !== (container.entrypoint || '')) return true; const saved = new Map(Object.entries(container.environmentVars || {})); const current = (watchedEnvVars || []).filter((e) => e.key.trim()); if (current.length !== saved.size) return true; - return current.some((e) => saved.get(e.key) !== e.value); - }, [isEdit, container, watchedEntrypoint, watchedEnvVars]); + if (current.some((e) => saved.get(e.key) !== e.value)) return true; + // A volume was attached (a new row with no id) or detached (an existing row + // flagged for detach) — both change the container's mounts. + const volumeChanged = (watchedVolumes || []).some( + (v) => (!v.id && (v.name?.trim() || v.mountPath?.trim())) || (v.id && v.detach && !v.builtin), + ); + return volumeChanged; + }, [isEdit, container, watchedEntrypoint, watchedEnvVars, watchedVolumes]); // The restart toggle follows restart-requiring edits (auto-on, so the user // isn't left wondering why changes didn't apply) until the user overrides @@ -247,6 +278,16 @@ export function ContainerFormPage() { key, value, })), + volumes: (container.volumes || []).map((v) => ({ + id: v.id, + name: v.name, + mountPath: v.mountPath, + mode: v.mode, + builtin: v.builtin, + detach: false, + status: v.status, + statusMessage: v.statusMessage, + })), collaborators: [], }); } @@ -350,6 +391,16 @@ export function ContainerFormPage() { nvidiaRequested: values.nvidiaRequested, services: servicesObj, environmentVars: values.environmentVars.filter((e) => e.key.trim()), + // Volume attach/detach. New rows (no id) attach; existing rows flagged + // `detach` detach; built-in and untouched existing rows are omitted. + volumes: values.volumes.flatMap( + (v): Array> => { + if (v.id) { + return v.detach && !v.builtin ? [{ id: v.id, detach: true }] : []; + } + return [{ name: v.name.trim(), mountPath: v.mountPath.trim(), mode: v.mode }]; + }, + ), restart: values.restart, // Only meaningful on create; the edit form manages sharing live. collaborators: values.collaborators, @@ -600,7 +651,7 @@ export function ContainerFormPage() {
{ restartTouchedRef.current = true; @@ -816,6 +867,122 @@ export function ContainerFormPage() { ))} + + + Volumes + + + +

+ Persistent bind-mount directories. Data survives delete + recreate on the same + hostname. Volume storage is not backed up by the platform. +

+ {volumes.fields.length === 0 && ( +

No volumes.

+ )} + {volumes.fields.map((f, idx) => { + // NOTE: `f.id` is react-hook-form's internal field key (always + // present), NOT the server volume id. Read the watched row so + // `existing`/`builtin` reflect the actual data — otherwise every + // freshly appended row would look "existing" and be read-only. + const row = watch(`volumes.${idx}`); + const existing = row?.id !== undefined && row?.id !== null; + const builtin = !!row?.builtin; + const detaching = row?.detach; + return ( +
+ {existing ? ( + <> + + +
+ {(row?.mode ?? '').toUpperCase()} + {row?.status ? ` · ${row.status}` : ''} + {builtin ? ' · built-in' : ''} +
+ {builtin ? ( + — + ) : ( + + )} + + ) : ( + <> + + + + + + )} +
+ ); + })} + + Sharing diff --git a/create-a-container/client/src/pages/nodes/NodeFormPage.tsx b/create-a-container/client/src/pages/nodes/NodeFormPage.tsx index 07e0d6ef..74b139df 100644 --- a/create-a-container/client/src/pages/nodes/NodeFormPage.tsx +++ b/create-a-container/client/src/pages/nodes/NodeFormPage.tsx @@ -91,8 +91,13 @@ export function NodeFormPage() { ? api.put(`/api/v1/sites/${siteId}/nodes/${id}`, payload) : api.post(`/api/v1/sites/${siteId}/nodes`, payload); }, - onSuccess: () => { + onSuccess: (saved) => { toast.success(isEdit ? 'Node updated' : 'Node created'); + // Surface advisory volume-storage warnings (issue #421) so the admin sees + // them even though we navigate away from the form. + for (const w of saved?.warnings ?? []) { + toast.warning(w); + } qc.invalidateQueries({ queryKey: keys.nodes(siteId!) }); navigate(`/sites/${siteId}/nodes`); }, diff --git a/create-a-container/openapi.v1.yaml b/create-a-container/openapi.v1.yaml index 1c1f24f6..81f6e935 100644 --- a/create-a-container/openapi.v1.yaml +++ b/create-a-container/openapi.v1.yaml @@ -179,6 +179,9 @@ components: (max of the services' lastAccessedAt). Null when never accessed. nodeName: { type: string, nullable: true } nodeApiUrl: { type: string, nullable: true } + volumes: + type: array + items: { $ref: '#/components/schemas/Volume' } services: type: array items: { $ref: '#/components/schemas/ContainerService' } @@ -266,6 +269,53 @@ components: properties: key: { type: string } value: { type: string } + Volume: + type: object + description: >- + A container bind-mount volume (issue #421). Host directories are + created by the site agent and attached as Proxmox mpN bind mounts. On + Docker nodes, volumes map to Docker binds. + properties: + id: { type: integer } + name: { type: string, description: 'Safe path segment; no traversal/separators' } + mountPath: { type: string, description: 'Absolute guest mount point (e.g. /mnt/data)' } + mode: { type: string, enum: [ro, rw] } + scope: { type: string, description: 'Currently always `container` (per-container). Per-user/site/node is a later extension.' } + builtin: { type: boolean, description: 'True only for the retired quick_and_dirty mount recorded on pre-existing containers. Never set on new volumes and never re-applied.' } + status: + type: string + enum: [pending, ready, failed] + description: >- + Directory-readiness lifecycle. `pending` until the site agent + creates the host directory, then `ready`; `failed` on a mkdir/chown + error (see statusMessage). The create job blocks on `ready` before + attaching the mount. + statusMessage: { type: string, nullable: true, description: 'Agent error detail when status is failed' } + appliedAt: { type: string, format: date-time, nullable: true } + VolumeAttach: + type: object + required: [name, mountPath, mode] + description: >- + A volume to attach. `hostPath` is NOT accepted from clients — it is + derived server-side from the node storage's configured path. + properties: + name: { type: string, description: 'Safe path segment (letters, digits, dot, dash, underscore)' } + mountPath: { type: string, description: 'Absolute guest mount point; no provider delimiters (comma/colon), backslashes, whitespace, control chars, or . / .. segments' } + mode: { type: string, enum: [ro, rw] } + VolumeDetach: + type: object + required: [id, detach] + description: 'Detach an existing volume by id (update only).' + properties: + id: { type: integer, description: 'Existing volume id to detach' } + detach: { type: boolean, enum: [true], description: 'Must be true to detach' } + VolumeChange: + description: >- + A single volume change on update: either an attach (`{ name, mountPath, + mode }`) or a detach (`{ id, detach: true }`). + oneOf: + - $ref: '#/components/schemas/VolumeAttach' + - $ref: '#/components/schemas/VolumeDetach' Node: type: object properties: @@ -285,6 +335,13 @@ components: networkBridge: { type: string } nvidiaAvailable: { type: boolean } hasSecret: { type: boolean } + warnings: + type: array + items: { type: string } + description: >- + Advisory warnings from the last save (create/update), e.g. the + volume storage is not shared across the cluster (issue #421). + Present on create/update responses only. NodeInput: type: object required: [name] @@ -787,6 +844,10 @@ paths: environmentVars: type: array items: { $ref: '#/components/schemas/EnvVar' } + volumes: + type: array + description: Volumes (bind mounts) to attach to the new container + items: { $ref: '#/components/schemas/VolumeAttach' } services: type: object description: Keyed map of services to create (keys are arbitrary) @@ -806,7 +867,7 @@ paths: jobId: { type: integer } hostname: { type: string } status: { $ref: '#/components/schemas/ContainerStatus' } - '400': { description: 'invalid_request or invalid_service', content: { application/json: { schema: { $ref: '#/components/schemas/Error' } } } } + '400': { description: 'invalid_request, invalid_service, or invalid_volume', content: { application/json: { schema: { $ref: '#/components/schemas/Error' } } } } '403': { description: 'forbidden — non-admins may not create containers for other users', content: { application/json: { schema: { $ref: '#/components/schemas/Error' } } } } '404': { description: 'site_not_found, or user_not_found when a collaborator does not exist', content: { application/json: { schema: { $ref: '#/components/schemas/Error' } } } } '409': { description: 'No provisionable node available (code: no_node or no_nvidia_node)', content: { application/json: { schema: { $ref: '#/components/schemas/Error' } } } } @@ -876,6 +937,13 @@ paths: items: { $ref: '#/components/schemas/EnvVar' } description: Full replacement set. Omitting it clears all user env vars (unless the request is restart-only). entrypoint: { type: string, nullable: true, description: Omitting/blank clears the entrypoint (unless the request is restart-only) } + volumes: + type: array + description: >- + Volume changes. Attach entries are `{ name, mountPath, mode }`; + detach entries are `{ id, detach: true }`. A volume mutation on + a provisioned container enqueues a reconfigure job to apply it. + items: { $ref: '#/components/schemas/VolumeChange' } restart: { type: boolean, description: 'A restart job is enqueued only when true — config changes alone never restart the container (they apply on the next restart); alone, it performs a restart-only request' } responses: '200': @@ -1184,12 +1252,12 @@ paths: tags: [Agents] summary: Agent check-in description: | - Site agents POST their system info and per-service status every 30 - seconds. The response carries the site's config snapshot with a strong - `ETag`; send it back via `If-None-Match` to receive `304 Not Modified` - when nothing changed. Allowed from localhost without credentials - (manager bootstrap) or with an admin API key. Exempt from the CSRF - guard. + Site agents POST their system info, per-service status, and per-volume + directory-provisioning results every 30 seconds. The response carries + the site's config snapshot with a strong `ETag`; send it back via + `If-None-Match` to receive `304 Not Modified` when nothing changed. + Allowed from localhost without credentials (manager bootstrap) or with + an admin API key. Exempt from the CSRF guard. parameters: - in: header name: If-None-Match @@ -1214,6 +1282,18 @@ paths: properties: state: { type: string, description: 'systemd ActiveState: active, inactive, failed, ...' } lastApply: { type: string, enum: [success, failure, unknown] } + volumes: + type: object + description: >- + Per-volume directory-provisioning results keyed by the + manager-assigned Volume id. The manager writes these into + Volume.status (ready when applied, failed otherwise) at + check-in (issue #421). + additionalProperties: + type: object + properties: + applied: { type: boolean } + message: { type: string, description: 'Failure detail (mkdir/chown error)' } responses: '200': description: 'Config snapshot (`{ data: { site, nginx } }`) with `ETag` header' diff --git a/create-a-container/routers/api/v1/__tests__/containers.volumes.test.js b/create-a-container/routers/api/v1/__tests__/containers.volumes.test.js new file mode 100644 index 00000000..15cff2ae --- /dev/null +++ b/create-a-container/routers/api/v1/__tests__/containers.volumes.test.js @@ -0,0 +1,186 @@ +/** + * PUT /api/v1/sites/:siteId/containers/:id — volume attach/detach (issue #421). + * Verifies that a volume mutation on a PROVISIONED container enqueues a + * reconfigure job even without `restart: true` (the migration reconciler is + * one-time, so normal attach/detach must schedule its own reconcile), and that + * validation (reserved mount, duplicates) is enforced. + */ + +const request = require('supertest'); +const { buildApp, bearer } = require('../../../../tests/helpers/app'); +const { resetDb, closeDb, createUser, createApiKey } = require('../../../../tests/helpers/db'); +const { Site, Node, Container, Volume, Job } = require('../../../../models'); + +describe('PUT container volumes', () => { + let app; + let admin; + let adminKey; + let site; + let provisioned; + let unprovisioned; + + beforeEach(async () => { + await resetDb(); + app = buildApp(); + admin = await createUser({ admin: true }); + ({ plainKey: adminKey } = await createApiKey(admin)); + site = await Site.create({ name: 's', internalDomain: 'ex.test' }); + const node = await Node.create({ siteId: site.id, name: 'n', nodeType: 'dummy' }); + provisioned = await Container.create({ + hostname: 'prov', + username: admin.uid, + nodeId: node.id, + siteId: site.id, + containerId: '123', // has a provider id → provisioned + }); + unprovisioned = await Container.create({ + hostname: 'unprov', + username: admin.uid, + nodeId: node.id, + siteId: site.id, + }); + }); + + afterAll(async () => { + await closeDb(); + }); + + function put(containerId, body) { + return request(app) + .put(`/api/v1/sites/${site.id}/containers/${containerId}`) + .set(...bearer(adminKey)) + .send(body); + } + + test('attaching a volume on a provisioned container enqueues a reconfigure job (no restart flag)', async () => { + const res = await put(provisioned.id, { + volumes: [{ name: 'data', mountPath: '/mnt/data', mode: 'rw' }], + }); + expect(res.status).toBe(200); + expect(res.body.data.jobId).toBeTruthy(); + + const rows = await Volume.findAll({ where: { containerId: provisioned.id } }); + expect(rows).toHaveLength(1); + expect(rows[0].status).toBe('pending'); + + const jobs = await Job.findAll({ where: { status: 'pending' } }); + expect(jobs.some((j) => j.command.includes('reconfigure-container.js'))).toBe(true); + }); + + test('attaching on an unprovisioned container persists rows without a job', async () => { + const res = await put(unprovisioned.id, { + volumes: [{ name: 'data', mountPath: '/mnt/data', mode: 'rw' }], + }); + expect(res.status).toBe(200); + expect(res.body.data.jobId).toBeNull(); + const rows = await Volume.findAll({ where: { containerId: unprovisioned.id } }); + expect(rows).toHaveLength(1); + }); + + test('detaching removes the row and enqueues a reconfigure job', async () => { + const v = await Volume.create({ + containerId: provisioned.id, + name: 'data', + hostPath: '/v/prov/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'ready', + }); + const res = await put(provisioned.id, { volumes: [{ id: v.id, detach: true }] }); + expect(res.status).toBe(200); + expect(res.body.data.jobId).toBeTruthy(); + expect(await Volume.findByPk(v.id)).toBeNull(); + }); + + test('rejects an attach that collides with an existing volume mount', async () => { + await Volume.create({ + containerId: provisioned.id, + name: 'existing', + hostPath: '/v/prov/existing', + mountPath: '/mnt/data', + mode: 'rw', + status: 'ready', + }); + const res = await put(provisioned.id, { + volumes: [{ name: 'data', mountPath: '/mnt/data', mode: 'rw' }], + }); + expect(res.status).toBe(409); + }); + + test('rejects the reserved quick_and_dirty mount path', async () => { + const res = await put(provisioned.id, { + volumes: [{ name: 'data', mountPath: '/mnt/quick_and_dirty', mode: 'rw' }], + }); + expect(res.status).toBe(400); + }); + + test('a volumes+restart update does not clobber saved env/entrypoint', async () => { + // Container has saved env + entrypoint; a request that only mutates volumes + // (with restart) and omits environmentVars/entrypoint must preserve them. + await provisioned.update({ + environmentVars: JSON.stringify({ FOO: 'bar' }), + entrypoint: '/sbin/init', + }); + const res = await put(provisioned.id, { + restart: true, + volumes: [{ name: 'data', mountPath: '/mnt/data', mode: 'rw' }], + }); + expect(res.status).toBe(200); + + await provisioned.reload(); + expect(JSON.parse(provisioned.environmentVars)).toEqual({ FOO: 'bar' }); + expect(provisioned.entrypoint).toBe('/sbin/init'); + }); + + test('an explicit environmentVars/entrypoint clear is still honored', async () => { + await provisioned.update({ + environmentVars: JSON.stringify({ FOO: 'bar' }), + entrypoint: '/sbin/init', + }); + const res = await put(provisioned.id, { environmentVars: [], entrypoint: '' }); + expect(res.status).toBe(200); + await provisioned.reload(); + expect(provisioned.environmentVars).toBeNull(); + expect(provisioned.entrypoint).toBeNull(); + }); + + test('rejects a malformed detach id like "12garbage"', async () => { + const v = await Volume.create({ + containerId: provisioned.id, + name: 'data', + hostPath: '/v/prov/data', + mountPath: '/mnt/data', + mode: 'rw', + status: 'ready', + }); + const res = await put(provisioned.id, { + volumes: [{ id: `${v.id}garbage`, detach: true }], + }); + expect(res.status).toBe(400); + // The real volume must not have been detached. + expect(await Volume.findByPk(v.id)).not.toBeNull(); + }); + + test('create without a volumes field succeeds and creates no volume rows', async () => { + const res = await request(app) + .post(`/api/v1/sites/${site.id}/containers`) + .set(...bearer(adminKey)) + .send({ hostname: 'novols', template: 'docker.io/library/nginx:latest' }); + expect(res.status).toBe(201); + expect(await Volume.count({ where: { containerId: res.body.data.containerId } })).toBe(0); + }); + + test('create with volumes persists them as pending', async () => { + const res = await request(app) + .post(`/api/v1/sites/${site.id}/containers`) + .set(...bearer(adminKey)) + .send({ + hostname: 'withvols', + template: 'docker.io/library/nginx:latest', + volumes: [{ name: 'data', mountPath: '/mnt/data', mode: 'rw' }], + }); + expect(res.status).toBe(201); + const rows = await Volume.findAll({ where: { containerId: res.body.data.containerId } }); + expect(rows.map((v) => [v.name, v.status])).toEqual([['data', 'pending']]); + }); +}); diff --git a/create-a-container/routers/api/v1/__tests__/nodes.volume-warnings.test.js b/create-a-container/routers/api/v1/__tests__/nodes.volume-warnings.test.js new file mode 100644 index 00000000..bf56dfef --- /dev/null +++ b/create-a-container/routers/api/v1/__tests__/nodes.volume-warnings.test.js @@ -0,0 +1,93 @@ +/** + * Node volume-storage shared-storage warnings (issue #421 (f)): saving a node + * warns (does not block) when the volume storage is not path-backed, not + * shared, or not active on every cluster node. Advisory only. + */ + +const { volumeStorageWarnings } = require('../nodes'); + +function fakeNode(overrides = {}) { + return { + name: 'pve1', + nodeType: 'proxmox', + volumeStorage: 'cephfs', + imageStorage: 'local', + hasApiAccess: () => true, + api: async () => overrides.client, + ...overrides, + }; +} + +describe('volumeStorageWarnings', () => { + test('no warning for shared path-backed storage present on all nodes', async () => { + const client = { + async storageConfig() { + return { storage: 'cephfs', type: 'cephfs', path: '/mnt/pve/cephfs', shared: 1 }; + }, + async clusterResources() { + return [ + { storage: 'cephfs', node: 'pve1', status: 'available' }, + { storage: 'cephfs', node: 'pve2', status: 'available' }, + ]; + }, + async nodes() { + return [{ node: 'pve1' }, { node: 'pve2' }]; + }, + }; + const warnings = await volumeStorageWarnings(fakeNode({ client })); + expect(warnings).toEqual([]); + }); + + test('warns when storage is not shared', async () => { + const client = { + async storageConfig() { + return { storage: 'local', type: 'dir', path: '/var/lib/vz', shared: 0 }; + }, + async clusterResources() { + return [{ storage: 'local', node: 'pve1', status: 'available' }]; + }, + async nodes() { + return [{ node: 'pve1' }]; + }, + }; + const warnings = await volumeStorageWarnings(fakeNode({ client, volumeStorage: 'local' })); + expect(warnings.some((w) => /not marked shared/.test(w))).toBe(true); + }); + + test('warns when storage has no host path (block storage)', async () => { + const client = { + async storageConfig() { + return { storage: 'local-lvm', type: 'lvmthin', shared: 0 }; + }, + async clusterResources() { + return []; + }, + async nodes() { + return [{ node: 'pve1' }]; + }, + }; + const warnings = await volumeStorageWarnings(fakeNode({ client, volumeStorage: 'local-lvm' })); + expect(warnings.some((w) => /no host path/.test(w))).toBe(true); + }); + + test('warns when storage is missing on some cluster nodes', async () => { + const client = { + async storageConfig() { + return { storage: 'nfs1', type: 'nfs', path: '/mnt/nfs', shared: 1 }; + }, + async clusterResources() { + return [{ storage: 'nfs1', node: 'pve1', status: 'available' }]; + }, + async nodes() { + return [{ node: 'pve1' }, { node: 'pve2' }]; + }, + }; + const warnings = await volumeStorageWarnings(fakeNode({ client, volumeStorage: 'nfs1' })); + expect(warnings.some((w) => /not present\/active on every node/.test(w))).toBe(true); + }); + + test('docker/dummy nodes are skipped', async () => { + expect(await volumeStorageWarnings(fakeNode({ nodeType: 'docker' }))).toEqual([]); + expect(await volumeStorageWarnings(fakeNode({ hasApiAccess: () => false }))).toEqual([]); + }); +}); diff --git a/create-a-container/routers/api/v1/__tests__/normalize-volume-attach.test.js b/create-a-container/routers/api/v1/__tests__/normalize-volume-attach.test.js new file mode 100644 index 00000000..1eea52c3 --- /dev/null +++ b/create-a-container/routers/api/v1/__tests__/normalize-volume-attach.test.js @@ -0,0 +1,78 @@ +/** + * normalizeVolumeAttach (issue #421): validates volume-attach entries, rejects + * provider-unsafe mount paths, and rejects the reserved quick_and_dirty + * name/mount point (compared canonically) so a user attach can't collide with a + * backfilled legacy row. + */ + +const { normalizeVolumeAttach } = require('../containers'); + +describe('normalizeVolumeAttach', () => { + test('accepts a valid rw volume', () => { + expect(normalizeVolumeAttach({ name: 'data', mountPath: '/mnt/data', mode: 'rw' })).toEqual({ + name: 'data', + mountPath: '/mnt/data', + mode: 'rw', + }); + }); + + test('canonicalizes the mount path (collapses slashes, strips trailing)', () => { + expect(normalizeVolumeAttach({ name: 'data', mountPath: '/mnt//data/', mode: 'rw' })).toEqual({ + name: 'data', + mountPath: '/mnt/data', + mode: 'rw', + }); + }); + + test('rejects a traversal name', () => { + expect(() => normalizeVolumeAttach({ name: '../x', mountPath: '/mnt/x', mode: 'rw' })).toThrow( + /safe path segment/, + ); + }); + + test('rejects a relative mount path', () => { + expect(() => normalizeVolumeAttach({ name: 'ok', mountPath: 'rel', mode: 'rw' })).toThrow( + /absolute path/, + ); + }); + + test.each([ + ['comma delimiter', '/mnt/a,ro=0'], + ['colon delimiter', '/mnt/a:b'], + ['backslash', '/mnt/a\\b'], + ['whitespace', '/mnt/a b'], + ['newline', '/mnt/a\nb'], + ['NUL', '/mnt/a\u0000b'], + ['traversal segment', '/mnt/../etc'], + ])('rejects a mount path with %s', (_label, mountPath) => { + expect(() => normalizeVolumeAttach({ name: 'ok', mountPath, mode: 'rw' })).toThrow( + /absolute path|delimiters|segments/, + ); + }); + + test('rejects an invalid mode', () => { + expect(() => normalizeVolumeAttach({ name: 'ok', mountPath: '/mnt/ok', mode: 'x' })).toThrow( + /mode must be/, + ); + }); + + test('rejects the reserved built-in name', () => { + expect(() => + normalizeVolumeAttach({ name: 'quick_and_dirty', mountPath: '/mnt/other', mode: 'rw' }), + ).toThrow(/reserved/); + }); + + test('rejects the reserved built-in mount path', () => { + expect(() => + normalizeVolumeAttach({ name: 'other', mountPath: '/mnt/quick_and_dirty', mode: 'rw' }), + ).toThrow(/reserved/); + }); + + test('rejects equivalent spellings of the reserved mount path', () => { + for (const mountPath of ['/mnt/quick_and_dirty/', '/mnt//quick_and_dirty']) { + expect(() => normalizeVolumeAttach({ name: 'other', mountPath, mode: 'rw' })).toThrow( + /reserved/, + ); + } + }); +}); diff --git a/create-a-container/routers/api/v1/containers.js b/create-a-container/routers/api/v1/containers.js index a329d57f..97b243d0 100644 --- a/create-a-container/routers/api/v1/containers.js +++ b/create-a-container/routers/api/v1/containers.js @@ -16,6 +16,7 @@ const { ExternalDomain, Job, Setting, + Volume, Sequelize, sequelize, } = require('../../../models'); @@ -52,6 +53,76 @@ function serializeUserEnvVars(environmentVars) { return Object.keys(normalized).length > 0 ? JSON.stringify(normalized) : null; } +/** + * Validate a single volume-attach entry from a create/update request. Returns + * a normalized `{ name, mountPath, mode }` or throws ApiError. `hostPath` is + * intentionally NOT accepted from clients — it is derived server-side from the + * node storage's configured path by the create job (issue #421). + * @param {*} v + * @returns {{name: string, mountPath: string, mode: 'ro'|'rw'}} + */ +function normalizeVolumeAttach(v) { + if (!v || typeof v !== 'object') { + throw new ApiError(400, 'invalid_volume', 'Each volume must be an object'); + } + const name = typeof v.name === 'string' ? v.name.trim() : ''; + if (!Volume.isValidName(name)) { + throw new ApiError( + 400, + 'invalid_volume', + 'Volume name must be a safe path segment (letters, digits, dot, dash, underscore; no traversal or separators)', + ); + } + const rawMountPath = typeof v.mountPath === 'string' ? v.mountPath.trim() : ''; + // Reject provider delimiters/control chars/traversal, then compare in + // canonical form so equivalent spellings can't corrupt config or bypass the + // reserved-path check below. + if (!Volume.isValidMountPath(rawMountPath)) { + throw new ApiError( + 400, + 'invalid_volume', + "Volume mountPath must be an absolute path with no delimiters (',' ':'), backslashes, whitespace, control characters, or '.'/'..' segments", + ); + } + const mountPath = Volume.canonicalizeMountPath(rawMountPath); + // The name and mount point of the retired quick_and_dirty mount are reserved: + // a user attach at either would collide with the backfilled built-in row that + // records the legacy mount on pre-existing containers. Compared canonically. + if (name === Volume.QUICK_AND_DIRTY_NAME) { + throw new ApiError(400, 'invalid_volume', `Volume name '${name}' is reserved`); + } + if (mountPath === Volume.canonicalizeMountPath(Volume.QUICK_AND_DIRTY_MOUNT)) { + throw new ApiError( + 400, + 'invalid_volume', + `Volume mountPath '${mountPath}' is reserved`, + ); + } + const mode = v.mode === 'ro' ? 'ro' : v.mode === 'rw' ? 'rw' : null; + if (!mode) { + throw new ApiError(400, 'invalid_volume', "Volume mode must be 'ro' or 'rw'"); + } + return { name, mountPath, mode }; +} + +/** + * Serialize a Volume row for API responses. + * @param {object} v + */ +function serializeVolume(v) { + return { + id: v.id, + name: v.name, + mountPath: v.mountPath, + mode: v.mode, + scope: v.scope, + builtin: !!v.builtin, + status: v.status, + statusMessage: v.statusMessage ?? null, + appliedAt: v.appliedAt ?? null, + }; +} + function normalizeDockerRef(ref) { if (ref.startsWith('http://') || ref.startsWith('https://') || ref.startsWith('git@')) return ref; let tag = 'latest'; @@ -148,6 +219,7 @@ function serializeContainer(c, site, status) { lastAccessedAt, nodeName: c.node ? c.node.name : null, nodeApiUrl: c.node ? c.node.apiUrl : null, + volumes: (c.volumes || []).map(serializeVolume), services: services.map((s) => ({ id: s.id, type: s.type, @@ -195,6 +267,8 @@ const CONTAINER_INCLUDE = [ { association: 'creationJob' }, // Users the container is shared with, for the serializer's `collaborators`. { association: 'collaborators' }, + // Volumes (bind mounts) for the serializer's `volumes`. + { association: 'volumes' }, ]; /** @@ -437,6 +511,7 @@ router.post( entrypoint, nvidiaRequested, collaborators, + volumes, username: bodyUsername, } = req.body || {}; @@ -467,6 +542,26 @@ router.post( // NVIDIA and admin-defined system defaults are merged in at configure-time. const envVarsJson = serializeUserEnvVars(environmentVars); + // Validate volume attachments up front so a bad entry fails before any + // node selection or row creation. `hostPath` is derived by the create job. + const requestedVolumes = volumes ?? []; + if (!Array.isArray(requestedVolumes)) { + throw new ApiError(400, 'invalid_request', 'volumes must be an array'); + } + const normalizedVolumes = requestedVolumes.map(normalizeVolumeAttach); + const seenMounts = new Set(); + const seenNames = new Set(); + for (const v of normalizedVolumes) { + if (seenMounts.has(v.mountPath)) { + throw new ApiError(400, 'invalid_volume', `Duplicate volume mountPath: ${v.mountPath}`); + } + if (seenNames.has(v.name)) { + throw new ApiError(400, 'invalid_volume', `Duplicate volume name: ${v.name}`); + } + seenMounts.add(v.mountPath); + seenNames.add(v.name); + } + const imageRef = template === 'custom' ? customTemplate?.trim() : template; if (!imageRef) throw new ApiError(400, 'invalid_request', 'template is required'); const templateName = normalizeDockerRef(imageRef); @@ -535,6 +630,28 @@ router.post( throw err; } + // Persist requested volumes as `pending`. The create job derives each + // hostPath from the node storage and blocks on Volume.status until the + // site agent has created the directory (issue #421). hostPath is set to a + // placeholder here and backfilled by the job. + if (normalizedVolumes.length > 0) { + await Volume.bulkCreate( + normalizedVolumes.map((v) => ({ + containerId: container.id, + name: v.name, + // hostPath is derived by the create job from the node storage's + // configured path; left null here. + hostPath: null, + mountPath: v.mountPath, + mode: v.mode, + scope: 'container', + builtin: false, + status: 'pending', + })), + { transaction: t }, + ); + } + if (services && typeof services === 'object') { for (const key in services) { const svc = services[key]; @@ -620,8 +737,72 @@ router.put( ); const { services, environmentVars, entrypoint, username: bodyUsername } = req.body || {}; + const volumesInput = req.body?.volumes; const forceRestart = req.body?.restart === true || req.body?.restart === 'true'; - const isRestartOnly = forceRestart && !services && !environmentVars && entrypoint === undefined; + // Whether the request actually carries an env/entrypoint edit. Presence of + // the key — not just a truthy value — is what distinguishes "omitted (leave + // as-is)" from "explicitly cleared". This is the single source of truth for + // deciding whether to touch these fields, so a request that only mutates + // volumes (with or without `restart: true`) never clobbers saved + // env/entrypoint. `environmentVars: []`/`null` and `entrypoint: null`/'' are + // still honored as explicit clears because the key is present. + const body = req.body || {}; + const envProvided = Object.prototype.hasOwnProperty.call(body, 'environmentVars'); + const entrypointProvided = Object.prototype.hasOwnProperty.call(body, 'entrypoint'); + + // Validate volume attach/detach entries up front. Attach entries are + // `{ name, mountPath, mode }`; detach entries are `{ id, detach: true }`. + let volumeAttaches = []; + let volumeDetachIds = []; + if (volumesInput !== undefined) { + if (!Array.isArray(volumesInput)) { + throw new ApiError(400, 'invalid_request', 'volumes must be an array'); + } + for (const v of volumesInput) { + if (v && (v.detach === true || v.detach === 'true')) { + // Strict parse: `Number('12garbage')` is NaN, unlike parseInt which + // would accept the numeric prefix and could detach the wrong volume. + const id = typeof v.id === 'number' ? v.id : Number(v.id); + if (!Number.isInteger(id) || id <= 0) { + throw new ApiError(400, 'invalid_volume', 'Detach requires a numeric volume id'); + } + volumeDetachIds.push(id); + } else { + volumeAttaches.push(normalizeVolumeAttach(v)); + } + } + // Reject duplicate names/mount points within the request (both normalized + // to their canonical form) so a single update can't create two rows that + // violate the (containerId, name)/(containerId, mountPath) unique indexes. + const seenNames = new Set(); + const seenMounts = new Set(); + for (const a of volumeAttaches) { + if (seenNames.has(a.name)) { + throw new ApiError(400, 'invalid_volume', `Duplicate volume name: ${a.name}`); + } + if (seenMounts.has(a.mountPath)) { + throw new ApiError(400, 'invalid_volume', `Duplicate volume mountPath: ${a.mountPath}`); + } + seenNames.add(a.name); + seenMounts.add(a.mountPath); + } + // Reject an attach that collides with an existing volume the same request + // isn't detaching (canonical mount comparison catches equivalent paths). + if (volumeAttaches.length > 0) { + const existing = await Volume.findAll({ where: { containerId: container.id } }); + const detachSet = new Set(volumeDetachIds); + for (const e of existing) { + if (detachSet.has(e.id)) continue; + const eMount = Volume.canonicalizeMountPath(e.mountPath); + if (seenNames.has(e.name)) { + throw new ApiError(409, 'volume_exists', `A volume named '${e.name}' is already attached`); + } + if (eMount && seenMounts.has(eMount)) { + throw new ApiError(409, 'volume_exists', `A volume is already mounted at '${eMount}'`); + } + } + } + } // Admins may reassign the container to another user by passing `username`. // Non-admins may not pass a different username — that is a 403. @@ -636,24 +817,33 @@ router.put( newOwnerUsername = bodyUsername.trim(); } + // Only recompute env/entrypoint when the request actually carried the key; + // an omitted field is preserved (never cleared), so a volumes-only or + // restart-only request leaves them untouched. let envVarsJson = container.environmentVars; - if (!isRestartOnly && Array.isArray(environmentVars)) { - envVarsJson = serializeUserEnvVars(environmentVars); - } else if (!isRestartOnly && !environmentVars) { - envVarsJson = null; + if (envProvided) { + envVarsJson = Array.isArray(environmentVars) ? serializeUserEnvVars(environmentVars) : null; } - const newEntrypoint = isRestartOnly - ? container.entrypoint - : entrypoint && entrypoint.trim() + const newEntrypoint = entrypointProvided + ? entrypoint && entrypoint.trim() ? entrypoint.trim() - : null; + : null + : container.entrypoint; const ownerChanged = newOwnerUsername !== null && newOwnerUsername !== container.username; - const envChanged = !isRestartOnly && container.environmentVars !== envVarsJson; - const entrypointChanged = !isRestartOnly && container.entrypoint !== newEntrypoint; + const envChanged = envProvided && container.environmentVars !== envVarsJson; + const entrypointChanged = entrypointProvided && container.entrypoint !== newEntrypoint; + const volumesChanged = volumeAttaches.length > 0 || volumeDetachIds.length > 0; // Never restart implicitly (issue #449): a restart job is enqueued only // when the caller explicitly asks for one. Saved env/entrypoint changes // are applied by reconfigure-container.js on the next restart. - const needsRestart = forceRestart; + // + // Volume changes are different: the migration reconciler is one-time, so an + // attach/detach on an already-provisioned container would otherwise only + // write DB rows and never derive host paths, wait on the readiness barrier, + // or set mpN. So a volume mutation on a provisioned container ALWAYS enqueues + // a reconfigure job (which does exactly that) even without `restart: true`. + const needsReconfigureJob = + !!container.containerId && (forceRestart || volumesChanged); let restartJob = null; const dnsWarnings = []; @@ -667,7 +857,7 @@ router.put( { transaction: t }, ); } - if (needsRestart && container.containerId) { + if (needsReconfigureJob) { restartJob = await Job.create( { command: `node bin/reconfigure-container.js --container-id=${container.id}`, @@ -764,6 +954,33 @@ router.put( dnsWarnings.push(...(await manageDnsRecords(newHttp, site, 'create'))); } } + + // Volume attach/detach. New volumes are persisted `pending` (hostPath + // derived, directory created, and mpN set on the next reconcile); detach + // removes the row (retain-on-delete of the host directory is a node-side + // concern — the row going away just stops future mounts). Built-in + // volumes can't be detached through this path. + if (volumeDetachIds.length > 0) { + await Volume.destroy({ + where: { id: volumeDetachIds, containerId: container.id, builtin: false }, + transaction: t, + }); + } + if (volumeAttaches.length > 0) { + await Volume.bulkCreate( + volumeAttaches.map((v) => ({ + containerId: container.id, + name: v.name, + hostPath: null, + mountPath: v.mountPath, + mode: v.mode, + scope: 'container', + builtin: false, + status: 'pending', + })), + { transaction: t }, + ); + } }); // Keep the Proxmox tag in sync with the owner — create-container.js tags @@ -781,17 +998,24 @@ router.put( } } + // env/entrypoint changes still take effect only on the next restart when no + // job was enqueued. Volume changes on a provisioned container always enqueue + // a job above, so they are never "pending"; on an unprovisioned container + // they apply at create time. const pendingRestart = !restartJob && (envChanged || entrypointChanged); + const message = restartJob + ? volumesChanged && !forceRestart + ? 'Applying volume changes — the container will restart to mount them' + : 'Container is restarting' + : pendingRestart + ? 'Container updated — changes take effect on the next restart' + : 'Container updated'; return ok(res, { containerId: container.id, jobId: restartJob ? restartJob.id : null, dnsWarnings, pendingRestart, - message: restartJob - ? 'Container is restarting' - : pendingRestart - ? 'Container updated — changes take effect on the next restart' - : 'Container updated', + message, }); }), ); @@ -851,7 +1075,11 @@ router.delete( } } // Sharing grants are removed by the database via the containerId foreign - // key's ON DELETE CASCADE. + // key's ON DELETE CASCADE. Volume rows cascade too, but their host + // directories are RETAINED on the node — the agent only ever creates + // directories, never removes them, so a later create on the same hostname + // reattaches the existing data (issue #421 (g)). Reclamation of orphaned + // directories is out of scope (no automatic GC). await container.destroy(); // Remove the VM from NetBox if the integration is configured @@ -932,3 +1160,5 @@ router.delete( module.exports = router; // Exported for unit tests (containers.serialize.test.js). module.exports.serializeContainer = serializeContainer; +module.exports.serializeVolume = serializeVolume; +module.exports.normalizeVolumeAttach = normalizeVolumeAttach; diff --git a/create-a-container/routers/api/v1/nodes.js b/create-a-container/routers/api/v1/nodes.js index b50ad716..816e9086 100644 --- a/create-a-container/routers/api/v1/nodes.js +++ b/create-a-container/routers/api/v1/nodes.js @@ -41,6 +41,84 @@ function normalizeNodeType(nodeType) { return nodeType || 'proxmox'; } +/** + * Warn (do not fail) when a node's volume storage is not suitable for durable, + * cross-node persistent volumes (issue #421 (f)): the volumes root must live on + * storage that is `shared=1` and active on every cluster node, or the site + * agent's per-node directory won't exist where a migrated container lands. + * + * Single-node sites are legitimately fine, so this is advisory only. Best + * effort: any Proxmox query failure yields no warning rather than blocking the + * save. Docker/dummy nodes are skipped. + * + * @param {object} node - Saved Node instance + * @returns {Promise} Human-readable warnings (empty when all good) + */ +async function volumeStorageWarnings(node) { + if (node.nodeType !== 'proxmox' || !node.hasApiAccess()) return []; + const storageName = node.volumeStorage || node.imageStorage || 'local'; + const warnings = []; + try { + const client = await node.api(); + + // Storage config exposes type + shared flag; a path is required for a bind + // directory (block storages like lvm/zfspool can't host one). + let cfg = null; + if (typeof client.storageConfig === 'function') { + try { + cfg = await client.storageConfig(storageName); + } catch (err) { + console.error(`Could not read storage config for ${storageName}:`, err.message); + } + } + if (cfg) { + const shared = cfg.shared === 1 || cfg.shared === '1' || cfg.shared === true; + if (!cfg.path) { + warnings.push( + `Volume storage "${storageName}" (type ${cfg.type || 'unknown'}) has no host path; ` + + 'persistent volumes require a path-backed storage (dir/nfs/cephfs). Move the volumes root to shared storage.', + ); + } else if (!shared) { + warnings.push( + `Volume storage "${storageName}" is not marked shared across the cluster; ` + + 'persistent volume data is not guaranteed to follow containers across nodes. ' + + 'Move the volumes root to a path-backed shared filesystem (CephFS or NFS) available on every node.', + ); + } + } + + // Cross-check per-node presence: the storage should be active on every node. + try { + const [resources, clusterNodes] = await Promise.all([ + client.clusterResources('storage'), + client.nodes(), + ]); + const nodeNames = new Set((clusterNodes || []).map((n) => n.node).filter(Boolean)); + if (nodeNames.size > 1) { + const presentOn = new Set( + (resources || []) + .filter((r) => r.storage === storageName && (r.status === 'available' || r.status === undefined)) + .map((r) => r.node) + .filter(Boolean), + ); + const missing = [...nodeNames].filter((n) => !presentOn.has(n)); + if (missing.length > 0) { + warnings.push( + `Volume storage "${storageName}" is not present/active on every node ` + + `(missing on: ${missing.join(', ')}). Persistent volumes will not be creatable/durable ` + + 'where a container lands on those nodes.', + ); + } + } + } catch (err) { + console.error(`Could not verify cluster storage presence for ${storageName}:`, err.message); + } + } catch (err) { + console.error(`Volume storage validation skipped for node ${node.name}:`, err.message); + } + return warnings; +} + function validateNodeInput({ nodeType, apiUrl }) { const type = normalizeNodeType(nodeType); @@ -192,7 +270,8 @@ router.post( nvidiaAvailable: nvidiaAvailable === true || nvidiaAvailable === 'true', siteId: site.id, }); - return created(res, serialize(node)); + const warnings = await volumeStorageWarnings(node); + return created(res, { ...serialize(node), warnings }); }), ); @@ -226,7 +305,8 @@ router.put( }; if (secret && secret.trim() !== '') update.secret = secret; await node.update(update); - return ok(res, serialize(node)); + const warnings = await volumeStorageWarnings(node); + return ok(res, { ...serialize(node), warnings }); }), ); @@ -364,3 +444,5 @@ router.post( ); module.exports = router; +// Exported for unit tests. +module.exports.volumeStorageWarnings = volumeStorageWarnings; From 41a5c623edacfcee249d3cf56d908fb9f14e78f6 Mon Sep 17 00:00:00 2001 From: Robert Gingras Date: Tue, 29 Sep 2026 11:42:10 -0400 Subject: [PATCH 5/5] volumes: document volumes and wire them into the dev stack (#421) Docs: - New admins/core-concepts/volumes.md: how volumes are provisioned and attached, the site/owner-scoped host path and same-owner reattach semantics, the path-backed shared-storage requirement (CephFS/NFS; block storage can't host volumes), that volume data is NOT backed up by the platform (bind mounts aren't in vzdump), retain-on-delete, and Docker behavior. Linked from the core-concepts index and nav. - deploying-agents.md: one agent per site (not per node); the one-time volumes setup (pre-create the root 100000:100000 on shared storage and bind-mount it into the agent at the same path); the agent's mount check; custom lxc.idmap unsupported for volumes. - containers.md, nodes.md, developers/agent.md, developers/database-schema.md updated for volumes and the check-in contract. Dev stack: create-manager.sh pre-creates /var/lib/vz/volumes (the derived root for the dev node's `local` storage) owned 100000:100000 and bind-mounts it into the Manager CT, whose embedded agent provisions volumes. --- images/proxmox-ve/create-manager.sh | 25 +++++- .../docs/admins/core-concepts/containers.md | 9 ++ .../docs/admins/core-concepts/index.md | 1 + .../docs/admins/core-concepts/nodes.md | 9 +- .../docs/admins/core-concepts/volumes.md | 83 +++++++++++++++++++ .../docs/admins/deploying-agents.md | 66 ++++++++++++++- .../docs/developers/agent.md | 32 ++++++- .../docs/developers/database-schema.md | 18 ++++ mie-opensource-landing/zensical.toml | 1 + 9 files changed, 239 insertions(+), 5 deletions(-) create mode 100644 mie-opensource-landing/docs/admins/core-concepts/volumes.md diff --git a/images/proxmox-ve/create-manager.sh b/images/proxmox-ve/create-manager.sh index cf1bfc10..32d2afd7 100644 --- a/images/proxmox-ve/create-manager.sh +++ b/images/proxmox-ve/create-manager.sh @@ -76,13 +76,34 @@ pct push 100 \ /opt/opensource-server/images/proxmox-ve/99-container-creator-dev.conf \ /etc/systemd/system/container-creator.service.d/99-container-creator-dev.conf +# The Manager runs the embedded site agent, which provisions persistent volume +# directories (issue #421) under the node volume storage's configured path. For +# the dev node that storage is `local` (a `dir` at /var/lib/vz), so the derived +# volumes root is /var/lib/vz/volumes. Pre-create it owned by the unprivileged-CT +# id-mapped root (host UID/GID 100000) so read-write volumes are writable from +# inside consuming unprivileged containers. This Manager CT is UNPRIVILEGED +# (`pct create` defaults --unprivileged to 1), so the embedded agent runs as +# guest root = host UID 100000 and its `mkdir` already yields 100000-owned +# subdirectories — matching this root. The agent additionally chowns each dir to +# 100000 best-effort; inside the unprivileged guest that chown is a tolerated +# no-op (EINVAL, since 100000 is outside the guest's mapped range). This mirrors +# the "Deploying Agents" docs and keeps the compose stack turnkey for volume +# testing. +VOLUMES_ROOT="/var/lib/vz/volumes" +mkdir -p "${VOLUMES_ROOT}" +chown 100000:100000 "${VOLUMES_ROOT}" +chmod 0770 "${VOLUMES_ROOT}" + # Now we can set the entrypoint back to normal so it'll boot up to the # default systemd target. We also use this opportunity to add the directory -# mount. Doing it with the container online or during the create step causes all +# mounts. Doing it with the container online or during the create step causes all # sorts of AppArmor and userns problems due to the nested Proxmox-in-Docker. +# mp1 bind-mounts the volumes root into the agent at the same path, so the host +# path the manager derives resolves identically inside the agent (issue #421). pct shutdown 100 pct set 100 \ - --mp0=/opt/opensource-server,mp=/opt/opensource-server + --mp0=/opt/opensource-server,mp=/opt/opensource-server \ + --mp1="${VOLUMES_ROOT},mp=${VOLUMES_ROOT}" # Remove the temporary emergency entrypoint before the final start so the # Manager CT boots to the default target with networking and services enabled. diff --git a/mie-opensource-landing/docs/admins/core-concepts/containers.md b/mie-opensource-landing/docs/admins/core-concepts/containers.md index 42f37213..e15a9d4c 100644 --- a/mie-opensource-landing/docs/admins/core-concepts/containers.md +++ b/mie-opensource-landing/docs/admins/core-concepts/containers.md @@ -16,6 +16,15 @@ Users in the **ldapusers** group can SSH into any container using their cluster | **Creating** | Being provisioned | | **Failed** | Creation or startup failed | +## Volumes + +Containers can attach persistent **[volumes](volumes.md)** — bind-mount +directories whose data survives delete + recreate on the same hostname. Each +volume has a name, a guest mount path, and a read-only or read-write mode. A +container has only the volumes its creator attaches; nothing is mounted by +default. See [Volumes](volumes.md) for the shared-storage requirement and backup +caveats. + ## Service Exposure Users can expose HTTP services from containers using [external domains](external-domains.md). Services are automatically configured with SSL/TLS certificates, reverse proxy routing, and DNS records. diff --git a/mie-opensource-landing/docs/admins/core-concepts/index.md b/mie-opensource-landing/docs/admins/core-concepts/index.md index 00ca5853..c13d8b76 100644 --- a/mie-opensource-landing/docs/admins/core-concepts/index.md +++ b/mie-opensource-landing/docs/admins/core-concepts/index.md @@ -8,5 +8,6 @@ The cluster is organized: **Sites** → **Nodes** → **Containers** - **[External Domains](external-domains.md)** — Public domains with automatic SSL/TLS - **[Nodes](nodes.md)** — Proxmox VE servers within a site - **[Containers](containers.md)** — LXC instances on nodes ([user guide](../../users/creating-containers/web-gui.md)) +- **[Volumes](volumes.md)** — Persistent bind-mount directories that survive delete + recreate **Setup order:** Users & Groups → Sites → External Domains (optional) → Nodes → Containers diff --git a/mie-opensource-landing/docs/admins/core-concepts/nodes.md b/mie-opensource-landing/docs/admins/core-concepts/nodes.md index fe08fa3a..49a95c1a 100644 --- a/mie-opensource-landing/docs/admins/core-concepts/nodes.md +++ b/mie-opensource-landing/docs/admins/core-concepts/nodes.md @@ -11,7 +11,14 @@ Nodes are Proxmox VE servers within a site that host containers. - **Authentication**: Username/password or API token - **TLS Verification**: Enable/disable certificate validation - **Template Storage**: Proxmox storage for CT template images (`vztmpl` content) -- **Volume Storage**: Proxmox storage for container root filesystems (`rootdir` content) +- **Volume Storage**: Proxmox storage for container root filesystems (`rootdir` content) and persistent [volumes](volumes.md) + +!!! warning "Volume storage should be shared across the cluster" + Persistent [volumes](volumes.md) require their host directories to exist on + whichever node a container lands on. Place the **volume storage** on storage + that is shared across every node (a path-backed shared filesystem such as CephFS or NFS). On save, the manager + warns (but does not block) if the chosen volume storage is not shared or is + not active on every node. Single-node sites are unaffected. ## Adding Nodes diff --git a/mie-opensource-landing/docs/admins/core-concepts/volumes.md b/mie-opensource-landing/docs/admins/core-concepts/volumes.md new file mode 100644 index 00000000..942a8686 --- /dev/null +++ b/mie-opensource-landing/docs/admins/core-concepts/volumes.md @@ -0,0 +1,83 @@ + +# Volumes + +Volumes are persistent bind-mount directories attached to containers. Unlike a +container's root filesystem, volume data **survives delete + recreate** on the +same hostname by the same owner — the host directory is retained when the +container is removed and reattached when that owner creates a container with the +same hostname again. + +Volumes replace the earlier hardcoded shared `quick_and_dirty` read-only mount. +That stopgap is fully removed: no volume is attached to a container unless its +creator explicitly adds one. (Containers that existed before this change keep +their original `quick_and_dirty` mount, recorded in the database for reference; +it is not re-applied and new containers never receive it.) + +## How volumes work + +1. A volume is defined per container with a **name**, a guest **mount path** + (e.g. `/mnt/data`), and a **mode** (`ro` read-only or `rw` read-write). +2. The host directory lives under + `/site-///`, where `` + is derived from the node's **volume storage** actual configured path (not + assumed) plus `/volumes`. The `site-` segment isolates containers + that share a hostname across sites on the same shared storage; the `` + segment (the container owner's username) ensures retained data is only ever + reattached for the same owner. +3. When a container is created (or reconfigured), the site's **agent** creates + the host directory. The agent is a single per-site container with the shared + volumes root bind-mounted in; it chowns each directory to the consuming + containers' id-mapped root (host UID/GID `100000`) best-effort, so read-write + volumes are writable from inside the container. See + [Deploying Agents → Volume storage](../deploying-agents.md#volume-storage-for-persistent-volumes) + for the one-time bind-mount + pre-create setup. +4. The create job **waits** until the directory is ready (the agent reports it + at check-in) before attaching the bind mount — Proxmox rejects a mount whose + host directory does not yet exist. The wait happens **before** the container + is provisioned, so a provisioning failure never leaves a half-created + container behind. + +## Shared storage is required + +!!! warning "Place the volumes root on shared storage" + A single per-site agent creates volume directories on the shared volumes + root that is bind-mounted into it. For a directory to exist wherever a + container is placed or migrated, the **volume storage must be a path-backed + shared filesystem** — CephFS or NFS (`shared=1`) — that every node mounts. + Block storages (Ceph RBD, LVM, ZFS) expose no host directory path and + therefore cannot host volumes. + + On node-local storage (`dir`/`lvm`/`zfspool` with `shared=0`), the directory + will not exist where a container on another node lands and the data will not + follow it. Single-node sites are unaffected. + + When you save a node's configuration, the manager checks the chosen volume + storage against the cluster topology and **warns** (it does not block) if the + storage is not shared or is not active on every node. Move the volumes root + to shared storage to clear the warning. + +## Backup + +!!! danger "Volume data is not backed up by the platform" + Bind-mount contents are **not** included in Proxmox `vzdump`, so container + backups do **not** cover volume data. The platform prescribes no backup + mechanism for volumes. **Operators are responsible for backing up the + underlying shared storage** that hosts the volumes root. + +## Deletion and reclamation + +Deleting a container **retains** its volume directories on the node so a later +container with the same hostname **and the same owner** reattaches the existing +data. A different user who creates a container with a previously used hostname +gets fresh, empty directories — never the previous owner's data. An admin +reassigning a container's owner keeps its existing directories (the path is +fixed when the volume is first provisioned). The platform does +**not** garbage-collect retained/orphaned directories; reclaiming them is a +manual/ops task. + +## Docker nodes + +On Docker nodes, volumes map to Docker bind mounts (`:[:ro]`). +Docker auto-creates the host bind-source directory at container start, so there +is no separate agent step and no shared-storage precondition on a single Docker +host. diff --git a/mie-opensource-landing/docs/admins/deploying-agents.md b/mie-opensource-landing/docs/admins/deploying-agents.md index 8e3e5f71..847d9e54 100644 --- a/mie-opensource-landing/docs/admins/deploying-agents.md +++ b/mie-opensource-landing/docs/admins/deploying-agents.md @@ -1,7 +1,7 @@ # Deploying Agents -An agent container runs nginx, dnsmasq, and [acme.sh](https://github.com/acmesh-official/acme.sh) for a site. Deploy one agent per Proxmox node to handle networking for containers on that node. +An agent container runs nginx, dnsmasq, and [acme.sh](https://github.com/acmesh-official/acme.sh) for a site. Deploy **one agent per site** — a site is one subnet, and the agent is that subnet's single DHCP/DNS and reverse-proxy authority, so running more than one per site on a shared L2 would collide. The agent also provisions persistent [volume](core-concepts/volumes.md) directories for the whole site (see [Volume storage](#volume-storage-for-persistent-volumes) below). Agents are deployed **manually in Proxmox** (not through the manager UI) and should be set up **after** configuring the site in the manager but **before** importing the node. This ensures DNS and reverse proxy services are running when the node comes online. @@ -71,6 +71,70 @@ lxc.environment = API_KEY= | `MANAGER_URL` | Base URL of the manager container (e.g., `http://192.168.1.10:3000`) | | `API_KEY` | API key from an admin account. Used to authenticate check-ins. | +## Volume storage for persistent volumes + +If the site uses persistent [volumes](core-concepts/volumes.md), the agent +creates each volume's host directory on check-in. For that to work, the site's +**volumes root** must be visible and writable inside the agent container, and it +must be on storage shared across every node so a directory the agent creates +exists wherever a container lands. + +The volumes root is `/volumes` — where `` +is the configured **path** of the node's volume storage (e.g. a CephFS/NFS mount +like `/mnt/pve/cephfs`). Do the following once per site, on the Proxmox host that +runs the agent: + +1. **Pre-create the volumes root on the shared storage**, owned by the + unprivileged-container id-mapped root (host UID/GID `100000`), so the agent — + itself an unprivileged CT mapped the same way — can create and own + per-volume subdirectories: + + ```bash + # e.g. /mnt/pve/cephfs/volumes + mkdir -p + chown 100000:100000 + chmod 0770 + ``` + +2. **Bind-mount the volumes root into the agent container at the same path** so + the host path the manager derives resolves identically inside the agent. Add + to `/etc/pve/lxc/.conf`: + + ```ini + mp0: ,mp= + ``` + + For example: `mp0: /mnt/pve/cephfs/volumes,mp=/mnt/pve/cephfs/volumes`. + + The agent checks for this mount: it only creates a volume directory if the + path lies on a mounted filesystem other than its own root. If the volumes + root isn't mounted, the volume is reported as `failed` with a message saying + so, instead of being created inside the agent container where Proxmox can't + see it. + +!!! note "Why the agent — not a per-node host process — creates these" + There is one agent per site, and the shared volumes root is bind-mounted + into it, so this single agent provisions every site volume's directory + (which is why the volumes root must be on shared storage). The agent chowns + each new directory to the consuming containers' id-mapped root (host UID/GID + `100000`) best-effort. In an **unprivileged** agent guest — the norm, since + `pct create` defaults to `--unprivileged 1` (this includes the embedded + Manager agent) — the agent's root maps to host `100000`, so `mkdir` already + yields the right owner and the chown is a tolerated no-op (`EINVAL`/`EPERM`). + In a **privileged** agent guest (only if you deliberately create it with + `--unprivileged 0`) the agent is host root, so the chown is what makes the + directory writable by the unprivileged consumer. + +!!! warning "Custom id-maps are not supported for volumes" + Volume ownership assumes the **default** Proxmox unprivileged-CT id-map, + which maps guest UID/GID 0 to host `100000`. The manager always advertises + `100000` as the owner and the agent chowns each volume directory to it, so + read-write volumes are writable by the consuming containers' mapped root. + Sites that override this with a custom `lxc.idmap` (a different base) are + **not supported** for persistent volumes: the per-volume directories would + still be owned by `100000` and would not be writable inside those + containers. Keep containers that use volumes on the default id-map. + ## 4. Start and Verify ```bash diff --git a/mie-opensource-landing/docs/developers/agent.md b/mie-opensource-landing/docs/developers/agent.md index e00e7012..6b350a03 100644 --- a/mie-opensource-landing/docs/developers/agent.md +++ b/mie-opensource-landing/docs/developers/agent.md @@ -33,7 +33,8 @@ sequenceDiagram Agent->>Timer: exit ``` -The check-in body carries system info and per-service status: +The check-in body carries system info, per-service status, and — when the agent +provisioned volume directories this pass — per-volume results: ```json { @@ -44,12 +45,41 @@ The check-in body carries system info and per-service status: "services": { "nginx": { "state": "active", "lastApply": "success" }, "dnsmasq": { "state": "active", "lastApply": "success" } + }, + "volumes": { + "42": { "applied": true }, + "43": { "applied": false, "message": "mkdir: permission denied" } } } ``` The manager records every check-in in the `Agents` table (shown on the web client's `/agents` page) and responds with the site's config snapshot as JSON. A strong `ETag` covers the snapshot; the agent stores it in `/var/lib/opensource-agent/state.json` and sends it back via `If-None-Match`, so unchanged configs cost a single `304` round trip. +## Volumes + +The config snapshot includes, at the **site** level, the volume directories the +agent must ensure exist (`site.volumes[]` = `{ id, hostPath, mode, uid, gid }`). +There is one agent per site, and the shared volumes root is bind-mounted into the +agent container, so this single agent provisions volumes for every node in the +site. Each pass the agent first checks, via `/proc/self/mountinfo`, that the +path lies on a mount other than its own root filesystem (otherwise the volumes +root isn't bind-mounted and the volume is reported `failed` rather than created +inside the agent). It then `mkdir -p`s the directory, `chown`s it to the +supplied `uid`/`gid` (the consuming containers' id-mapped root, `100000`) +best-effort, and `chmod`s it per mode. The chown is a tolerated no-op +(`EINVAL`/`EPERM`) in an unprivileged agent guest — the norm, since `pct create` +defaults to `--unprivileged 1`, where the id-map already yields the right owner +and `100000` is outside the guest's mapped range — and the actual fix in a +privileged agent guest (deliberately created with `--unprivileged 0`), where the +agent is host root and mkdir would otherwise create a root-owned directory. Per-volume +results are reported back keyed by volume id via the `volumes` field above; the +manager writes them into `Volume.status` (`ready` / `failed`). The container +create job blocks on `Volume.status = 'ready'` before attaching the Proxmox +`mpN` bind mount. The agent only ever creates directories — it never removes +them, so volume data is retained across container +delete + recreate. See [Volumes](../admins/core-concepts/volumes.md) and +[Deploying Agents → Volume storage](../admins/deploying-agents.md#volume-storage-for-persistent-volumes). + ## Environment Variables Read from the process environment (systemd loads `/etc/environment`). Set via container runtime (Docker `ENV`, Proxmox LXC config) — the base image's `environment.sh` service propagates them on boot. diff --git a/mie-opensource-landing/docs/developers/database-schema.md b/mie-opensource-landing/docs/developers/database-schema.md index 149fec58..799d4932 100644 --- a/mie-opensource-landing/docs/developers/database-schema.md +++ b/mie-opensource-landing/docs/developers/database-schema.md @@ -14,6 +14,7 @@ erDiagram Sites ||--o{ Agents : "checked in by" Nodes ||--o{ Containers : hosts Containers ||--o{ Services : exposes + Containers ||--o{ Volumes : mounts Containers }o--o| Jobs : "created by" Services ||--|| HTTPServices : "type: http" Services ||--|| TransportServices : "type: transport" @@ -83,6 +84,20 @@ erDiagram int containerPort } + Volumes { + int id PK + int containerId FK + string name "unique per container" + string hostPath "derived; nullable until provisioned" + string mountPath "unique per container" + enum mode "ro | rw" + string scope "default: container" + boolean builtin "legacy quick_and_dirty backfill" + enum status "pending | ready | failed" + string statusMessage "nullable" + date appliedAt "nullable" + } + HTTPServices { int id PK int serviceId FK,UK @@ -202,6 +217,9 @@ Base model with `type` discriminator (`http`, `transport`, `dns`). Belongs to Co - **TransportService**: `(protocol, externalPort)` unique. `findNextAvailablePort()` static method. - **DnsService**: SRV records with `serviceName`. +### Volume +Persistent bind-mount attached to a container (issue #421). Unique composite indexes on `(containerId, name)` and `(containerId, mountPath)`. `hostPath` is derived server-side from the node volume storage's actual configured `path` (`/volumes/site-///`; scoped by site so containers with the same hostname across sites don't collide on shared storage, and by owner so retained data is only reattached for the same owner) and is null until the create job derives it. `mode` is `ro`/`rw`; each row renders to a Proxmox `mpN` bind mount (`Volume.buildMountConfig`). `status` (`pending` → `ready` | `failed`) is the readiness barrier the create/reconfigure jobs block on — the site agent creates the host directory and reports the result at check-in, which the manager writes to `status`/`statusMessage`/`appliedAt`. `builtin` marks the retired `quick_and_dirty` mount backfilled onto pre-#421 containers for reference (never re-applied; its live `mp0` slot is reserved by `buildMountConfig`). Belongs to Container. See [Volumes](../admins/core-concepts/volumes.md). + ### ExternalDomain Manages public domains for HTTP service exposure. `siteId` is nullable — when set, indicates the "default site" whose DNS is assumed pre-configured (e.g., wildcard A record). Global resource available to all sites. Has many HTTPServices. Cloudflare credentials used for both ACME DNS-01 challenges and cross-site A record management. `authServer` is an optional address of an [oauth2-proxy](https://oauth2-proxy.github.io/oauth2-proxy/) process (e.g. `http://127.0.0.1:4180`) that NGINX proxies `/oauth2/*` to for `auth_request` (see [External Domains](../admins/core-concepts/external-domains.md#authentication)). diff --git a/mie-opensource-landing/zensical.toml b/mie-opensource-landing/zensical.toml index 9108783f..6461d144 100644 --- a/mie-opensource-landing/zensical.toml +++ b/mie-opensource-landing/zensical.toml @@ -39,6 +39,7 @@ nav = [ { "Nodes" = "admins/core-concepts/nodes.md" }, { "Sites" = "admins/core-concepts/sites.md" }, { "Users and Groups" = "admins/core-concepts/users-and-groups.md" }, + { "Volumes" = "admins/core-concepts/volumes.md" }, ] }, { "Settings" = "admins/settings.md" }, { "OIDC Single Sign-On" = "admins/oidc.md" },