libstore/build: just copy the magic /etc files into the sandbox

Saves us a bunch of thinking about how to handle symlinks, and prevents
the DNS config from changing on the fly under the build, which may or may
not be a good thing?

Change-Id: I071e6ae7e220884690b788d94f480866f428db71
This commit is contained in:
Ilya K 2024-04-12 12:29:10 +03:00
parent d363bc2f12
commit b469c6509b
5 changed files with 79 additions and 13 deletions

View file

@ -410,7 +410,7 @@ static void doBind(const Path & source, const Path & target, bool optional = fal
} else if (S_ISLNK(st.st_mode)) { } else if (S_ISLNK(st.st_mode)) {
// Symlinks can (apparently) not be bind-mounted, so just copy it // Symlinks can (apparently) not be bind-mounted, so just copy it
createDirs(dirOf(target)); createDirs(dirOf(target));
copyFile(source, target, /* andDelete */ false); copyFile(source, target, {});
} else { } else {
createDirs(dirOf(target)); createDirs(dirOf(target));
writeFile(target, ""); writeFile(target, "");
@ -1811,8 +1811,25 @@ void LocalDerivationGoal::runChild()
happens when testing Nix building fixed-output derivations happens when testing Nix building fixed-output derivations
within a pure derivation. */ within a pure derivation. */
for (auto & path : { "/etc/resolv.conf", "/etc/services", "/etc/hosts" }) for (auto & path : { "/etc/resolv.conf", "/etc/services", "/etc/hosts" })
if (pathExists(path)) if (pathExists(path)) {
ss.push_back(path); // Copy the actual file, not the symlink, because we don't know where
// the symlink is pointing, and we don't want to chase down the entire
// chain.
//
// This means if your network config changes during a FOD build,
// the DNS in the sandbox will be wrong. However, this is pretty unlikely
// to actually be a problem, because FODs are generally pretty fast,
// and machines with often-changing network configurations probably
// want to run resolved or some other local resolver anyway.
//
// There's also just no simple way to do this correctly, you have to manually
// inotify watch the files for changes on the outside and update the sandbox
// while the build is running (or at least that's what Flatpak does).
//
// I also just generally feel icky about modifying sandbox state under a build,
// even though it really shouldn't be a big deal. -K900
copyFile(path, chrootRootDir + path, { .followSymlinks = true });
}
if (settings.caFile != "") if (settings.caFile != "")
pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true); pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true);
@ -2542,7 +2559,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
// that there's no stale file descriptor pointing to it // that there's no stale file descriptor pointing to it
Path tmpOutput = actualPath + ".tmp"; Path tmpOutput = actualPath + ".tmp";
movePath(actualPath, tmpOutput); movePath(actualPath, tmpOutput);
copyFile(tmpOutput, actualPath, true); copyFile(tmpOutput, actualPath, { .deleteAfter = true });
auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating { auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating {
.method = dof.ca.method, .method = dof.ca.method,

View file

@ -103,31 +103,37 @@ void setWriteTime(const fs::path & p, const struct stat & st)
throw SysError("changing modification time of '%s'", p); throw SysError("changing modification time of '%s'", p);
} }
void copy(const fs::directory_entry & from, const fs::path & to, bool andDelete) void copy(const fs::directory_entry & from, const fs::path & to, CopyFileFlags flags)
{ {
// TODO: Rewrite the `is_*` to use `symlink_status()` // TODO: Rewrite the `is_*` to use `symlink_status()`
auto statOfFrom = lstat(from.path().c_str()); auto statOfFrom = lstat(from.path().c_str());
auto fromStatus = from.symlink_status(); auto fromStatus = from.symlink_status();
// Mark the directory as writable so that we can delete its children // Mark the directory as writable so that we can delete its children
if (andDelete && fs::is_directory(fromStatus)) { if (flags.deleteAfter && fs::is_directory(fromStatus)) {
fs::permissions(from.path(), fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow); fs::permissions(from.path(), fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow);
} }
if (fs::is_symlink(fromStatus) || fs::is_regular_file(fromStatus)) { if (fs::is_symlink(fromStatus) || fs::is_regular_file(fromStatus)) {
fs::copy(from.path(), to, fs::copy_options::copy_symlinks | fs::copy_options::overwrite_existing); auto opts = fs::copy_options::overwrite_existing;
if (!flags.followSymlinks) {
opts |= fs::copy_options::copy_symlinks;
}
fs::copy(from.path(), to, opts);
} else if (fs::is_directory(fromStatus)) { } else if (fs::is_directory(fromStatus)) {
fs::create_directory(to); fs::create_directory(to);
for (auto & entry : fs::directory_iterator(from.path())) { for (auto & entry : fs::directory_iterator(from.path())) {
copy(entry, to / entry.path().filename(), andDelete); copy(entry, to / entry.path().filename(), flags);
} }
} else { } else {
throw Error("file '%s' has an unsupported type", from.path()); throw Error("file '%s' has an unsupported type", from.path());
} }
setWriteTime(to, statOfFrom); setWriteTime(to, statOfFrom);
if (andDelete) { if (flags.deleteAfter) {
if (!fs::is_symlink(fromStatus)) if (!fs::is_symlink(fromStatus))
fs::permissions(from.path(), fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow); fs::permissions(from.path(), fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow);
fs::remove(from.path()); fs::remove(from.path());
@ -135,9 +141,9 @@ void copy(const fs::directory_entry & from, const fs::path & to, bool andDelete)
} }
void copyFile(const Path & oldPath, const Path & newPath, bool andDelete) void copyFile(const Path & oldPath, const Path & newPath, CopyFileFlags flags)
{ {
return copy(fs::directory_entry(fs::path(oldPath)), fs::path(newPath), andDelete); return copy(fs::directory_entry(fs::path(oldPath)), fs::path(newPath), flags);
} }
void renameFile(const Path & oldName, const Path & newName) void renameFile(const Path & oldName, const Path & newName)
@ -160,7 +166,7 @@ void moveFile(const Path & oldName, const Path & newName)
if (e.code().value() == EXDEV) { if (e.code().value() == EXDEV) {
fs::remove(newPath); fs::remove(newPath);
warn("Cant rename %s as %s, copying instead", oldName, newName); warn("Cant rename %s as %s, copying instead", oldName, newName);
copy(fs::directory_entry(oldPath), tempCopyTarget, true); copy(fs::directory_entry(oldPath), tempCopyTarget, { .deleteAfter = true });
renameFile(tempCopyTarget, newPath); renameFile(tempCopyTarget, newPath);
} }
} }

