Eigenaar meenemen bij het verhuizen
All checks were successful
Deploy server-up (dev) / deploy (push) Successful in 15m32s
All checks were successful
Deploy server-up (dev) / deploy (push) Successful in 15m32s
`shutil.copy2` neemt de rechten mee maar niet de eigenaar, en Server Up draait als root. Verhuisde appdata werd daardoor root:root, waarna een container die als PUID 1000 draait er niet meer in kon. - elk bestand, elke map, elke symlink én de hoofdmap krijgen de uid/gid van het origineel - lukt chownen niet (je draait niet als root), dan gaat de verhuizing door met een waarschuwing in plaats van te struikelen - tests voor beide gevallen, en voor het feit dat de uid van de bron komt Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q9eqpADJSRs49SoGGr4NAy
This commit is contained in:
parent
1530df277d
commit
f4b652e182
3 changed files with 108 additions and 0 deletions
|
|
@ -34,6 +34,14 @@ dan blijft het origineel staan en is de melding de fout. Het omschrijven kijkt
|
|||
naar hele pad-segmenten, zodat `/opt/serverup` vervangen `/opt/serverup-oud` met
|
||||
rust laat.
|
||||
|
||||
**Rechten én eigenaar gaan mee.** `shutil.copy2` neemt de rechten over maar niet
|
||||
de eigenaar, en Server Up draait als root — verhuisde appdata werd dus root:root,
|
||||
waarna een container die als PUID 1000 draait niet meer bij zijn eigen gegevens
|
||||
kon. De linuxserver-images zetten `/config` bij het starten terug, maar postgres,
|
||||
Nextcloud en Immich lopen er gewoon op vast. De verhuizing chownt nu elk bestand,
|
||||
elke map, elke symlink én de hoofdmap zelf naar de eigenaar van het origineel.
|
||||
Lukt dat niet, dan gaat de verhuizing door met een waarschuwing in het log.
|
||||
|
||||
Jobs kregen daarvoor een `voortgang()`; de terminal toont de balk boven het log.
|
||||
|
||||
# v0.8.82-beta — "Poort {port} is vrij" stond er letterlijk
|
||||
|
|
|
|||
|
|
@ -127,6 +127,25 @@ def past_het(bron: Path, doel: Path) -> tuple[bool, str]:
|
|||
|
||||
# ── Verhuizen ────────────────────────────────────────────────────────────────
|
||||
|
||||
def _neem_eigenaar_over(bron: Path, doel: Path, mislukt: list) -> None:
|
||||
"""Geef het gekopieerde dezelfde eigenaar en rechten als het origineel.
|
||||
|
||||
`shutil.copy2` neemt de rechten mee maar níét de eigenaar. Server Up draait
|
||||
als root, dus zonder dit wordt alles root:root — en een container die als
|
||||
PUID 1000 draait kan dan niet meer bij zijn eigen appdata. Bij de
|
||||
linuxserver-images valt dat nog mee (die zetten `/config` bij het starten
|
||||
terug), maar postgres, Nextcloud en Immich lopen er gewoon op vast.
|
||||
"""
|
||||
try:
|
||||
st = os.stat(bron, follow_symlinks=False)
|
||||
os.chown(doel, st.st_uid, st.st_gid, follow_symlinks=False)
|
||||
except (OSError, NotImplementedError) as e:
|
||||
# Draai je niet als root, dan is alles toch al van jou; dan is dit geen
|
||||
# fout maar een no-op. Alleen echte gevallen verzamelen we.
|
||||
if not mislukt:
|
||||
mislukt.append(str(e))
|
||||
|
||||
|
||||
def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None,
|
||||
verwijder_bron: bool = True) -> int:
|
||||
"""Kopieer alles van bron naar doel; verwijder de bron pas als het klopt.
|
||||
|
|
@ -134,6 +153,9 @@ def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None,
|
|||
`voortgang_fn(gedaan_bytes, totaal_bytes, label)` wordt tijdens het kopiëren
|
||||
aangeroepen. Retourneert het aantal gekopieerde bestanden.
|
||||
|
||||
Rechten én eigenaar gaan mee: een verhuisde appdata-map die opeens van root
|
||||
is, breekt elke container die als een gewone gebruiker draait.
|
||||
|
||||
Verwijderen gebeurt alleen als aantal én omvang overeenkomen. Klopt er iets
|
||||
niet, dan blijft de bron staan en volgt er een fout — een half gelukte
|
||||
verhuizing waarbij het origineel al weg is, is het ergste dat hier kan
|
||||
|
|
@ -153,6 +175,12 @@ def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None,
|
|||
f"{diskspace.leesbaar(totaal)}")
|
||||
|
||||
doel.mkdir(parents=True, exist_ok=True)
|
||||
# Ook de hoofdmap zelf: staat die op root, dan kan een container er niet in
|
||||
# schrijven al klopt alles eronder.
|
||||
chown_mislukt: list = []
|
||||
shutil.copystat(bron, doel)
|
||||
_neem_eigenaar_over(bron, doel, chown_mislukt)
|
||||
|
||||
gedaan = kopieerd = 0
|
||||
for p in sorted(bron.rglob("*")):
|
||||
rel = p.relative_to(bron)
|
||||
|
|
@ -175,9 +203,15 @@ def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None,
|
|||
kopieerd += 1
|
||||
if voortgang_fn:
|
||||
voortgang_fn(gedaan, totaal, str(rel))
|
||||
_neem_eigenaar_over(p, uit, chown_mislukt)
|
||||
except OSError as e:
|
||||
raise PadFout(f"Kopiëren van {rel} mislukte: {e}") from e
|
||||
|
||||
if chown_mislukt and log_fn:
|
||||
log_fn(f"Let op: eigenaar kon niet worden overgenomen "
|
||||
f"({chown_mislukt[0]}). Controleer de rechten als een app niet "
|
||||
f"meer bij zijn gegevens kan.")
|
||||
|
||||
# Controleren vóór we iets weggooien.
|
||||
n_doel, b_doel = meet(doel)
|
||||
if n_doel < aantal or b_doel < totaal:
|
||||
|
|
|
|||
|
|
@ -113,6 +113,72 @@ def test_zelfde_map_doet_niets(env, tmp_path):
|
|||
assert (bron / "a.txt").exists()
|
||||
|
||||
|
||||
def test_eigenaar_en_rechten_gaan_mee(env, tmp_path, monkeypatch):
|
||||
"""`shutil.copy2` neemt de rechten mee maar niet de eigenaar.
|
||||
|
||||
Server Up draait als root, dus zonder chown werd verhuisde appdata root:root
|
||||
en kwam een container die als PUID 1000 draait er niet meer in. De test kan
|
||||
zelf niet chownen (dat mag alleen root), dus we leggen vast wát er gevraagd
|
||||
wordt.
|
||||
"""
|
||||
from core import paden
|
||||
bron = _vul(tmp_path / "oud", {"a.txt": "hallo", "sub/b.txt": "wereld"})
|
||||
os.chmod(bron / "a.txt", 0o640)
|
||||
doel = tmp_path / "nieuw"
|
||||
|
||||
gevraagd = {}
|
||||
monkeypatch.setattr(paden.os, "chown",
|
||||
lambda p, u, g, **kw: gevraagd.__setitem__(str(p), (u, g)))
|
||||
|
||||
paden.verhuis(bron, doel)
|
||||
|
||||
eigen = (os.getuid(), os.getgid())
|
||||
assert gevraagd.get(str(doel)) == eigen, "de hoofdmap zelf blijft van root"
|
||||
assert gevraagd.get(str(doel / "a.txt")) == eigen
|
||||
assert gevraagd.get(str(doel / "sub")) == eigen
|
||||
assert gevraagd.get(str(doel / "sub" / "b.txt")) == eigen
|
||||
# De rechten komen van copy2/copystat en horen ook te kloppen.
|
||||
assert os.stat(doel / "a.txt").st_mode & 0o777 == 0o640
|
||||
|
||||
|
||||
def test_de_eigenaar_komt_van_de_bron(env, tmp_path, monkeypatch):
|
||||
"""De uid/gid moet van het origineel komen, niet van wie er toevallig
|
||||
kopieert. Chownen kan alleen root, dus we leggen de vraag vast."""
|
||||
from core import paden
|
||||
bron = _vul(tmp_path, {"a.txt": "x"}) / "a.txt"
|
||||
doel = tmp_path / "b.txt"
|
||||
doel.write_text("x")
|
||||
|
||||
st = os.stat(bron)
|
||||
# Een echte stat_result met een andere eigenaar, zodat copystat blijft werken.
|
||||
nep = os.stat_result((st.st_mode, st.st_ino, st.st_dev, st.st_nlink,
|
||||
1000, 1000, st.st_size,
|
||||
int(st.st_atime), int(st.st_mtime), int(st.st_ctime)))
|
||||
monkeypatch.setattr(paden.os, "stat", lambda p, **kw: nep)
|
||||
gevraagd = []
|
||||
monkeypatch.setattr(paden.os, "chown",
|
||||
lambda p, u, g, **kw: gevraagd.append((u, g)))
|
||||
|
||||
paden._neem_eigenaar_over(bron, doel, [])
|
||||
assert gevraagd == [(1000, 1000)]
|
||||
|
||||
|
||||
def test_chown_die_niet_mag_stopt_de_verhuizing_niet(env, tmp_path, monkeypatch):
|
||||
"""Draai je niet als root, dan is alles toch al van jou. Dat mag geen fout
|
||||
zijn — wel een melding, want dan kán het misgaan."""
|
||||
from core import paden
|
||||
bron = _vul(tmp_path / "oud", {"a.txt": "hallo"})
|
||||
doel = tmp_path / "nieuw"
|
||||
monkeypatch.setattr(paden.os, "chown",
|
||||
lambda *a, **k: (_ for _ in ()).throw(PermissionError("mag niet")))
|
||||
|
||||
regels = []
|
||||
n = paden.verhuis(bron, doel, log_fn=regels.append)
|
||||
|
||||
assert n == 1 and (doel / "a.txt").read_text() == "hallo"
|
||||
assert any("eigenaar" in r for r in regels), "geen waarschuwing over de rechten"
|
||||
|
||||
|
||||
def test_symlinks_blijven_symlinks(env, tmp_path):
|
||||
"""Een link volgen zou de inhoud dupliceren of buiten de boom kunnen wijzen."""
|
||||
from core import paden
|
||||
|
|
|
|||
Loading…
Reference in a new issue