diff --git a/.github/scripts/tests/test_oci_storage_sk.py b/.github/scripts/tests/test_oci_storage_sk.py index 99a5bb76..deae0954 100644 --- a/.github/scripts/tests/test_oci_storage_sk.py +++ b/.github/scripts/tests/test_oci_storage_sk.py @@ -202,7 +202,8 @@ class InputAndCancellationTests(StorageFixture): def test_real_menu_error_handler_has_slovak_heading_and_preserves_detail(self): ui=Mock();ui.choose.side_effect=['custom',None] - with patch.object(cli,'interactive_ui',return_value=ui),patch.object(custom,'explore',side_effect=ValueError('fiction-detail')), \ + with patch.object(cli,'interactive_ui',return_value=ui),patch.object(cli,'_catalog_options',return_value=([],{}, {}, {}, [])), \ + patch.object(custom,'explore',side_effect=ValueError('fiction-detail')), \ patch.object(updates,'UpdateCheck') as checker, \ patch.object(catalog_refresh,'CatalogRefresh') as catalog_checker: # Keep this wording/error-handler test offline and free of writes. diff --git a/lang/de.json b/lang/de.json index 8cdd233f..d6fc5b23 100644 --- a/lang/de.json +++ b/lang/de.json @@ -9017,5 +9017,8 @@ "⚠ Disk data will NOT be erased.": "⚠ Festplattendaten werden NICHT gelöscht.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ Die Festplatte wird ausgehängt und aus /etc/fstab entfernt.", "⚠ The /etc/fstab entry will be removed.": "⚠ Der /etc/fstab-Eintrag wird entfernt.", - "⚠ The disk will be unmounted.": "⚠ Die Festplatte wird ausgehängt." + "⚠ The disk will be unmounted.": "⚠ Die Festplatte wird ausgehängt.", + "Invalid container device GID": "Ungültige Geräte-GID im Container", + "The container device settings changed since the last recorded operation": "Die Geräteeinstellungen im Container haben sich seit dem letzten protokollierten Vorgang geändert", + "The recorded container device GID does not match its configuration": "Die protokollierte Geräte-GID im Container stimmt nicht mit der Konfiguration überein" } diff --git a/lang/es.json b/lang/es.json index 18af7e3c..6b2ff17b 100644 --- a/lang/es.json +++ b/lang/es.json @@ -8987,5 +8987,8 @@ "⚠ Disk data will NOT be erased.": "⚠ Los datos del disco NO se borrarán.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ El disco se desmontará y se eliminará de /etc/fstab.", "⚠ The /etc/fstab entry will be removed.": "⚠ Se eliminará la entrada /etc/fstab.", - "⚠ The disk will be unmounted.": "⚠ El disco se desmontará." + "⚠ The disk will be unmounted.": "⚠ El disco se desmontará.", + "Invalid container device GID": "El GID del dispositivo del contenedor no es válido", + "The container device settings changed since the last recorded operation": "La configuración del dispositivo del contenedor ha cambiado desde la última operación registrada", + "The recorded container device GID does not match its configuration": "El GID registrado del dispositivo del contenedor no coincide con su configuración" } diff --git a/lang/fr.json b/lang/fr.json index e8e0e257..646d4157 100644 --- a/lang/fr.json +++ b/lang/fr.json @@ -9016,5 +9016,8 @@ "⚠ Disk data will NOT be erased.": "⚠ Les données du disque ne seront PAS effacées.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ Le disque sera démonté et supprimé de /etc/fstab.", "⚠ The /etc/fstab entry will be removed.": "⚠ L'entrée /etc/fstab sera supprimée.", - "⚠ The disk will be unmounted.": "⚠ Le disque sera démonté." + "⚠ The disk will be unmounted.": "⚠ Le disque sera démonté.", + "Invalid container device GID": "GID de périphérique du conteneur invalide", + "The container device settings changed since the last recorded operation": "La configuration du périphérique du conteneur a changé depuis la dernière opération enregistrée", + "The recorded container device GID does not match its configuration": "Le GID enregistré du périphérique du conteneur ne correspond pas à sa configuration" } diff --git a/lang/it.json b/lang/it.json index 6ca7a171..1a896f17 100644 --- a/lang/it.json +++ b/lang/it.json @@ -9017,5 +9017,8 @@ "⚠ Disk data will NOT be erased.": "⚠ I dati del disco NON verranno cancellati.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ Il disco verrà smontato e rimosso da /etc/fstab.", "⚠ The /etc/fstab entry will be removed.": "⚠ La voce /etc/fstab verrà rimossa.", - "⚠ The disk will be unmounted.": "⚠ Il disco verrà smontato." + "⚠ The disk will be unmounted.": "⚠ Il disco verrà smontato.", + "Invalid container device GID": "GID del dispositivo del container non valido", + "The container device settings changed since the last recorded operation": "Le impostazioni del dispositivo del container sono cambiate dall’ultima operazione registrata", + "The recorded container device GID does not match its configuration": "Il GID registrato del dispositivo del container non corrisponde alla sua configurazione" } diff --git a/lang/pt.json b/lang/pt.json index d21c6f98..dc60401a 100644 --- a/lang/pt.json +++ b/lang/pt.json @@ -9017,5 +9017,8 @@ "⚠ Disk data will NOT be erased.": "⚠ Os dados do disco NÃO serão apagados.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ O disco será desmontado e removido de /etc/fstab.", "⚠ The /etc/fstab entry will be removed.": "⚠ A entrada /etc/fstab será removida.", - "⚠ The disk will be unmounted.": "⚠ O disco será desmontado." + "⚠ The disk will be unmounted.": "⚠ O disco será desmontado.", + "Invalid container device GID": "GID do dispositivo do contentor inválido", + "The container device settings changed since the last recorded operation": "As definições do dispositivo do contentor mudaram desde a última operação registada", + "The recorded container device GID does not match its configuration": "O GID registado do dispositivo do contentor não corresponde à configuração" } diff --git a/lang/sk.json b/lang/sk.json index cc09d7bf..ca7d14d5 100644 --- a/lang/sk.json +++ b/lang/sk.json @@ -9017,5 +9017,8 @@ "⚠ Disk data will NOT be erased.": "⚠ Dáta na disku sa NEVYMAŽÚ.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ Disk sa odpojí a odstráni z /etc/fstab.", "⚠ The /etc/fstab entry will be removed.": "⚠ Záznam v /etc/fstab bude odstránený.", - "⚠ The disk will be unmounted.": "⚠ Disk bude odpojený." + "⚠ The disk will be unmounted.": "⚠ Disk bude odpojený.", + "Invalid container device GID": "Neplatné GID zariadenia v kontajneri", + "The container device settings changed since the last recorded operation": "Nastavenia zariadenia v kontajneri sa od poslednej zaznamenanej operácie zmenili", + "The recorded container device GID does not match its configuration": "Zaznamenané GID zariadenia v kontajneri sa nezhoduje s jeho konfiguráciou" } diff --git a/lang/sv.json b/lang/sv.json index 784f0e00..6e93f9a4 100644 --- a/lang/sv.json +++ b/lang/sv.json @@ -9017,5 +9017,8 @@ "⚠ Disk data will NOT be erased.": "⚠ Diskdata kommer INTE att raderas.", "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ Disken kommer att avmonteras och tas bort från /etc/fstab.", "⚠ The /etc/fstab entry will be removed.": "⚠ /etc/fstab-posten kommer att tas bort.", - "⚠ The disk will be unmounted.": "⚠ Disken kommer att avmonteras." + "⚠ The disk will be unmounted.": "⚠ Disken kommer att avmonteras.", + "Invalid container device GID": "Ogiltigt enhets-GID i containern", + "The container device settings changed since the last recorded operation": "Inställningarna för enheten i containern har ändrats sedan den senaste registrerade åtgärden", + "The recorded container device GID does not match its configuration": "Det registrerade enhets-GID:t i containern stämmer inte med konfigurationen" } diff --git a/oci/remote/install_oci.sh b/oci/remote/install_oci.sh index 3a1b59d0..99682a82 100755 --- a/oci/remote/install_oci.sh +++ b/oci/remote/install_oci.sh @@ -939,7 +939,7 @@ set_lxc_directive() { add_device() { local host_path=$1 expected_type=$2 mode_spec=$3 gid_strategy=$4 deny_write=$5 - local device_uid=${6:-} + local device_uid=${6:-} container_gid=${7:-} local mode gid dev_value [[ $host_path == /dev/* && $host_path != *[[:space:]]* && $host_path != *","* ]] \ || die "$(translate "Invalid device path:") $host_path" @@ -963,7 +963,12 @@ add_device() { || die "$(translate "Invalid device UID:") $device_uid" dev_value="${dev_value},uid=${device_uid}" fi - if [[ $gid_strategy == host-device-gid ]]; then + if [[ -n $container_gid ]]; then + [[ $container_gid =~ ^[0-9]{1,10}$ ]] && (( container_gid < 4294967295 )) \ + || die "$(translate "Invalid container device GID"): $container_gid" + gid=$container_gid + dev_value="${dev_value},gid=${gid}" + elif [[ $gid_strategy == host-device-gid ]]; then gid=$(stat -c '%g' "$host_path") dev_value="${dev_value},gid=${gid}" elif [[ $gid_strategy != none ]]; then @@ -978,7 +983,7 @@ add_device() { } add_character_device() { - add_device "$1" character "$2" "$3" "$4" "${5:-}" + add_device "$1" character "$2" "$3" "$4" "${5:-}" "${6:-}" } prepare_nvidia_driver_links() { @@ -2043,6 +2048,7 @@ while IFS= read -r DEVICE_ENCODED; do DEVICE=$(printf '%s' "$DEVICE_ENCODED" | base64 -d) DEVICE_KIND=$(jq -r '.kind // "character-device"' <<<"$DEVICE") DEVICE_GID_ENV=$(jq -r '.append_host_device_gid_to_environment // empty' <<<"$DEVICE") + CONTAINER_GID=$(jq -r '.container_gid // empty' <<<"$DEVICE") if [[ $DEVICE_KIND == nvidia-runtime ]]; then NVIDIA_GID_ENV=$DEVICE_GID_ENV if (( NVIDIA_RUNTIME_CONFIGURED == 0 )); then @@ -2062,13 +2068,13 @@ while IFS= read -r DEVICE_ENCODED; do case "$DEVICE_KIND" in character-device) DEVICE_UID=$(jq -r '.uid // empty' <<<"$DEVICE") - add_character_device "$HOST_PATH" "$MODE" "$GID_STRATEGY" "$DENY_WRITE" "$DEVICE_UID" + add_character_device "$HOST_PATH" "$MODE" "$GID_STRATEGY" "$DENY_WRITE" "$DEVICE_UID" "$CONTAINER_GID" if [[ -n $DEVICE_GID_ENV ]]; then append_deployment_environment_csv "$DEVICE_GID_ENV" "$(stat -c '%g' "$HOST_PATH")" fi ;; block-device) - add_device "$HOST_PATH" block "$MODE" "$GID_STRATEGY" "$DENY_WRITE" + add_device "$HOST_PATH" block "$MODE" "$GID_STRATEGY" "$DENY_WRITE" "" "$CONTAINER_GID" if [[ -n $DEVICE_GID_ENV ]]; then append_deployment_environment_csv "$DEVICE_GID_ENV" "$(stat -c '%g' "$HOST_PATH")" fi @@ -2078,7 +2084,7 @@ while IFS= read -r DEVICE_ENCODED; do FOUND_TREE_DEVICE=0 while IFS= read -r TREE_DEVICE; do [[ -n $TREE_DEVICE ]] || continue - add_character_device "$TREE_DEVICE" "$MODE" "$GID_STRATEGY" "$DENY_WRITE" + add_character_device "$TREE_DEVICE" "$MODE" "$GID_STRATEGY" "$DENY_WRITE" "" "$CONTAINER_GID" FOUND_TREE_DEVICE=1 done < <(find "$HOST_PATH" -print 2>/dev/null | while IFS= read -r path; do [[ -c $path ]] && printf '%s\n' "$path" diff --git a/oci/remote/oci_accelerators.py b/oci/remote/oci_accelerators.py index d6f24b84..18ea5dd2 100644 --- a/oci/remote/oci_accelerators.py +++ b/oci/remote/oci_accelerators.py @@ -40,6 +40,14 @@ def drm_plan(deployment): return dict(deployment, devices=[d for d in deployment.get('devices', []) if d.get('kind') != 'nvidia-runtime']) +def container_gid(device, host_value): + return drm.container_gid(device, host_value) + + +def preserve_recorded_container_gids(record, candidate, config): + drm.preserve_recorded_container_gids(record, candidate, config) + + def planned(deployment): result = drm.planned(drm_plan(deployment)) if nvidia.enabled(deployment): diff --git a/oci/remote/oci_gpu_devices.py b/oci/remote/oci_gpu_devices.py index 1155ff6e..4ed57c60 100644 --- a/oci/remote/oci_gpu_devices.py +++ b/oci/remote/oci_gpu_devices.py @@ -88,10 +88,71 @@ def planned(deployment): raise ValueError(translate('Invalid device mode')) if device.get('gid_strategy') not in ('none', 'host-device-gid'): raise ValueError(translate('Unsupported device GID strategy')) + if 'container_gid' in device: + container_gid(device, value) result[path] = value return result +def container_gid(device, host_value): + """The GID assigned to the device node in the LXC's user namespace. + + A recorded container GID is distinct from the host device's GID. Proxmox + maps the configured LXC GID through the container's id map when it creates + its passthrough node; the numeric host GID is not automatically translated. + """ + if 'container_gid' in device: + gid = device['container_gid'] + if isinstance(gid, bool) or not isinstance(gid, int) or not 0 <= gid < 4294967295: + raise ValueError(translate('Invalid container device GID')) + return gid + return host_value['gid'] if device['gid_strategy'] == 'host-device-gid' else 0 + + +def _permission_fields(fields): + return (fields.get('path'), int(fields.get('mode', '0660'), 8), + int(fields.get('uid', 0)), int(fields.get('gid', 0)), + fields.get('deny-write', '0')) + + +def preserve_recorded_container_gids(record, candidate, config): + """Carry forward only device GIDs already captured in the saved config. + + This is not a host-to-container GID translation. The saved config is the + baseline for the GID Proxmox previously assigned inside the LXC. Any + unrecorded change is rejected, and the host device is still verified + independently by ``verify_baseline``. + """ + saved_config = record.get('observed', {}).get('config') + if not isinstance(saved_config, str) or not saved_config: + return + saved = actual_devices(saved_config.encode()) + current = actual_devices(config) + if set(saved) != set(current): + return # check() below reports the device set mismatch. + if any(_permission_fields(saved[path]) != _permission_fields(current[path]) for path in saved): + raise ValueError(translate('The container device settings changed since the last recorded operation')) + + original_devices = record.get('deployment', {}).get('devices', []) + old_snapshots = planned(record.get('deployment', {})) + for device in original_devices: + path = device.get('host_path') + if path not in saved or path not in old_snapshots: + continue + observed_gid = int(saved[path].get('gid', 0)) + expected_gid = container_gid(device, old_snapshots[path]) + if observed_gid != expected_gid and 'container_gid' in device: + raise ValueError(translate('The recorded container device GID does not match its configuration')) + if observed_gid != expected_gid: + device['container_gid'] = observed_gid + + for desired in candidate.get('deployment', {}).get('devices', []): + if (desired.get('host_path') == path + and desired.get('gid_strategy') == device.get('gid_strategy') + and 'container_gid' not in desired): + desired['container_gid'] = observed_gid + + def verify(expected): for path, value in expected.items(): if snapshot(path) != value: @@ -137,7 +198,7 @@ def check(config, deployment): value = expected[path] requested_mode = device.get('mode', '0660') mode = value['mode'] if requested_mode == 'preserve-host' else int(requested_mode, 8) - gid = value['gid'] if device['gid_strategy'] == 'host-device-gid' else 0 + gid = container_gid(device, value) if (set(fields) - {'path', 'mode', 'gid', 'uid', 'deny-write'} or int(fields.get('mode', '0660'), 8) != mode or int(fields.get('gid', 0)) != gid diff --git a/oci/remote/oci_instance_transaction.py b/oci/remote/oci_instance_transaction.py index 5639d97d..8e2c44da 100644 --- a/oci/remote/oci_instance_transaction.py +++ b/oci/remote/oci_instance_transaction.py @@ -745,6 +745,7 @@ def preflight(record, candidate, config, coordinated=None, in_place=False, basel if desired['rootfs'] != deployment['rootfs']: raise ValueError(translate('Changing the rootfs or its storage requires a separate migration')) actual = mounts(config) + gpu_devices.preserve_recorded_container_gids(record, candidate, config) gpu_devices.check(config, deployment) if baseline: gpu_devices.verify_baseline(record['observed'].get('gpu_devices', {}), deployment) diff --git a/oci/remote/oci_native_modify.py b/oci/remote/oci_native_modify.py index 40fbc5f1..767d8161 100644 --- a/oci/remote/oci_native_modify.py +++ b/oci/remote/oci_native_modify.py @@ -185,7 +185,7 @@ def plan(record, candidate, config): info = inventory[path] mode = info['mode'] if device.get('mode') == 'preserve-host' else int(device.get('mode', '0660'), 8) values[key] = (f"path={path},mode={mode:04o},uid={int(device.get('uid', 0))}," - f"gid={info['gid'] if device['gid_strategy'] == 'host-device-gid' else 0}," + f"gid={devices.container_gid(device, info)}," f"deny-write={1 if device.get('deny_write') else 0}") if 'mounts' in changed: actual = transaction.mounts(config) diff --git a/oci/remote/oci_stack_native.py b/oci/remote/oci_stack_native.py index cd851687..c2c16d18 100644 --- a/oci/remote/oci_stack_native.py +++ b/oci/remote/oci_stack_native.py @@ -585,7 +585,7 @@ class NativeAdapter: if sha(config) != child.get('staged_config_sha256'): raise ValueError(translate('The configuration of a new member changed after it was created')) member_tx.check_runtime_mounts(config, original['deployment']) - member_tx.gpu_devices.check(config, original['deployment']) + member_tx.gpu_devices.check(config, child['candidate_contract']['deployment']) child['candidate_host_sources'] = member_tx.candidate_host_sources(journal, child) child['validated_config_sha256'] = sha(config) instances.write(journal, child) diff --git a/oci/tests/test_gpu_device_gid.py b/oci/tests/test_gpu_device_gid.py new file mode 100644 index 00000000..d6f4d243 --- /dev/null +++ b/oci/tests/test_gpu_device_gid.py @@ -0,0 +1,76 @@ +"""Keep recorded LXC device-node GIDs distinct from host device ownership.""" +from pathlib import Path +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'remote')) + +import oci_gpu_devices as devices +import oci_accelerators as accelerators + + +HOST = { + 'path': '/dev/dri/renderD128', 'major': 226, 'minor': 128, + 'uid': 0, 'gid': 993, 'mode': 0o660, + 'vendor': '0x8086', 'pci_path': '/sys/devices/pci0000:00/0000:00:02.0', +} +DEPLOYMENT = {'devices': [{ + 'kind': 'character-device', 'host_path': '/dev/dri/renderD128', + 'container_path': '/dev/dri/renderD128', 'gid_strategy': 'host-device-gid', + 'mode': '0660', 'drm_vendor_ids': ['0x8086', '0x1002'], +}]} +SAVED_CONFIG = 'dev0: path=/dev/dri/renderD128,mode=0660,deny-write=0,gid=44\n' +CURRENT_CONFIG = SAVED_CONFIG.encode() + + +class ContainerDeviceGidTests(unittest.TestCase): + @patch.object(devices, 'snapshot', return_value=HOST) + def test_recorded_guest_gid_is_carried_to_update_and_still_checks_host_identity(self, _snapshot): + record = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}, + 'observed': {'config': SAVED_CONFIG}} + candidate = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}} + + devices.preserve_recorded_container_gids(record, candidate, CURRENT_CONFIG) + + self.assertEqual(record['deployment']['devices'][0]['container_gid'], 44) + self.assertEqual(candidate['deployment']['devices'][0]['container_gid'], 44) + self.assertEqual(devices.check(CURRENT_CONFIG, record['deployment'])['/dev/dri/renderD128'], HOST) + self.assertEqual(devices.check(CURRENT_CONFIG, candidate['deployment'])['/dev/dri/renderD128'], HOST) + + @patch.object(devices, 'snapshot', return_value=HOST) + def test_unrecorded_guest_gid_change_is_rejected(self, _snapshot): + record = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}, + 'observed': {'config': SAVED_CONFIG}} + candidate = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}} + changed = CURRENT_CONFIG.replace(b'gid=44', b'gid=45') + + with self.assertRaisesRegex(ValueError, 'device settings changed'): + devices.preserve_recorded_container_gids(record, candidate, changed) + + @patch.object(devices, 'snapshot', return_value=HOST) + def test_host_gid_is_not_automatically_mapped_to_guest_gid_44(self, _snapshot): + with self.assertRaisesRegex(ValueError, 'native GPU permissions were not kept'): + devices.check(CURRENT_CONFIG, DEPLOYMENT) + + # Under Proxmox's default unprivileged map, guest GID 44 maps to host + # GID 100044; host GID 993 is outside that map. + self.assertEqual(100000 + 44, 100044) + self.assertFalse(100000 <= 993 < 165536) + + @patch.object(devices, 'snapshot', return_value=HOST) + def test_changed_host_device_identity_or_permissions_still_fail_baseline(self, snapshot): + baseline = {'/dev/dri/renderD128': HOST} + current = dict(HOST, gid=992) + snapshot.return_value = current + with self.assertRaisesRegex(ValueError, 'GPU identity or permissions changed'): + accelerators.verify_baseline(baseline, DEPLOYMENT) + + def test_container_gid_must_be_a_bounded_integer(self): + for invalid in (True, -1, 4294967295, '44'): + with self.subTest(invalid=invalid), self.assertRaisesRegex(ValueError, 'Invalid container device GID'): + devices.container_gid({'gid_strategy': 'host-device-gid', 'container_gid': invalid}, HOST) + + +if __name__ == '__main__': + unittest.main()