View file

@ -279,13 +279,26 @@ void renameFile(const Path & src, const Path & dst);
*/ */
void moveFile(const Path & src, const Path & dst); void moveFile(const Path & src, const Path & dst);
struct CopyFileFlags
{
/**
* Delete the file after copying.
*/
bool deleteAfter = false;
/**
* Follow symlinks and copy the eventual target.
*/
bool followSymlinks = false;
};
/** /**
* Recursively copy the content of `oldPath` to `newPath`. If `andDelete` is * Recursively copy the content of `oldPath` to `newPath`. If `andDelete` is
* `true`, then also remove `oldPath` (making this equivalent to `moveFile`, but * `true`, then also remove `oldPath` (making this equivalent to `moveFile`, but
* with the guaranty that the destination will be fresh, with no stale inode * with the guaranty that the destination will be fresh, with no stale inode
* or file descriptor pointing to it). * or file descriptor pointing to it).
*/ */
void copyFile(const Path & oldPath, const Path & newPath, bool andDelete); void copyFile(const Path & oldPath, const Path & newPath, CopyFileFlags flags);
/** /**
* Wrappers arount read()/write() that read/write exactly the * Wrappers arount read()/write() that read/write exactly the

View file

@ -158,4 +158,6 @@ in
ca-fd-leak = runNixOSTestFor "x86_64-linux" ./ca-fd-leak; ca-fd-leak = runNixOSTestFor "x86_64-linux" ./ca-fd-leak;
fetch-git = runNixOSTestFor "x86_64-linux" ./fetch-git; fetch-git = runNixOSTestFor "x86_64-linux" ./fetch-git;
symlinkResolvconf = runNixOSTestFor "x86_64-linux" ./symlink-resolvconf.nix;
} }

View file

@ -0,0 +1,28 @@
{ pkgs, ... }:
let
checkResolvconfInSandbox = pkgs.runCommand "resolvconf-works-in-sandbox" {
# must be an FOD to have a resolv.conf in the first place
outputHash = "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=";
outputHashAlgo = "sha256";
outputHashType = "flat";
} ''
cat /etc/resolv.conf
touch $out
'';
in {
name = "symlink-resolvconf";
nodes.machine = {
# Enabling resolved makes /etc/resolv.conf a symlink to /etc/static/resolv.conf, which is itself a symlink to /run.
# If this works, most other things probably will too.
services.resolved.enable = true;
virtualisation.additionalPaths = [checkResolvconfInSandbox.drvPath];
};
testScript = { nodes }: ''
start_all()
machine.succeed('nix-build --check ${checkResolvconfInSandbox.drvPath}')
'';
}