fix(oci): verify web readiness and persist private hook contracts

This commit is contained in:
Jonatan
2026-10-28 08:52:57 +01:00
parent 3a0a7eaf14
commit 6a30d92ba7
5 changed files with 165 additions and 1 deletions
+3
View File
@@ -252,6 +252,9 @@ restarts. Update and restore invalidate the completion proof when the image,
runtime configuration or recovery identity changes. A migration update is runtime configuration or recovery identity changes. A migration update is
refused when database or unclassified backend data lives in host directories refused when database or unclassified backend data lives in host directories
that the transaction cannot safely restore. that the transaction cannot safely restore.
The native exit-status monitor currently accepts LXC 7.0.0, the verified protocol
version. Other versions refuse one-shot execution rather than treating a stopped
container as proof of success.
The bridges are local to the installation node; they are not a cluster SDN. The bridges are local to the installation node; they are not a cluster SDN.
Unsupported network and runtime semantics are reported before installation. Unsupported network and runtime semantics are reported before installation.
+28 -1
View File
@@ -10,6 +10,7 @@ import socket
import stat import stat
import subprocess import subprocess
import time import time
import tempfile
import uuid import uuid
import oci_image_cache as image_cache import oci_image_cache as image_cache
@@ -165,6 +166,28 @@ def immich_plan(primary, records, inventory, lifecycle, operation):
return plan return plan
def write_hook_contract(path, value):
"""Atomically replace a pmxcfs private file without changing its directory mode.
pmxcfs assigns permissions from the path and rejects chmod on /etc/pve/priv.
The registry writer manages ordinary private directories, so it is not used here.
"""
for target, directory in ((path.parent, True), (path, False)):
info = target.lstat()
correct_type = stat.S_ISDIR(info.st_mode) if directory else stat.S_ISREG(info.st_mode)
if not correct_type or info.st_uid != os.geteuid() or info.st_mode & 0o077:
raise ValueError(translate('Unsafe dependency hook contract'))
fd, temporary = tempfile.mkstemp(dir=path.parent, prefix='.proxmenux-job-')
try:
with os.fdopen(fd, 'w') as output:
json.dump(value, output, indent=2)
output.write('\n')
os.replace(temporary, path)
finally:
if os.path.exists(temporary):
os.unlink(temporary)
class NativeAdapter: class NativeAdapter:
def __init__(self, root, journal, plan, acknowledge_external_data=False): def __init__(self, root, journal, plan, acknowledge_external_data=False):
self.root, self.journal, self.plan = root, Path(journal), plan self.root, self.journal, self.plan = root, Path(journal), plan
@@ -480,7 +503,7 @@ class NativeAdapter:
if current != rebuilt or intent is not None: if current != rebuilt or intent is not None:
state['job_hook_contract_intent'] = {'before': current, 'after': rebuilt} state['job_hook_contract_intent'] = {'before': current, 'after': rebuilt}
instances.write(marker, state) instances.write(marker, state)
instances.write(path, rebuilt) write_hook_contract(path, rebuilt)
state['job_hook_contract'] = rebuilt state['job_hook_contract'] = rebuilt
state.pop('job_hook_contract_intent', None) state.pop('job_hook_contract_intent', None)
instances.write(marker, state) instances.write(marker, state)
@@ -514,6 +537,10 @@ class NativeAdapter:
try: try:
if check['type'] == 'exec': if check['type'] == 'exec':
member_tx.run('pct', 'exec', str(vmid), '--', *check['argv']) member_tx.run('pct', 'exec', str(vmid), '--', *check['argv'])
# Main/custom web services retain both contracts: an exec
# check never replaces the advertised HTTP readiness probe.
if check.get('url') and (self.services[vmid].get('frontend') or check.get('endpoint')):
member_tx.run('curl', '-fsS', '--noproxy', '*', '--max-time', '5', check['url'])
elif check['type'] == 'http': elif check['type'] == 'http':
member_tx.run('curl', '-fsS', '--noproxy', '*', '--max-time', '5', check['url']) member_tx.run('curl', '-fsS', '--noproxy', '*', '--max-time', '5', check['url'])
elif check['type'] == 'tcp': elif check['type'] == 'tcp':
@@ -135,6 +135,7 @@ class NativeCompletedJobs(unittest.TestCase):
with patch.object(native, 'Path', return_value=path), \ with patch.object(native, 'Path', return_value=path), \
patch.object(adapter, 'state', side_effect=lambda: copy.deepcopy(state)), \ patch.object(adapter, 'state', side_effect=lambda: copy.deepcopy(state)), \
patch.object(instances, 'write', side_effect=write), \ patch.object(instances, 'write', side_effect=write), \
patch.object(native, 'write_hook_contract', side_effect=write), \
patch.object(instances, 'read', return_value=staged), \ patch.object(instances, 'read', return_value=staged), \
patch.object(native.member_tx, 'owned', return_value=CONFIG.encode()), \ patch.object(native.member_tx, 'owned', return_value=CONFIG.encode()), \
patch.object(jobs, 'job_spec_from_record', return_value=updated) as bind: patch.object(jobs, 'job_spec_from_record', return_value=updated) as bind:
@@ -181,6 +182,7 @@ class NativeCompletedJobs(unittest.TestCase):
actual_write(path, value) actual_write(path, value)
with patch.object(native, 'Path', return_value=hook), \ with patch.object(native, 'Path', return_value=hook), \
patch.object(instances, 'write', side_effect=write), \ patch.object(instances, 'write', side_effect=write), \
patch.object(native, 'write_hook_contract', side_effect=write), \
patch.object(instances, 'read', return_value=staged), \ patch.object(instances, 'read', return_value=staged), \
patch.object(native.member_tx, 'owned', return_value=CONFIG.encode()), \ patch.object(native.member_tx, 'owned', return_value=CONFIG.encode()), \
patch.object(jobs, 'job_spec_from_record', return_value=updated): patch.object(jobs, 'job_spec_from_record', return_value=updated):
+54
View File
@@ -0,0 +1,54 @@
"""Job hook contracts can be published on pmxcfs without chmod of /etc/pve/priv."""
from pathlib import Path
import json
import sys
import tempfile
import unittest
from unittest.mock import patch
ROOT = Path(__file__).resolve().parents[1]
sys.path.insert(0, str(ROOT / 'remote'))
import oci_stack_native as native
class JobHookPmxcfsTests(unittest.TestCase):
def test_atomic_publish_does_not_change_parent_permissions(self):
with tempfile.TemporaryDirectory() as folder:
parent = Path(folder)
path = parent / 'proxmenux-stack-4900.json'
path.write_text('{"old":true}')
path.chmod(0o600)
mode = parent.stat().st_mode
with patch.object(native, 'private_directory', side_effect=AssertionError('No directory chmod')), \
patch.object(native.os, 'chmod', side_effect=PermissionError('pmxcfs')):
native.write_hook_contract(path, {'schema': 1, 'dependencies': []})
self.assertEqual(json.loads(path.read_text()), {'schema': 1, 'dependencies': []})
self.assertEqual(parent.stat().st_mode, mode)
self.assertEqual(list(parent.iterdir()), [path])
self.assertEqual(path.stat().st_mode & 0o777, 0o600)
def test_failed_atomic_publish_preserves_existing_contract(self):
with tempfile.TemporaryDirectory() as folder:
path = Path(folder) / 'proxmenux-stack-4900.json'
path.write_text('{"old":true}')
path.chmod(0o600)
with patch.object(native.os, 'replace', side_effect=OSError('pmxcfs write failed')):
with self.assertRaisesRegex(OSError, 'write failed'):
native.write_hook_contract(path, {'schema': 1})
self.assertEqual(json.loads(path.read_text()), {'old': True})
self.assertEqual(list(Path(folder).iterdir()), [path])
def test_untrusted_parent_or_file_is_refused_without_writing(self):
for parent_mode, file_mode in ((0o755, 0o600), (0o700, 0o644)):
with self.subTest(parent_mode=parent_mode, file_mode=file_mode), tempfile.TemporaryDirectory() as folder:
parent = Path(folder); parent.chmod(parent_mode)
path = parent / 'contract.json'; path.write_text('{"old":true}'); path.chmod(file_mode)
with self.assertRaisesRegex(ValueError, 'Unsafe dependency hook contract'):
native.write_hook_contract(path, {'schema': 1})
self.assertEqual(json.loads(path.read_text()), {'old': True})
def test_symlink_contract_is_refused(self):
with tempfile.TemporaryDirectory() as folder:
real = Path(folder) / 'real.json'; real.write_text('{"old":true}'); real.chmod(0o600)
link = Path(folder) / 'contract.json'; link.symlink_to(real)
with self.assertRaisesRegex(ValueError, 'Unsafe dependency hook contract'):
native.write_hook_contract(link, {'schema': 1})
self.assertEqual(json.loads(real.read_text()), {'old': True})
@@ -0,0 +1,78 @@
"""A passing exec check cannot commit an update whose published web service fails."""
import http.server
from pathlib import Path
import subprocess
import sys
import threading
import unittest
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'remote'))
import oci_stack_native as native
class Reply(http.server.BaseHTTPRequestHandler):
def do_GET(self):
self.send_response(self.server.reply_status)
self.end_headers()
def log_message(self, *args):
pass
class NativeExecWebReadinessTests(unittest.TestCase):
def adapter(self, url=None, frontend=True):
check = {'type': 'exec', 'argv': ['true'], 'timeout_seconds': 1}
if url:
check.update(url=url, endpoint={'port': 80})
member = {'vmid': 4800, 'deployment': {}, 'stack': {'deployment': {'services': [
{'vmid': 4800, 'name': 'service', 'frontend': frontend, 'healthcheck': check}]}}}
return native.NativeAdapter(Path('/registry'), Path('/journal'),
{'members': [member], 'primary_vmid': 4800})
def check_web(self, status):
server = http.server.HTTPServer(('127.0.0.1', 0), Reply)
server.reply_status = status
thread = threading.Thread(target=server.serve_forever, daemon=True)
thread.start()
adapter = self.adapter(f'http://127.0.0.1:{server.server_port}/')
probes = []
def execute(*args):
probes.append(args[0])
if args[0] == 'pct':
return b'' # The custom CMD succeeded; web readiness is independent.
result = subprocess.run(args, capture_output=True, timeout=5)
if result.returncode:
raise RuntimeError('HTTP endpoint is not ready')
return result.stdout
try:
with patch.object(adapter, 'is_running', return_value=True), \
patch.object(native.member_tx.gpu_devices, 'validate_runtime'), \
patch.object(native.member_tx, 'run', side_effect=execute), \
patch.object(native.time, 'monotonic', side_effect=[0, 0, 2]), \
patch.object(native.time, 'sleep'):
if status >= 400:
with self.assertRaisesRegex(ValueError, 'service check'):
adapter.healthcheck(4800)
else:
adapter.healthcheck(4800)
self.assertEqual(probes, ['pct', 'curl'])
finally:
server.shutdown(); server.server_close(); thread.join(timeout=2)
def test_http_failure_rejects_even_when_exec_succeeds(self):
self.check_web(503)
def test_web_and_exec_must_both_pass(self):
self.check_web(200)
def test_database_exec_without_frontend_keeps_its_existing_check(self):
adapter = self.adapter(frontend=False)
with patch.object(adapter, 'is_running', return_value=True), \
patch.object(native.member_tx.gpu_devices, 'validate_runtime'), \
patch.object(native.member_tx, 'run', return_value=b'') as run:
adapter.healthcheck(4800)
run.assert_called_once_with('pct', 'exec', '4800', '--', 'true')
if __name__ == '__main__':
unittest.main()