launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/add-trusted-keys-python into lp:launchpad-buildd with lp:~cjwatson/launchpad-buildd/override-sources-list-python as a prerequisite.

Commit message:
Rewrite add-trusted-keys in Python, allowing it to have unit tests.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/add-trusted-keys-python/+merge/328264
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/add-trusted-keys-python into lp:launchpad-buildd.

=== modified file 'bin/add-trusted-keys'
--- bin/add-trusted-keys	2017-07-25 22:11:19 +0000
+++ bin/add-trusted-keys	2017-07-28 23:16:43 +0000
@@ -1,24 +1,35 @@
-#!/bin/sh
+#! /usr/bin/python -u
 #
 # Copyright 2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# Write out new trusted keys in the chroot.
-
-# Expects the build id as the first argument, and key material on stdin.
-
-set -e
-
-SUDO=/usr/bin/sudo
-CHROOT=/usr/sbin/chroot
-
-BUILDID="$1"
-ROOT="$HOME/build-$BUILDID/chroot-autobuild"
-
-cd $HOME
-cd "$ROOT/etc/apt"
-
-echo "Adding trusted keys to build-$BUILDID"
-
-$SUDO $CHROOT $ROOT apt-key add -
-$SUDO $CHROOT $ROOT apt-key list
+"""Write out new trusted keys in the chroot.
+
+Expects key material on stdin.
+"""
+
+from __future__ import print_function
+
+__metaclass__ = type
+
+from argparse import ArgumentParser
+import sys
+
+from lpbuildd.target.chroot import ChrootSetup
+
+
+def main():
+    parser = ArgumentParser(
+        description="Write out new trusted keys in the chroot.")
+    parser.add_argument(
+        "build_id", metavar="ID", help="write trusted keys for build ID")
+    args = parser.parse_args()
+
+    print("Adding trusted keys to build-%s" % args.build_id)
+    setup = ChrootSetup(args.build_id)
+    setup.add_trusted_keys(sys.stdin)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

=== modified file 'debian/changelog'
--- debian/changelog	2017-07-28 23:16:43 +0000
+++ debian/changelog	2017-07-28 23:16:43 +0000
@@ -11,6 +11,7 @@
   * Rewrite update-debian-chroot in Python, allowing it to use lpbuildd.util
     and to have unit tests.
   * Rewrite override-sources-list in Python, allowing it to have unit tests.
+  * Rewrite add-trusted-keys in Python, allowing it to have unit tests.
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 25 Jul 2017 23:07:58 +0100
 

=== modified file 'lpbuildd/target/chroot.py'
--- lpbuildd/target/chroot.py	2017-07-28 23:16:43 +0000
+++ lpbuildd/target/chroot.py	2017-07-28 23:16:43 +0000
@@ -39,6 +39,8 @@
         if self.arch is not None:
             args = set_personality(args, self.arch, series=self.series)
         cmd = ["/usr/bin/sudo", "/usr/sbin/chroot", self.chroot_path] + args
+        if "cwd" not in kwargs:
+            kwargs["cwd"] = self.chroot_path
         if input_text is None:
             subprocess.check_call(cmd, **kwargs)
         else:
@@ -91,3 +93,8 @@
                 print(archive, file=sources_list)
             sources_list.flush()
             self.insert_file(sources_list.name, "/etc/apt/sources.list")
+
+    def add_trusted_keys(self, input_file):
+        """Add trusted keys from an input file."""
+        self.chroot(["apt-key", "add", "-"], stdin=input_file)
+        self.chroot(["apt-key", "list"])

=== modified file 'lpbuildd/target/tests/test_chroot.py'
--- lpbuildd/target/tests/test_chroot.py	2017-07-28 23:16:43 +0000
+++ lpbuildd/target/tests/test_chroot.py	2017-07-28 23:16:43 +0000
@@ -3,6 +3,7 @@
 
 __metaclass__ = type
 
+import io
 import sys
 import time
 from textwrap import dedent
@@ -154,3 +155,19 @@
         self.assertEqual(
             ("/etc/apt/sources.list",),
             mock_insert_file.extract_args()[0][1:])
+
+    def test_add_trusted_keys(self):
+        self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
+        setup = ChrootSetup("1")
+        # XXX cjwatson 2017-07-29: With a newer version of fixtures we could
+        # mock this at the subprocess level instead, but at the moment doing
+        # that wouldn't allow us to test stdin.
+        mock_chroot = self.useFixture(MockPatchObject(setup, "chroot")).mock
+        input_file = io.BytesIO()
+        setup.add_trusted_keys(input_file)
+
+        self.assertEqual(2, len(mock_chroot.mock_calls))
+        mock_chroot.assert_has_calls([
+            ((["apt-key", "add", "-"],), {"stdin": input_file}),
+            ((["apt-key", "list"],), {}),
+            ])