launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-tools-fingerprint into lp:launchpad.

Commit message:
Allow configuring a fingerprint for the archive given in snappy.tools_source.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1626739 in Launchpad itself: "Snapcraft build failing in Yakkety for unauthenticated stage-packages"
  https://bugs.launchpad.net/launchpad/+bug/1626739

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-tools-fingerprint/+merge/325620
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-tools-fingerprint into lp:launchpad.

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2017-06-01 12:05:30 +0000
+++ lib/lp/services/config/schema-lazr.conf	2017-06-14 08:12:16 +0000
@@ -1804,6 +1804,11 @@
 # datatype: string
 tools_source: none
 
+# Optional OpenPGP signing key fingerprint for the archive given in
+# tools_source.
+# datatype: string
+tools_fingerprint: none
+
 # The store's primary URL endpoint.
 store_url: none
 

=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py	2017-06-13 17:08:16 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py	2017-06-14 08:12:16 +0000
@@ -69,7 +69,7 @@
 
     def setUp(self):
         super(TestSnapBuildBehaviourBase, self).setUp()
-        self.pushConfig("snappy", tools_source=None)
+        self.pushConfig("snappy", tools_source=None, tools_fingerprint=None)
 
     def makeJob(self, archive=None, pocket=PackagePublishingPocket.UPDATES,
                 **kwargs):

=== modified file 'lib/lp/soyuz/adapters/archivedependencies.py'
--- lib/lp/soyuz/adapters/archivedependencies.py	2017-04-29 23:51:28 +0000
+++ lib/lp/soyuz/adapters/archivedependencies.py	2017-06-14 08:12:16 +0000
@@ -153,7 +153,8 @@
 
 
 def expand_dependencies(archive, distro_arch_series, pocket, component,
-                        source_package_name, tools_source=None, logger=None):
+                        source_package_name, tools_source=None,
+                        tools_fingerprint=None, logger=None):
     """Return the set of dependency archives, pockets and components.
 
     :param archive: the context `IArchive`.
@@ -164,6 +165,8 @@
     :param tools_source: if not None, a sources.list entry to use as an
         additional dependency for build tools, just before the default
         primary archive.
+    :param tools_fingerprint: if not None, the OpenPGP signing key
+        fingerprint for the archive given in `tools_source`.
     :param logger: an optional logger.
     :return: a list of (archive, distro_arch_series, pocket, [component]),
         representing the dependencies defined by the given build context.
@@ -201,7 +204,9 @@
     # Consider build tools archive dependencies.
     if tools_source is not None:
         try:
-            deps.append(tools_source % {'series': distro_series.name})
+            deps.append(
+                (tools_source % {'series': distro_series.name},
+                 tools_fingerprint))
         except Exception:
             # Someone messed up the configuration; don't add it.
             if logger is not None:
@@ -240,7 +245,8 @@
 
 @defer.inlineCallbacks
 def get_sources_list_for_building(build, distroarchseries, sourcepackagename,
-                                  tools_source=None, logger=None):
+                                  tools_source=None, tools_fingerprint=None,
+                                  logger=None):
     """Return sources.list entries and keys required to build the given item.
 
     The sources.list entries are returned in the order that is most useful:
@@ -257,6 +263,8 @@
     :param tools_source: if not None, a sources.list entry to use as an
         additional dependency for build tools, just before the default
         primary archive.
+    :param tools_fingerprint: if not None, the OpenPGP signing key
+        fingerprint for the archive given in `tools_source`.
     :param logger: an optional logger.
     :return: a Deferred resolving to a tuple containing a list of deb
         sources.list entries (lines) and a list of base64-encoded public
@@ -265,7 +273,8 @@
     deps = expand_dependencies(
         build.archive, distroarchseries, build.pocket,
         build.current_component, sourcepackagename,
-        tools_source=tools_source, logger=logger)
+        tools_source=tools_source, tools_fingerprint=tools_fingerprint,
+        logger=logger)
     sources_list_lines, trusted_keys = (
         yield _get_sources_list_for_dependencies(deps, logger=logger))
 
@@ -360,8 +369,9 @@
     # interaction.
     gpghandler = removeSecurityProxy(getUtility(IGPGHandler))
     for dep in dependencies:
-        if isinstance(dep, basestring):
-            sources_list_lines.append(dep)
+        if len(dep) == 2:
+            sources_list_line, fingerprint = dep
+            sources_list_lines.append(sources_list_line)
         else:
             archive, distro_arch_series, pocket, components = dep
             has_published_binaries = _has_published_binaries(
@@ -379,23 +389,23 @@
                 archive, distro_arch_series, pocket, components)
             sources_list_lines.append(sources_list_line)
             fingerprint = archive.signing_key_fingerprint
