From 5e75f3da31697e9348fa41c845ffed39183dde54 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 24 Aug 2026 22:55:46 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20proxmox=20:=20un=20seul=20nom=20par=20e?= =?UTF-8?q?ntr=C3=A9e=20~/.ssh/config,=20et=20le=20bon?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L'entrée portait deux noms sur sa ligne « Host » — le chaîné « hôte+vm » et le court : « Host erplibre-proxmox-9+erplibre-arch-latest erplibre-arch-latest ». Le second est un doublon dès que le premier suffit. ssh n'a besoin que d'un nom ; le doubler n'ajoute qu'une façon de plus d'écrire la même adresse. Rapporté. Un seul, donc, et choisi : le nom court quand il est LIBRE, c'est celui qu'on tape ; le chaîné quand il désignerait une autre machine — une VM locale homonyme, ou la VM d'un autre hôte Proxmox. Ce qui a forcé le changement est nommé à l'écran plutôt que laissé en surprise. « Pris » se juge sur le ProxyJump du bloc et non sur sa seule présence. Le test l'a montré avant l'usage : notre propre entrée, réécrite à chaque déploiement, se prenait pour une rivale et le nom basculait d'une fois sur l'autre. --- EN --- The entry carried two names on its "Host" line — the chained "host+vm" and the short one: "Host erplibre-proxmox-9+erplibre-arch-latest erplibre-arch-latest". The second is redundant as soon as the first is enough. ssh needs one name; doubling it only adds another way to write the same address. Reported. One name then, and a chosen one: the short one while it is FREE, since that is what you type; the chained one when it would point at another machine — a local VM of the same name, or another Proxmox host's VM. Whatever forced the change is named on screen rather than left as a surprise. "Taken" is judged on the block's ProxyJump, not on its mere presence. The test showed it before use: our own entry, rewritten at every deployment, took itself for a rival and the name flipped from one run to the next. Assisted-by: Claude Opus 5 --- script/todo/proxmox_menu.py | 70 ++++++++++++++++------ script/todo/todo.py | 34 +++++++++++ script/todo/todo_i18n.py | 8 +++ test/test_proxmox_form.py | 115 ++++++++++++++++++++++++++++++++++-- 4 files changed, 203 insertions(+), 24 deletions(-) diff --git a/script/todo/proxmox_menu.py b/script/todo/proxmox_menu.py index 294fef0..79be598 100644 --- a/script/todo/proxmox_menu.py +++ b/script/todo/proxmox_menu.py @@ -1121,6 +1121,36 @@ class ProxmoxMenuMixin: fh.write("\n".join(entete) + "\n") return chemin + def _pve_alias_names(self, nom, chaine, locaux, rebond=""): + """UN seul nom pour l'entrée ~/.ssh/config, et lequel. + + Deux noms sur la même ligne « Host » — le court et le chaîné + « hôte+vm » — étaient un doublon dès que le court était libre : ssh + n'a besoin que d'un nom, et le second n'ajoutait qu'une façon de plus + d'écrire la même adresse. Rapporté. + + Le court quand il est LIBRE, c'est celui qu'on tape ; le chaîné + sinon, car un nom déjà pris désigne une AUTRE machine — une VM locale + du même nom, ou la VM d'un autre hôte Proxmox. Vécu : « ssh » partait + vers la machine locale qui partageait le nom. + + « Pris » se juge sur le ProxyJump du bloc et non sur sa seule + présence : notre propre entrée, réécrite à chaque déploiement, se + serait autrement prise pour une rivale — et le nom aurait basculé + d'une fois sur l'autre. + + Rend (noms, volé) — `volé` nomme ce qui a forcé le nom chaîné, pour + que l'appelant le dise plutôt que de laisser la surprise.""" + if nom in locaux: + return [chaine], t("a local VM") + bloc = self._ssh_config_block(nom) + notre = ( + not bloc + or chaine in bloc.get("names", ()) + or (rebond and bloc.get("proxyjump") == rebond) + ) + return ([nom], "") if notre else ([chaine], "~/.ssh/config") + def _pve_set_timezone(self, cible, spec): """Pose le fuseau DANS la VM, par ssh. @@ -1335,10 +1365,16 @@ class ProxmoxMenuMixin: # d'installation — qui ré-résout par virsh — irait installer # ERPLibre sur ELLE. Vécu : « erplibre-ubuntu-2604 » déployée sur # Proxmox, installation partie sur la VM locale du même nom. - if vm["name"] in locaux: + noms_alias, vole = self._pve_alias_names( + vm["name"], + alias_chaine(vm["name"]), + locaux, + host["target"], + ) + if vole: print( - f" ⚠ {t('A local VM already bears this name:')}" - f" {t('the alias goes to')} {alias_chaine(vm['name'])}" + f" ⚠ {t('This name is already taken by')} {vole} :" + f" {t('the alias goes to')} {noms_alias[0]}" ) # L'entrée ~/.ssh/config est le SEUL chemin vers cette VM : elle # est derrière l'hôte Proxmox (pont interne), donc son adresse @@ -1350,13 +1386,6 @@ class ProxmoxMenuMixin: if not spec.get("add_ssh_config") and besoin: print(f" → {t('~/.ssh/config written anyway (install)')}") if spec.get("add_ssh_config") or besoin: - # Deux noms, comme pour les VM imbriquées : le nom CHAÎNÉ - # « hôte+vm » dit où la machine vit et ne peut rien voler, et - # le nom court n'est ajouté que s'il ne désigne pas déjà une - # VM locale. - noms_alias = [alias_chaine(vm["name"])] - if vm["name"] not in locaux: - noms_alias.append(vm["name"]) self._write_ssh_config_entry( noms_alias, spec.get("user") or "erplibre", @@ -1364,8 +1393,8 @@ class ProxmoxMenuMixin: identity_file=self._ssh_private_key(cle_locale), proxy_jump=host["target"], ) - alias[vm["name"]] = noms_alias[-1] - print(f" ✓ ~/.ssh/config : ssh {noms_alias[-1]}") + alias[vm["name"]] = noms_alias[0] + print(f" ✓ ~/.ssh/config : ssh {noms_alias[0]}") vm["adresse"] = ip vm["alias"] = alias.get(vm["name"], vm["name"]) # Le guide AVANT l'installation : il doit être là même si rien ne @@ -1659,13 +1688,16 @@ class ProxmoxMenuMixin: if not ip: print(f" ⚠ {vm['name']} : {t('no address, skipped')}") continue - noms = [f"{hote_court}+{vm['name']}"] - if vm["name"] not in locaux: - noms.append(vm["name"]) - else: + noms, vole = self._pve_alias_names( + vm["name"], + f"{hote_court}+{vm['name']}", + locaux, + host["target"], + ) + if vole: print( - f" ⚠ {t('A local VM already bears this name:')}" - f" {vm['name']}" + f" ⚠ {t('This name is already taken by')} {vole} :" + f" {t('the alias goes to')} {noms[0]}" ) self._write_ssh_config_entry( noms, @@ -1675,7 +1707,7 @@ class ProxmoxMenuMixin: proxy_jump=host["target"], ) print( - f" ✓ ssh {noms[-1]} ({ip} {t('through')} {host['target']})" + f" ✓ ssh {noms[0]} ({ip} {t('through')} {host['target']})" ) def _pve_test_vm(self): diff --git a/script/todo/todo.py b/script/todo/todo.py index 0249191..fef5f1c 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -1705,6 +1705,40 @@ class TODO( pass return names + @classmethod + def _ssh_config_block(cls, name): + """Le bloc « Host … » qui déclare `name`, ou {}. + + Rend ses noms ET ses directives : savoir qu'un nom est pris ne suffit + pas, il faut savoir PAR QUI. Un nom court déjà déclaré peut être notre + propre entrée qu'on réécrit — auquel cas il n'y a rien de volé — ou + celle d'une autre machine, et c'est le ProxyJump qui les distingue. + + {"names": [...], "proxyjump": "...", "hostname": "..."}.""" + path = os.path.expanduser("~/.ssh/config") + try: + with open(path, encoding="utf-8") as fh: + contenu = fh.read() + except OSError: + return {} + bloc = None + for line in contenu.splitlines(): + if re.match(r"^[ \t]*Host[ \t]+", line): + if bloc is not None: + return bloc + noms = line.split()[1:] + bloc = {"names": noms} if name in noms else None + continue + if bloc is None: + continue + # Une ligne non indentée et non vide clôt le bloc. + if line.strip() and not line[:1].isspace(): + return bloc + mots = line.split() + if len(mots) >= 2 and mots[0].lower() in ("proxyjump", "hostname"): + bloc[mots[0].lower()] = mots[1] + return bloc or {} + @staticmethod def _ssh_config_user(host): """`User` déclaré pour cet hôte dans ~/.ssh/config, ou "". diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 8543898..c8e76e9 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -4002,6 +4002,14 @@ TRANSLATIONS = { "fr": "Quitter (q) pour suivre la mise en route de la VM", "en": "Quit (q) to follow the VM starting up", }, + "This name is already taken by": { + "fr": "Ce nom est déjà pris par", + "en": "This name is already taken by", + }, + "a local VM": { + "fr": "une VM locale", + "en": "a local VM", + }, "A local VM already bears this name:": { "fr": "Une VM locale porte déjà ce nom :", "en": "A local VM already bears this name:", diff --git a/test/test_proxmox_form.py b/test/test_proxmox_form.py index 592b7a6..2f7c622 100644 --- a/test/test_proxmox_form.py +++ b/test/test_proxmox_form.py @@ -15,6 +15,7 @@ aucun hôte Proxmox n'est joint. import asyncio import sys +import os import unittest sys.argv = ["todo.py"] @@ -597,6 +598,104 @@ class TestDeuxVmDuMemeNom(unittest.TestCase): ) +class TestUnSeulNomDansSshConfig(unittest.TestCase): + """L'entrée portait DEUX noms sur sa ligne « Host » : le nom chaîné + « hôte+vm » et le nom court. + + Rapporté : « Host erplibre-proxmox-9+erplibre-arch-latest + erplibre-arch-latest ». Le second est un doublon dès que le premier + suffit — ssh n'a besoin que d'un nom, et le doubler n'ajoute qu'une façon + de plus d'écrire la même adresse. + + Un seul, donc, et le bon : le court quand il est LIBRE, le chaîné quand + il désignerait une autre machine. « Pris » se juge sur le ProxyJump du + bloc, pas sur sa seule présence — sinon notre propre entrée, réécrite à + chaque déploiement, se prendrait pour une rivale et le nom basculerait + d'une fois sur l'autre.""" + + def setUp(self): + import sys + import tempfile + + sys.argv = ["todo.py"] + from script.todo.todo import TODO + + self.maison = tempfile.mkdtemp() + os.makedirs(os.path.join(self.maison, ".ssh")) + self._vrai_home = os.environ.get("HOME") + os.environ["HOME"] = self.maison + self.todo = TODO.__new__(TODO) + + def tearDown(self): + import shutil + + if self._vrai_home is not None: + os.environ["HOME"] = self._vrai_home + shutil.rmtree(self.maison, ignore_errors=True) + + def _ecrit(self, noms, rebond=""): + self.todo._write_ssh_config_entry( + noms, "erplibre", "10.10.10.150", proxy_jump=rebond or None + ) + + def _lignes_host(self): + with open( + os.path.join(self.maison, ".ssh/config"), encoding="utf-8" + ) as fh: + return [ + ligne.rstrip() for ligne in fh if ligne.startswith("Host ") + ] + + def _choisit(self, nom, locaux=(), rebond="pve9"): + return self.todo._pve_alias_names( + nom, f"pve9+{nom}", set(locaux), rebond + ) + + def test_a_free_name_is_written_alone(self): + noms, vole = self._choisit("erplibre-arch-latest") + self.assertEqual(noms, ["erplibre-arch-latest"]) + self.assertFalse(vole) + self._ecrit(noms, "pve9") + self.assertEqual(self._lignes_host(), ["Host erplibre-arch-latest"]) + + def test_redeploying_the_same_vm_keeps_the_same_name(self): + # Le piège du correctif : notre propre bloc déclare déjà le nom. + self._ecrit(["erplibre-arch-latest"], "pve9") + noms, vole = self._choisit("erplibre-arch-latest") + self.assertEqual(noms, ["erplibre-arch-latest"], "le nom a basculé") + self.assertFalse(vole) + + def test_a_local_vm_keeps_its_name(self): + # Vécu : « ssh » partait vers la machine locale du même nom. + noms, vole = self._choisit( + "erplibre-arch-latest", locaux=("erplibre-arch-latest",) + ) + self.assertEqual(noms, ["pve9+erplibre-arch-latest"]) + self.assertTrue(vole) + + def test_another_proxmox_host_keeps_its_name(self): + self._ecrit(["erplibre-ubuntu-2604"], "pve7") + noms, vole = self._choisit("erplibre-ubuntu-2604") + self.assertEqual(noms, ["pve9+erplibre-ubuntu-2604"]) + self.assertEqual(vole, "~/.ssh/config") + self._ecrit(noms, "pve9") + # Les deux machines cohabitent, chacune sous son nom. + self.assertEqual( + self._lignes_host(), + ["Host erplibre-ubuntu-2604", "Host pve9+erplibre-ubuntu-2604"], + ) + + def test_no_deploy_path_writes_two_names_anymore(self): + import re + from pathlib import Path as P + + src = P("script/todo/proxmox_menu.py").read_text(encoding="utf-8") + self.assertIsNone( + re.search(r"noms_alias\.append|noms\.append\(vm\[.name.\]\)", src), + "le second nom ne doit plus être ajouté", + ) + + class TestLeGuideDeConnexion(unittest.TestCase): """Une VM Proxmox n'avait AUCUN guide, quelle que soit sa distribution. @@ -773,6 +872,11 @@ class TestLeSuivi(unittest.TestCase): nom ) todo._ssh_private_key = lambda k: None + # Hermétique : le choix du nom lit ~/.ssh/config et la liste des + # domaines locaux. Sans ces deux bouchons, le test dépendrait de + # la machine qui le lance. + todo._ssh_config_block = lambda nom: {} + todo._qemu_list_domains = lambda: [] todo._pve_guest_ip = lambda vmid, attente=120: "" todo._qemu_install_erplibre_monitored = lambda *a, **k: None todo._qemu_install_erplibre_vm = lambda *a, **k: None @@ -796,15 +900,16 @@ class TestLeSuivi(unittest.TestCase): return ecrites cmd = {"branch": "develop", "cmd": "make x", "label": "X"} - # Deux noms : le chaîné « hôte+vm », qui dit où la machine vit, et le - # nom court quand aucun domaine local ne le porte déjà. - self.assertEqual(essai(False, cmd, False), [["pve1+vm-a", "vm-a"]]) + # UN nom : le court, puisque rien ne le porte déjà. Le chaîné + # « hôte+vm » ne sort que lorsqu'il faut départager (voir + # TestUnSeulNomDansSshConfig). + self.assertEqual(essai(False, cmd, False), [["vm-a"]]) # Décoché, suivi demandé : le suivi entre aussi par le rebond. - self.assertEqual(essai(False, None, True), [["pve1+vm-a", "vm-a"]]) + self.assertEqual(essai(False, None, True), [["vm-a"]]) # Décoché et rien à faire dans la VM : le choix est respecté. self.assertEqual(essai(False, None, False), []) # Coché : écrite, évidemment. - self.assertEqual(essai(True, None, False), [["pve1+vm-a", "vm-a"]]) + self.assertEqual(essai(True, None, False), [["vm-a"]]) def test_a_local_vm_of_the_same_name_keeps_its_alias(self): """Le piège qui a fait installer ERPLibre sur la MAUVAISE machine.