Fix owners in many deep file copy operations (fixes tailhook#121)

docs/build_steps.rst

@@ -370,13 +370,22 @@ Files and Directories
    source
      (required) Absolute to directory or file to copy. If path starts with
      ``/work`` files are checksummed to get the version of the container.
+
    path
      (required) Destination path
+
    ignore-regex
      (default ``(^|/)\.(git|hg|svn|vagga)($|/)|~$|\.bak$|\.orig$|^#.*#$``)
      Regular expression of paths to ignore. Default regexp ignores common
      revision control folders and editor backup files.
 
+   owner-uid, owner-gid
+     (preserved by default) Override uid and gid of files and directories when
+     copying. It's expected that most useful case is ``owner-uid: 0`` and
+     ``owner-gid: 0`` but we try to preserve the owner by default. Note that
+     unmapped users (the ones that don't belong to user's subuid/subgid range),
+     will be set to ``nobody`` (65535).
+
    .. warning:: If the source directory starts with `/work` all the files are
       read and checksummed on each run of the application in the container. So
       copying large directories for this case may influence container startup

docs/volumes.rst

@@ -100,12 +100,13 @@ Available volume types:
    Parameters:
 
    size
-     Size of the allocated ``tmpfs`` volume. Including the size of the original
-     contents. This is the limit of how much data you can write on the volume.
+     (default ``100Mi``) Size of the allocated ``tmpfs`` volume. Including the
+     size of the original contents. This is the limit of how much data you can
+     write on the volume.
 
-   owner-uid
-     The user id of the owner of the directory. If not specified the ownership
-     will be copied  from the original
+   owner-uid, owner-gid
+     (default is to preserve) The user id of the owner of the directory. If not
+     specified the ownership will be copied  from the original
 
    Additional properties, like the source directory will be added to the later
    versions of vagga

src/builder/bld.rs

@@ -26,7 +26,7 @@ use builder::distrib::{DistroBox};
 use builder::guard::Guard;
 use builder::error::StepError;
 use path_util::PathExt;
-use file_util::{create_dir, shallow_copy};
+use file_util::{create_dir_mode, shallow_copy};
 
 
 pub trait BuildCommand {
@@ -71,7 +71,8 @@ impl BuildCommand for Builder {
                         .map_err(|(s, e)| format!("step {}: {}", s, e)));
                     let path = Path::new("/vagga/base/.roots")
                         .join(format!("{}.{}", name, version)).join("root");
-                    try_msg!(copy_dir(&path, &Path::new("/vagga/root")),
+                    try_msg!(copy_dir(&path, &Path::new("/vagga/root"),
+                                      None, None),
                         "Error copying dir {p:?}: {err}", p=path);
                 }
             }
@@ -122,7 +123,7 @@ impl BuildCommand for Builder {
                     let typ = try!(symlink_metadata(src)
                         .map_err(|e| E::Write(src.into(), e)));
                     if typ.is_dir() {
-                        try!(create_dir(&dest, false)
+                        try!(create_dir_mode(&dest, typ.permissions().mode())
                             .map_err(|e| E::Write(dest.clone(), e)));
                         let re = try!(Regex::new(&cinfo.ignore_regex));
                         try!(ScanDir::all().walk(src, |iter| {
@@ -137,13 +138,15 @@ impl BuildCommand for Builder {
                                     continue;
                                 }
                                 let fdest = dest.join(path);
-                                try!(shallow_copy(&fpath, &fdest)
+                                try!(shallow_copy(&fpath, &fdest,
+                                        cinfo.owner_uid, cinfo.owner_gid)
                                     .map_err(|e| E::Write(fdest, e)));
                             }
                             Ok(())
                         }).map_err(E::ScanDir).and_then(|x| x));
                     } else {
-                        try!(shallow_copy(&cinfo.source, &dest)
+                        try!(shallow_copy(&cinfo.source, &dest,
+                                          cinfo.owner_uid, cinfo.owner_gid)
                              .map_err(|e| E::Write(dest.clone(), e)));
                     }
                 }

src/config/builders.rs

@@ -3,7 +3,7 @@ use std::default::Default;
 use std::collections::BTreeMap;
 
 use quire::validate as V;
-use libc::uid_t;
+use libc::{uid_t, gid_t};
 
 #[derive(RustcEncodable, RustcDecodable, Debug, Clone)]
 pub struct DebianRepo {
@@ -129,6 +129,7 @@ pub struct CopyInfo {
     pub source: PathBuf,
     pub path: PathBuf,
     pub owner_uid: Option<uid_t>,
+    pub owner_gid: Option<gid_t>,
     pub ignore_regex: String,
 }
 
@@ -237,7 +238,8 @@ pub fn builder_validator<'x>() -> V::Enum<'x> {
         .member("path", V::Directory::new().is_absolute(true))
         .member("ignore_regex", V::Scalar::new().default(
             r#"(^|/)\.(git|hg|svn|vagga)($|/)|~$|\.bak$|\.orig$|^#.*#$"#))
-        .member("owner_uid", V::Numeric::new().min(0).optional()))
+        .member("owner_uid", V::Numeric::new().min(0).optional())
+        .member("owner_gid", V::Numeric::new().min(0).optional()))
 
     .option("Ubuntu", V::Scalar::new())
     .option("UbuntuRelease", V::Structure::new()

src/config/containers.rs

@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
 use std::path::PathBuf;
 
 use quire::validate as V;
-use libc::uid_t;
+use libc::{uid_t, gid_t};
 
 use super::builders::{Builder, builder_validator};
 use super::Range;
@@ -12,6 +12,7 @@ use super::Range;
 pub struct SnapshotInfo {
     pub size: usize,
     pub owner_uid: Option<uid_t>,
+    pub owner_gid: Option<gid_t>,
 }
 
 #[derive(RustcDecodable, Clone, PartialEq, Eq)]
@@ -72,6 +73,7 @@ pub fn volume_validator<'x>() -> V::Enum<'x> {
     .option("Snapshot",  V::Structure::new()
         .member("size",  V::Numeric::new().min(0).default(100*1024*1024))
         .member("owner_uid", V::Numeric::new().min(0).optional())
+        .member("owner_gid", V::Numeric::new().min(0).optional())
         )
 }
 

src/container/util.rs

@@ -5,10 +5,10 @@ use std::os::unix::fs::{symlink, MetadataExt};
 use std::ptr::null;
 use std::path::{Path, PathBuf};
 
-use libc::{c_int, c_void, timeval};
+use libc::{uid_t, gid_t, c_int, c_void, timeval};
 
 use super::root::temporary_change_root;
-use file_util::{create_dir_mode, copy};
+use file_util::{create_dir_mode, shallow_copy, set_owner_group};
 use path_util::PathExt;
 
 quick_error!{
@@ -96,7 +96,10 @@ pub fn get_time() -> Time {
     return tv.tv_sec as f64 + 0.000001 * tv.tv_usec as f64;
 }
 
-pub fn copy_dir(old: &Path, new: &Path) -> Result<(), CopyDirError> {
+pub fn copy_dir(old: &Path, new: &Path,
+    owner_uid: Option<uid_t>, owner_gid: Option<gid_t>)
+    -> Result<(), CopyDirError>
+{
     use self::CopyDirError::*;
     // TODO(tailhook) use reflinks if supported
     let dir = try!(read_dir(old).map_err(|e| ReadDir(old.to_path_buf(), e)));
@@ -110,30 +113,14 @@ pub fn copy_dir(old: &Path, new: &Path) -> Result<(), CopyDirError> {
             oldp.push(&filename);
             newp.push(&filename);
 
-            let typ = try!(entry.file_type()
-                .map_err(|e| Stat(oldp.clone(), e)));
-            if typ.is_file() {
-                try!(copy(&oldp, &newp)
-                    .map_err(|e| CopyFile(oldp.clone(), newp.clone(), e)));
-            } else if typ.is_dir() {
-                let stat = try!(symlink_metadata(&oldp)
-                    .map_err(|e| Stat(oldp.clone(), e)));
-                if !newp.is_dir() {
-                    try!(create_dir_mode(&newp, stat.mode())
-                        .map_err(|e| CreateDir(newp.clone(), e)));
-                }
+            let copy_rc = try!(shallow_copy(&oldp, &newp, owner_uid, owner_gid)
+                .map_err(|e| CopyFile(oldp.clone(), newp.clone(), e)));
+            if !copy_rc {
                 stack.push(dir);  // Return dir to stack
                 let ndir = try!(read_dir(&oldp)
                     .map_err(|e| ReadDir(oldp.to_path_buf(), e)));
                 stack.push(ndir); // Add new dir to the stack too
                 continue 'next_dir;
-            } else if typ.is_symlink() {
-                let lnk = try!(read_link(&oldp)
-                               .map_err(|e| ReadLink(oldp.clone(), e)));
-                try!(symlink(&lnk, &newp)
-                    .map_err(|e| Symlink(newp.clone(), e)));
-            } else {
-                warn!("Unknown file type {:?}", &entry.path());
             }
             oldp.pop();
             newp.pop();
@@ -169,6 +156,8 @@ pub fn hardlink_dir(old: &Path, new: &Path) -> Result<(), CopyDirError> {
                 if !newp.is_dir() {
                     try!(create_dir_mode(&newp, stat.mode())
                         .map_err(|e| CreateDir(newp.clone(), e)));
+                    try!(set_owner_group(&newp, stat.uid(), stat.gid())
+                        .map_err(|e| CreateDir(newp.clone(), e)));
                 }
                 stack.push(dir);  // Return dir to stack
                 let ndir = try!(read_dir(&oldp)

src/file_util.rs

@@ -2,16 +2,22 @@ use std::io;
 use std::io::{Read, Write, Error};
 use std::path::{Path, PathBuf};
 use std::fs;
-use std::os::unix::fs::{PermissionsExt, symlink};
+use std::ffi::CString;
+use std::os::unix::fs::{PermissionsExt, MetadataExt, symlink};
 use std::os::unix::io::AsRawFd;
 use std::os::unix::ffi::OsStrExt;
 
 use nix;
+use libc::{uid_t, gid_t, c_int};
 use shaman::digest::Digest;
 use nix::fcntl::{flock, FlockArg};
 
 use path_util::PathExt;
 
+extern "C" {
+    fn lchown(path: *const i8, owner: uid_t, group: gid_t) -> c_int;
+}
+
 pub struct Lock {
     file: fs::File,
 }
@@ -152,11 +158,17 @@ impl Drop for Lock {
     }
 }
 
-pub fn hash_file(path: &Path, dig: &mut Digest)
+pub fn hash_file(path: &Path, dig: &mut Digest,
+    owner_uid: Option<uid_t>, owner_gid: Option<gid_t>)
     -> Result<(), io::Error>
 {
     // TODO(tailhook) include permissions and ownership into the equation
     let stat = try!(fs::symlink_metadata(path));
+    dig.input(path.as_os_str().as_bytes());
+    dig.input(b"\0");
+    dig.input(format!("mode:{:o}\0", stat.permissions().mode()).as_bytes());
+    dig.input(format!("uid:{}\0", owner_uid.unwrap_or(stat.uid())).as_bytes());
+    dig.input(format!("gid:{}\0", owner_gid.unwrap_or(stat.gid())).as_bytes());
     if stat.file_type().is_symlink() {
         let data = try!(fs::read_link(path));
         dig.input(data.as_os_str().as_bytes());
@@ -175,7 +187,7 @@ pub fn hash_file(path: &Path, dig: &mut Digest)
     Ok(())
 }
 
-pub fn force_symlink(target: &Path, linkpath: &Path, perm: fs::Permissions)
+pub fn force_symlink(target: &Path, linkpath: &Path)
     -> Result<(), io::Error>
 {
     let tmpname = linkpath.with_extension(".vagga.tmp.link~");
@@ -184,16 +196,63 @@ pub fn force_symlink(target: &Path, linkpath: &Path, perm: fs::Permissions)
     Ok(())
 }
 
-pub fn shallow_copy(src: &Path, dest: &Path) -> Result<(), io::Error> {
+pub fn set_owner_group(target: &Path, uid: uid_t, gid: gid_t)
+    -> Result<(), io::Error>
+{
+    let rc = unsafe { lchown(
+        CString::new(target.as_os_str().as_bytes()).unwrap().as_ptr(),
+        uid, gid) };
+    if rc != 0 {
+        warn!("Can't chown {:?}: {}", target, io::Error::last_os_error());
+        Ok(())
+        // Err(io::Error::last_os_error())
+    } else {
+        Ok(())
+    }
+}
+
+/// Shallow copy of file/dir/symlink
+///
+/// The owner_uid/owner_gid parameters optionally override the owner
+///
+/// Returns false if path is a directory
+pub fn shallow_copy(src: &Path, dest: &Path,
+    owner_uid: Option<uid_t>, owner_gid: Option<gid_t>)
+    -> Result<bool, io::Error>
+{
     let stat = try!(fs::symlink_metadata(src));
     let typ = stat.file_type();
+    let mut is_dir = false;
     if typ.is_dir() {
-        try!(create_dir_mode(dest, stat.permissions().mode()));
+        is_dir = true;
+        let nstat = fs::symlink_metadata(dest);
+        // We don't change permissions and owner of already created directories
+        //
+        // There are couple of reasons:
+        // 1. We consider such overlaps a "merge" of a directory, so it's
+        //    unclear which one should override the permissions
+        // 2. Changing permissions of the root directory of the copied system
+        //    is usually counter-intuitive (so we contradict rsync)
+        // 3. Some directories (/proc, /sys, /dev) are mount points and we
+        //    can't change the permissions
+        if nstat.is_err() {
+            try!(fs::create_dir(dest));
+            try!(fs::set_permissions(dest, stat.permissions()));
+            try!(set_owner_group(dest,
+                owner_uid.unwrap_or(stat.uid()),
+                owner_gid.unwrap_or(stat.gid())));
+        }
     } else if typ.is_symlink() {
         let value = try!(fs::read_link(&src));
-        try!(force_symlink(&value, dest, stat.permissions()));
+        try!(force_symlink(&value, dest));
+        try!(set_owner_group(dest,
+            owner_uid.unwrap_or(stat.uid()),
+            owner_gid.unwrap_or(stat.gid())));
     } else {
         try!(copy(src, dest));
+        try!(set_owner_group(dest,
+            owner_uid.unwrap_or(stat.uid()),
+            owner_gid.unwrap_or(stat.gid())));
     }
-    Ok(())
+    Ok(!is_dir)
 }

src/version/version.rs

@@ -140,7 +140,7 @@ impl VersionHash for Builder {
             }
             &B::Depends(ref filename) => {
                 let path = Path::new("/work").join(filename);
-                hash_file(&path, hash)
+                hash_file(&path, hash, None, None)
                     .map_err(|e| Error::Io(e, path.clone()))
             }
             &B::Container(ref name) => {
@@ -228,13 +228,15 @@ impl VersionHash for Builder {
                                 hash.input(b"\0");
                                 hash.input(name.as_bytes());
                                 hash.input(b"\0");
-                                try!(hash_file(&fpath, hash)
+                                try!(hash_file(&fpath, hash,
+                                        cinfo.owner_uid, cinfo.owner_gid)
                                     .map_err(|e| Error::Io(e, fpath)));
                             }
                             Ok(())
                         }).map_err(Error::ScanDir).and_then(|x| x));
                     } else {
-                        try!(hash_file(src, hash)
+                        try!(hash_file(src, hash,
+                                cinfo.owner_uid, cinfo.owner_gid)
                             .map_err(|e| Error::Io(e, src.into())));
                     }
                 } else {

src/wrapper/snapshot.rs

@@ -15,7 +15,7 @@ pub fn make_snapshot(dest: &Path, info: &SnapshotInfo)
     try!(bind_mount(&dest, &tmp));
     try!(mount_tmpfs(&dest,
         &format!("size={},mode=0755", info.size)));
-    try_msg!(copy_dir(&tmp, dest),
+    try_msg!(copy_dir(&tmp, dest, info.owner_uid, info.owner_gid),
         "Error copying directory: {err}");
     try!(unmount(tmp));
     Ok(())

tests/copy.bats

@@ -9,7 +9,7 @@ setup() {
     [[ $status = 0 ]]
     [[ ${lines[@]} = "world file sub" ]]
     link=$(readlink .vagga/dir-copy)
-    [[ $link = ".roots/dir-copy.6147e496/root" ]]
+    [[ $link = ".roots/dir-copy.789b58e2/root" ]]
 }
 
 @test "copy: file" {
@@ -19,6 +19,6 @@ setup() {
     [[ $status = 0 ]]
     [[ ${lines[@]} = "data" ]]
     link=$(readlink .vagga/file-copy)
-    [[ $link = ".roots/file-copy.cfbbcc99/root" ]]
+    [[ $link = ".roots/file-copy.3cf34a1f/root" ]]
 }