Skip to content

Preserve attributes in syncfiles without disturbing parent directories - #264

Merged
jjohnson42 merged 1 commit into
xcat2:masterfrom
Obihoernchen:syncfiles-preserve-attrs
Aug 4, 2026
Merged

Preserve attributes in syncfiles without disturbing parent directories#264
jjohnson42 merged 1 commit into
xcat2:masterfrom
Obihoernchen:syncfiles-preserve-attrs

Conversation

@Obihoernchen

@Obihoernchen Obihoernchen commented Aug 4, 2026

Copy link
Copy Markdown
Member

Problem

The syncfiles rsync push carried no preservation flags:

rsync -rvLD <staging>/ root@[node]:/

Without -p, rsync explicitly disables setuid/setgid/sticky on new files, so a syncfile entry could never deliver
a setuid binary, and existing files on the node kept whatever mode they already had.

Why the flags could not just be added back

It was tried once in e52a9ff7
("Have syncfiles attempt to preserve more") and rolled back in c0287e93:
"Unfortunately, it will try to change parent directories on the way to the files actually being written."

The staging tree mirrors the full destination path, so -r walks it as real directory entries and rsync applied
the staging directory's attributes to the real /etc on the node. Naming every staged file through --files-from
plus --no-implied-dirs confines preservation to the content actually being synchronized:

rsync -vLpgotDA --no-implied-dirs --files-from=- \
      --usermap=<confluent uid>:root --groupmap=<confluent gid>:root \
      <staging>/ root@[node]:/

What changed

  • The file list is built in memory and fed to --files-from=- on stdin. Otherwise empty directories are named
    explicitly, since nothing else would pull them across.
  • util.check_output gained a keyword only input=, mirroring subprocess.run. stdin is only piped when
    input is given, so existing callers are unaffected.
  • mkpathorlink copies the source directory's attributes onto the staged copy. Files are staged as symlinks and
    rsync reads through to the real file, but directories are staged as directories.
  • The confluent account's ownership is mapped to root via --usermap/--groupmap, since it normally does not
    exist on the node. Every other owner still matches by name, so this is not a blanket --chown.
  • --xattrs from the earlier attempt is left out: with --copy-links rsync reads xattrs off the symlink, not its
    referent, so it transfers nothing here while adding a failure mode on hosts without xattr support.

Warning

This changes existing syncfiles behaviour. Attributes that used to be left alone are now taken from the
source on every sync:

  • Permissions are asserted every time. Existing files on the node previously kept their own mode, so a local
    chmod on a synced file survived. It is now reverted on the next sync.
  • Ownership comes from the server. Synced files used to land root:root regardless of the source. A file
    owned by a named account now arrives owned by that account, matched by name, falling back to the server's
    numeric id when that name does not exist on the node. Only the confluent account is mapped to root.
  • Timestamps and ACLs are carried over (-t, -A), where previously they were not.

Directories that are only traversed on the way to a synced file remain untouched, which is what the second test
table below covers.

Testing

End to end against a deployed AlmaLinux 10 node, same fixture before and after.

Files and directories delivered by the sync:

synced item path source on server before after
setuid binary /opt/sync/sub/setuidapp 4755 root:root 755 4755 root:root
setgid binary /opt/sync/sub/setgidapp 2755 root:root 755 2755 root:root
existing file /opt/sync/sub/tightmode 0640 root:confluent 666, stale mode kept 640 root:root
confluent owned file /opt/sync/sub/cflowned 0640 confluent:confluent 640 root:root 640 root:root
system user file /opt/sync/sub/daemonowned 0644 daemon:daemon 644 root:root 644 daemon:daemon
empty directory /opt/sync/emptydir 2750 confluent:confluent 700 root:root 2750 root:root

Directories the sync only has to traverse to reach those files. Nothing in the syncfile list refers to them, and
both before and after they must come out exactly as the node had them:

directory node state before sync before after
parent directory (/opt/sync) 700 root:root 700 root:root 700 root:root
parent subdirectory (/opt/sync/sub) 750 daemon:daemon 750 daemon:daemon 750 daemon:daemon

The second table is the point: files were written into the subdirectory, yet neither its mode nor its ownership
moved. In the first table, the system user file keeping daemon:daemon while the confluent owned file became
root:root confirms the ownership mapping is selective.

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 e52a9ff ("Have syncfiles
attempt to preserve more") and rolled back the same day in c0287e9
("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.
@jjohnson42
jjohnson42 merged commit 4564f51 into xcat2:master Aug 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants