launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/delay-ppa-publication into lp:launchpad.

Commit message:
Only publish PPAs once their signing key has been generated.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #374395 in Launchpad itself: "New PPAs are published unsigned if used immediately"
  https://bugs.launchpad.net/launchpad/+bug/374395

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/delay-ppa-publication/+merge/269090

Only publish PPAs once their signing key has been generated.  Going ahead and publishing them unsigned has been confusing people for years.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/delay-ppa-publication into lp:launchpad.

=== 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-25 16:14:34 +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-25 16:14:34 +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,16 @@
             os.path.join("%s" % distsroot, distroseries, 'Release'))
         shutil.rmtree(tmp_path)
 
+    def testForPPAWithoutSigningKey(self):
+        """publish-distro skips PPAs that do not yet have a signing key."""
+        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 +232,13 @@
         naked_archive = removeSecurityProxy(name16.archive)
         naked_archive.distribution = self.ubuntutest
 
+        tac = KeyServerTac()
+        tac.setUp()
+        self.addCleanup(tac.tearDown)
+        key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
+        IArchiveSigningKey(cprov.archive).setSigningKey(key_path)
+        name16.archive.signing_key = cprov.archive.signing_key
+
         self.layer.txn.commit()
 
         self.runPublishDistro(['--ppa'])
@@ -261,6 +281,12 @@
             sourcename='baz', filecontent='baz', archive=private_ppa)
         self.layer.txn.commit()
 
+        tac = KeyServerTac()
+        tac.setUp()
+        self.addCleanup(tac.tearDown)
+        key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
+        IArchiveSigningKey(private_ppa).setSigningKey(key_path)
+
         # Try a plain PPA run, to ensure the private one is NOT published.
         self.runPublishDistro(['--ppa'])
 
@@ -358,6 +384,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/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-25 16:14:34 +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-25 16:14:34 +0000
@@ -413,6 +413,14 @@
         return self.status == ArchiveStatus.ACTIVE
 
     @property
+    def can_be_published(self):
+        """See `IArchive`."""
+        # PPAs can only be published once their signing key has been generated.
+        return (
+            self.publish and
+            (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: