From 271e3b4d9351d587fb9b2e8b7c29a5683822d961 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Tue, 4 Aug 2026 05:10:49 +0200 Subject: [PATCH] Preserve attributes in syncfiles without disturbing parent directories The rsync push carried no preservation flags, so files arrived with their special permission bits explicitly disabled and a setuid/setgid entry could only be honored by the permissions= chmod on the client side. Preservation was turned on once before in e52a9ff70fee ("Have syncfiles attempt to preserve more") and rolled back the same day in c0287e93edf6 ("Roll back rsync ownership"), because rsync also applied the staging copy's attributes to the parent directories it merely traversed on the way to the synced files, clobbering the permissions of system directories such as /etc. Naming every staged file explicitly through --files-from and adding --no-implied-dirs confines preservation to the content actually being synchronized, leaving traversed directories alone and creating missing ones with default attributes. Two details follow from the way the staging tree is built. Files are staged as symlinks, so rsync reads their attributes through to the real file, but directories are staged as directories and need the source attributes copied onto them for the otherwise empty ones that have to be named explicitly. Ownership is mapped from the account the daemon runs as to root, since that account generally does not exist on the node and would otherwise arrive as a meaningless numeric id. --xattrs from that earlier attempt is deliberately left out: with --copy-links rsync reads xattrs off the symlink rather than its referent, so it transfers nothing here while adding a failure mode on hosts without xattr support. --- confluent_server/confluent/syncfiles.py | 32 ++++++++++++++++++++++++- confluent_server/confluent/util.py | 7 +++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/confluent_server/confluent/syncfiles.py b/confluent_server/confluent/syncfiles.py index 50ac8e52..a57c7d05 100644 --- a/confluent_server/confluent/syncfiles.py +++ b/confluent_server/confluent/syncfiles.py @@ -191,12 +191,33 @@ async def sync_list_to_node(sl, node, suffixes, peerip=None): for ent in sl.appendoncemap: stage_ent(sl.appendoncemap, ent, os.path.join(targdir, suffixes['appendonce']), True) + filelist = [] + for root, dirs, files in os.walk(targdir): + if not dirs and not files and root != targdir: + # otherwise empty directories have to be named to be synced + filelist.append(os.path.relpath(root, targdir)) + for filename in files: + filelist.append( + os.path.relpath(os.path.join(root, filename), targdir)) await sshutil.prep_ssh_key('/etc/confluent/ssh/automation') targip = node if peerip: targip = peerip + # Naming every file (--files-from implies --relative) together with + # --no-implied-dirs confines the preservation flags to the content + # actually being synchronized. Without it, the parent directories + # traversed on the way get the attributes of the staging copy, which + # is what forced preservation to be rolled back in the past. output, stderr = await util.check_output( - 'rsync', '-rvLD', targdir + '/', 'root@[{}]:/'.format(targip)) + 'rsync', '-vLpgotDA', '--no-implied-dirs', '--files-from=-', + # content out of confluent's own storage is owned by the account + # the daemon runs as, which normally does not exist on the node + # and would arrive as a meaningless numeric id; every other owner + # is still matched by name as usual + '--usermap={}:root'.format(os.getuid()), + '--groupmap={}:root'.format(os.getgid()), + targdir + '/', 'root@[{}]:/'.format(targip), + input=''.join([x + '\n' for x in filelist]).encode('utf8')) except subprocess.CalledProcessError as e: unreadablefiles = [] for root, dirnames, filenames in os.walk(targdir): @@ -251,6 +272,15 @@ def stage_ent(currmap, ent, targdir, appendexist=False): def mkpathorlink(source, destination, appendexist=False): if os.path.isdir(source): mkdirp(destination) + # Files are staged as symlinks, so rsync reads their attributes + # through to the real file, but a directory is staged as a directory + # and has to be given the source attributes to pass them along. + srcstat = os.stat(source) + shutil.copystat(source, destination) + try: + os.chown(destination, srcstat.st_uid, srcstat.st_gid) + except OSError: # lacking CAP_CHOWN, leave it owned by the daemon + pass for ent in os.listdir(source): currsrc = os.path.join(source, ent) currdst = os.path.join(destination, ent) diff --git a/confluent_server/confluent/util.py b/confluent_server/confluent/util.py index a1a1be07..0f3ae2c8 100644 --- a/confluent_server/confluent/util.py +++ b/confluent_server/confluent/util.py @@ -63,12 +63,13 @@ async def check_call(*cmd, **kwargs): -async def check_output(*cmd): +async def check_output(*cmd, input=None): if len(cmd) == 1 and isinstance(cmd[0], (list, tuple)): cmd = cmd[0] process = await asyncio.create_subprocess_exec( - *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE) - stdout, stderr = await process.communicate() + *cmd, stdin=asyncio.subprocess.PIPE if input is not None else None, + stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE) + stdout, stderr = await process.communicate(input) retcode = process.returncode if retcode: raise subprocess.CalledProcessError(