Merge lp:~cjwatson/launchpad/delay-ppa-publication into lp:launchpad

[Colin Watson]

Only publish PPAs once their signing key has been generated.

Only publish PPAs once their signing key has been generated. Going ahead and publishing them unsigned has been confusing people for years.

[William Grant (wgrant)]

This will make dogfood and dev testing annoying, so I'd possibly consider using a config flag to apply it only on prod.

review: Approve (code)

=== modified file 'lib/lp/archivepublisher/scripts/publishdistro.py'
--- lib/lp/archivepublisher/scripts/publishdistro.py	2015-03-18 17:49:57 +0000
+++ lib/lp/archivepublisher/scripts/publishdistro.py	2015-08-28 08:39:32 +0000
@@ -291,7 +291,7 @@
 
                 if archive.status == ArchiveStatus.DELETING:
                     work_done = self.deleteArchive(archive, publisher)
-                elif archive.publish:
+                elif archive.can_be_published:
                     for suite in self.options.dirty_suites:
                         distroseries, pocket = self.findSuite(
                             distribution, suite)

=== modified file 'lib/lp/archivepublisher/tests/test_publishdistro.py'
--- lib/lp/archivepublisher/tests/test_publishdistro.py	2015-03-20 07:49:14 +0000
+++ lib/lp/archivepublisher/tests/test_publishdistro.py	2015-08-28 08:39:32 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Functional tests for publish-distro.py script."""
@@ -16,6 +16,7 @@
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.archivepublisher.config import getPubConfig
+from lp.archivepublisher.interfaces.archivesigningkey import IArchiveSigningKey
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
 from lp.archivepublisher.publishing import Publisher
 from lp.archivepublisher.scripts.publishdistro import PublishDistro
@@ -39,6 +40,8 @@
 from lp.testing.dbuser import switch_dbuser
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.faketransaction import FakeTransaction
+from lp.testing.gpgkeys import gpgkeysdir
+from lp.testing.keyserver import KeyServerTac
 from lp.testing.layers import ZopelessDatabaseLayer
 
 
@@ -195,6 +198,24 @@
             os.path.join("%s" % distsroot, distroseries, 'Release'))
         shutil.rmtree(tmp_path)
 
+    def setUpRequireSigningKeys(self):
+        config.push('ppa-require-signing-keys', """
+            [personalpackagearchive]
+            require_signing_keys: true
+            """)
+        self.addCleanup(config.pop, 'ppa-require-signing-keys')
+
+    def testForPPAWithoutSigningKey(self):
+        """publish-distro skips PPAs that do not yet have a signing key."""
+        self.setUpRequireSigningKeys()
+        cprov = getUtility(IPersonSet).getByName('cprov')
+        pub_source = self.getPubSource(archive=cprov.archive)
+        removeSecurityProxy(cprov.archive).distribution = self.ubuntutest
+        self.layer.txn.commit()
+        self.runPublishDistro(['--ppa'])
+        pub_source.sync()
+        self.assertEqual(PackagePublishingStatus.PENDING, pub_source.status)
+
     def testForPPA(self):
         """Try to run publish-distro in PPA mode.
 
@@ -219,6 +240,14 @@
         naked_archive = removeSecurityProxy(name16.archive)
         naked_archive.distribution = self.ubuntutest
 
+        self.setUpRequireSigningKeys()
+        tac = KeyServerTac()
+        tac.setUp()
+        self.addCleanup(tac.tearDown)
+        key_path = os.path.join(gpgkeysdir, '[e-mail]')
+        IArchiveSigningKey(cprov.archive).setSigningKey(key_path)
+        name16.archive.signing_key = cprov.archive.signing_key
+
         self.layer.txn.commit()
 
         self.runPublishDistro(['--ppa'])
@@ -261,6 +290,13 @@
             sourcename='baz', filecontent='baz', archive=private_ppa)
         self.layer.txn.commit()
 
+        self.setUpRequireSigningKeys()
+        tac = KeyServerTac()
+        tac.setUp()
+        self.addCleanup(tac.tearDown)
+        key_path = os.path.join(gpgkeysdir, '[e-mail]')
+        IArchiveSigningKey(private_ppa).setSigningKey(key_path)
+
         # Try a plain PPA run, to ensure the private one is NOT published.
         self.runPublishDistro(['--ppa'])
 
@@ -358,6 +394,7 @@
     """A very simple fake `Archive`."""
     def __init__(self, purpose=ArchivePurpose.PRIMARY):
         self.publish = True
+        self.can_be_published = True
         self.purpose = purpose
         self.status = ArchiveStatus.ACTIVE
 

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2015-08-04 11:03:04 +0000
+++ lib/lp/services/config/schema-lazr.conf	2015-08-28 08:39:32 +0000
@@ -1411,6 +1411,11 @@
 # datatype: string
 signing_keys_root: /var/tmp/ppa-signing-keys
 
+# If true, PPAs will only be published once their signing-keys have been
+# generated.
+# datatype: boolean
+require_signing_keys: false
+
 
 [ppa_apache_log_parser]
 logs_root: /srv/ppa.launchpad.net-logs

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2015-08-03 14:47:25 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2015-08-28 08:39:32 +0000
@@ -406,6 +406,11 @@
         description=_("Whether or not to update the apt repository.  If "
             "disabled, nothing will be published.  If the archive is "
             "private then additionally no builds will be dispatched."))
+    can_be_published = Bool(
+        title=_("Can be published"), required=True,
+        description=_(
+            "True if this archive can be published, considering both the "
+            "explicit publish flag and any other constraints."))
     series_with_sources = Attribute(
         "DistroSeries to which this archive has published sources")
     signing_key = Object(

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2015-07-08 16:05:11 +0000
+++ lib/lp/soyuz/model/archive.py	2015-08-28 08:39:32 +0000
@@ -413,6 +413,19 @@
         return self.status == ArchiveStatus.ACTIVE
 
     @property
+    def can_be_published(self):
+        """See `IArchive`."""
+        # The explicit publish flag must be set.
+        if not self.publish:
+            return False
+        # In production configurations, PPAs can only be published once
+        # their signing key has been generated.
+        return (
+            not config.personalpackagearchive.require_signing_keys or
+            not self.is_ppa or
+            self.signing_key is not None)
+
+    @property
     def reference(self):
         template = ARCHIVE_REFERENCE_TEMPLATES.get(self.purpose)
         if template is None: