launchpad-reviewers team mailing list archive

[Andrey Fedoseev <mp+433636@xxxxxxxxxxxxxxxxxx>]

Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad-buildd:backend-open into launchpad-buildd:master.

Commit message:
Fix the bug with `Backend.open()` caused by file ownership in chroot environment

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad-buildd/+git/launchpad-buildd/+merge/433636

Previously, `Chroot.copy_out()` produced a file owned by root which prevented `Backend.open()` to open it for writing.

Now, the file created by `Chroot.copy_out()` is owned by `buildd`
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad-buildd:backend-open into launchpad-buildd:master.

diff --git a/lpbuildd/target/chroot.py b/lpbuildd/target/chroot.py
index 11752c8..80ad1b3 100644
--- a/lpbuildd/target/chroot.py
+++ b/lpbuildd/target/chroot.py
@@ -103,13 +103,17 @@ class Chroot(Backend):
              source_path, full_target_path])
 
     def copy_out(self, source_path, target_path):
-        # We can just use a plain copy here, since the file ownership in the
-        # host system isn't important.
+        # Don't use install(1) here because running `os.stat` to get file mode
+        # may be impossible. Instead, copy the with `cp` and set file ownership
+        # to buildd (this is necessary so that buildd can read/write the copied
+        # file).
         full_source_path = os.path.join(
             self.chroot_path, source_path.lstrip("/"))
         subprocess.check_call(
             ["sudo", "cp", "--preserve=timestamps",
              full_source_path, target_path])
+        uid, gid = os.getuid(), os.getgid()
+        subprocess.check_call(["sudo", "chown", f"{uid}:{gid}", target_path])
 
     def kill_processes(self):
         """See `Backend`."""
diff --git a/lpbuildd/target/tests/test_chroot.py b/lpbuildd/target/tests/test_chroot.py
index a8971d3..8f2fd38 100644
--- a/lpbuildd/target/tests/test_chroot.py
+++ b/lpbuildd/target/tests/test_chroot.py
@@ -181,10 +181,12 @@ class TestChroot(TestCase):
         Chroot("1", "xenial", "amd64").copy_out(
             "/path/to/source", "/path/to/target")
 
+        uid, gid = os.getuid(), os.getgid()
         expected_args = [
             ["sudo", "cp", "--preserve=timestamps",
              "/expected/home/build-1/chroot-autobuild/path/to/source",
              "/path/to/target"],
+            ["sudo", "chown", f"{uid}:{gid}", "/path/to/target"],
             ]
         self.assertEqual(
             expected_args,