-            if fingerprint is not None and fingerprint not in trusted_keys:
-                def get_key():
-                    with default_timeout(15.0):
-                        try:
-                            return gpghandler.retrieveKey(fingerprint)
-                        except GPGKeyNotFoundError as e:
-                            # For now, just log this and proceed without the
-                            # key.  We'll have to fix any outstanding cases
-                            # of this before we can switch to requiring
-                            # authentication across the board.
-                            if logger is not None:
-                                logger.warning(str(e))
-                            return None
+        if fingerprint is not None and fingerprint not in trusted_keys:
+            def get_key():
+                with default_timeout(15.0):
+                    try:
+                        return gpghandler.retrieveKey(fingerprint)
+                    except GPGKeyNotFoundError as e:
+                        # For now, just log this and proceed without the
+                        # key.  We'll have to fix any outstanding cases of
+                        # this before we can switch to requiring
+                        # authentication across the board.
+                        if logger is not None:
+                            logger.warning(str(e))
+                        return None
 
-                key = yield deferToThread(get_key)
-                if key is not None:
-                    trusted_keys[fingerprint] = base64.b64encode(key.export())
+            key = yield deferToThread(get_key)
+            if key is not None:
+                trusted_keys[fingerprint] = base64.b64encode(key.export())
 
     defer.returnValue(
         (sources_list_lines, [v for k, v in sorted(trusted_keys.items())]))

=== modified file 'lib/lp/soyuz/adapters/tests/test_archivedependencies.py'
--- lib/lp/soyuz/adapters/tests/test_archivedependencies.py	2017-06-13 17:22:44 +0000
+++ lib/lp/soyuz/adapters/tests/test_archivedependencies.py	2017-06-14 08:12:16 +0000
@@ -16,7 +16,9 @@
     )
 import transaction
 from twisted.internet import defer
+from twisted.internet.threads import deferToThread
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.archivepublisher.interfaces.archivesigningkey import (
     IArchiveSigningKey,
@@ -24,6 +26,7 @@
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.config import config
+from lp.services.gpg.interfaces import IGPGHandler
 from lp.services.log.logger import BufferLogger
 from lp.soyuz.adapters.archivedependencies import (
     default_component_dependency_name,
@@ -522,6 +525,18 @@
         # sources.list, which is useful for specialised build types.
         ppa = yield self.makeArchive(publish_binary=True)
         build = self.makeBuild(archive=ppa)
+
+        # Upload the tools archive key to the keyserver.
+        tools_key_name = "ppa-sample-4096@xxxxxxxxxxxxx"
+        tools_key_path = os.path.join(gpgkeysdir, "%s.sec" % tools_key_name)
+        with open(tools_key_path) as tools_key_file:
+            secret_key_export = tools_key_file.read()
+        # Remove security proxy to avoid problems with running in a thread.
+        gpghandler = removeSecurityProxy(getUtility(IGPGHandler))
+        gpghandler.importSecretKey(secret_key_export)
+        yield deferToThread(
+            gpghandler.uploadPublicKey, self.fingerprints[tools_key_name])
+
         yield self.assertSourcesListAndKeys(
             [(ppa, ["hoary main"]),
              ("deb http://example.org";, ["hoary main"]),
@@ -531,8 +546,9 @@
                  "hoary-updates main restricted universe multiverse",
                  ]),
              ],
-            ["ppa-sample@xxxxxxxxxxxxx"], build,
-            tools_source="deb http://example.org %(series)s main")
+            ["ppa-sample@xxxxxxxxxxxxx", tools_key_name], build,
+            tools_source="deb http://example.org %(series)s main",
+            tools_fingerprint=self.fingerprints[tools_key_name])
 
     @defer.inlineCallbacks
     def test_build_tools_bad_formatting(self):

=== modified file 'lib/lp/soyuz/tests/soyuz.py'
--- lib/lp/soyuz/tests/soyuz.py	2017-04-29 23:51:28 +0000
+++ lib/lp/soyuz/tests/soyuz.py	2017-06-14 08:12:16 +0000
@@ -114,3 +114,6 @@
         key = base64.b64decode(encoded_key)
         return Equals(self.fingerprint).match(
             getUtility(IGPGHandler).importPublicKey(key).fingerprint)
+
+    def __str__(self):
+        return "Base64KeyMatches(%s)" % self.fingerprint