Skip to content

Commit

Permalink
Detect directory entry modifications to uid/gid (cvmfs#3473)
Browse files Browse the repository at this point in the history 
From issue cvmfs#3435 :
>`DirectoryEntryBase::CompareTo` will miss isolated changes to uid, gid, xattrs, chunks. This isn't an issue usually, as >such changes when made via an overlayfs transaction will also change the mtime. It causes a problem if the changes >are being made programmatically.
>It also results in changed behaviour when publishing via a gateway vs not.

uid/gid changes are now properly handled.

Changes to individual extended attributes or to the chunking layout are more difficult to handle because they require a follow-up lookup in the corresponding catalog tables, to be done in a future patch.
---------

Co-authored-by: Valentin Volkl <[email protected]>
  • Loading branch information
@jblomer @vvolkl
jblomer and vvolkl authored Nov 22, 2024
1 parent 4a1c1b5 commit 520530d
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 17 deletions.
6 changes: 6 additions & 0 deletions cvmfs/directory_entry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ DirectoryEntryBase::Differences DirectoryEntryBase::CompareTo(
if (HasXattrs() != other.HasXattrs()) {
result |= Difference::kHasXattrsFlag;
}
if (uid() != other.uid()) {
result |= Difference::kUid;
}
if (gid() != other.gid()) {
result |= Difference::kGid;
}

return result;
}
Expand Down
37 changes: 20 additions & 17 deletions cvmfs/directory_entry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,25 +77,28 @@ class DirectoryEntryBase {

/**
* Used in the swissknife for sanity checks and catalog migration. If
* anything is added, also adjust PrintDifferences in swissknife::CommandDiff.
* anything is added, also adjust PrintDifferences in swissknife::CommandDiff
* and CommandCheck::CompareEntries
*/
struct Difference {
static const unsigned int kIdentical = 0x000;
static const unsigned int kName = 0x001;
static const unsigned int kLinkcount = 0x002;
static const unsigned int kSize = 0x004;
static const unsigned int kMode = 0x008;
static const unsigned int kMtime = 0x010;
static const unsigned int kSymlink = 0x020;
static const unsigned int kChecksum = 0x040;
static const unsigned int kHardlinkGroup = 0x080;
static const unsigned int kNestedCatalogTransitionFlags = 0x100;
static const unsigned int kChunkedFileFlag = 0x200;
static const unsigned int kHasXattrsFlag = 0x400;
static const unsigned int kExternalFileFlag = 0x800;
static const unsigned int kBindMountpointFlag = 0x1000;
static const unsigned int kHiddenFlag = 0x2000;
static const unsigned int kDirectIoFlag = 0x4000;
static const unsigned int kIdentical = 0x00000;
static const unsigned int kName = 0x00001;
static const unsigned int kLinkcount = 0x00002;
static const unsigned int kSize = 0x00004;
static const unsigned int kMode = 0x00008;
static const unsigned int kMtime = 0x00010;
static const unsigned int kSymlink = 0x00020;
static const unsigned int kChecksum = 0x00040;
static const unsigned int kHardlinkGroup = 0x00080;
static const unsigned int kNestedCatalogTransitionFlags = 0x00100;
static const unsigned int kChunkedFileFlag = 0x00200;
static const unsigned int kHasXattrsFlag = 0x00400;
static const unsigned int kExternalFileFlag = 0x00800;
static const unsigned int kBindMountpointFlag = 0x01000;
static const unsigned int kHiddenFlag = 0x02000;
static const unsigned int kDirectIoFlag = 0x04000;
static const unsigned int kUid = 0x08000;
static const unsigned int kGid = 0x10000;
};
typedef unsigned int Differences;

Expand Down
4 changes: 4 additions & 0 deletions cvmfs/publish/cmd_diff.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ class DiffReporter : public publish::DiffListener {
result_list.push_back(machine_readable_ ? "H" : "hidden");
if (diff & catalog::DirectoryEntryBase::Difference::kDirectIoFlag)
result_list.push_back(machine_readable_ ? "D" : "direct-io");
if (diff & catalog::DirectoryEntryBase::Difference::kUid)
result_list.push_back(machine_readable_ ? "U" : "uid");
if (diff & catalog::DirectoryEntryBase::Difference::kGid)
result_list.push_back(machine_readable_ ? "R" : "gid");

return machine_readable_ ? ("[" + JoinStrings(result_list, "") + "]")
: (" [" + JoinStrings(result_list, ", ") + "]");
Expand Down
12 changes: 12 additions & 0 deletions cvmfs/swissknife_check.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,18 @@ bool CommandCheck::CompareEntries(const catalog::DirectoryEntry &a,
a.name().c_str(), b.name().c_str());
retval = false;
}
if (!is_transition_point) {
if (diffs & Difference::kUid) {
LogCvmfs(kLogCvmfs, kLogStderr, "uids differ: %d / %d (%s / %s)",
a.uid(), b.uid(), a.name().c_str(), b.name().c_str());
retval = false;
}
if (diffs & Difference::kGid) {
LogCvmfs(kLogCvmfs, kLogStderr, "gids differ: %d / %d (%s / %s)",
a.gid(), b.gid(), a.name().c_str(), b.name().c_str());
retval = false;
}
}

return retval;
}
Expand Down
4 changes: 4 additions & 0 deletions test/src/641-diff/main
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ create_repository_content() {
touch directory/nested/.cvmfscatalog
touch directory/foo
mkdir dir2
touch owner

popdir
}
Expand All @@ -35,6 +36,7 @@ change_repository_content() {
mkdir dir3
touch dir3/new_file
touch dir3/.cvmfscatalog
sudo chown 99:99 owner

popdir
}
Expand Down Expand Up @@ -85,6 +87,7 @@ A D /dir3
A F /dir3/new_file +0
A F /dir3/.cvmfscatalog +0
M[SC] F /file
M[UR] F /owner
R S /symlink -4
M[SMLC] FS /directory/foo
M[N] D /directory/nested
Expand All @@ -104,6 +107,7 @@ M[N] D /dir2
R F /dir2/.cvmfscatalog -0
R D /dir3
M[SC] F /file
M[UR] F /owner
A S /symlink +4
M[SMLC] SF /directory/foo
M[N] D /directory/nested
Expand Down

0 comments on commit 520530d

Please sign in to comment.