launchpad-reviewers team mailing list archive

[Guruprasad <mp+464961@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:fix-ppa-key-updater-default-ppa-fingerprint-none-or-missing-in-gpgkey-table into launchpad:master.

Commit message:
Add error handling when recursively trying to update the default PPA's signing key

This handles unexpected situations like when the default PPA has no
signing key or it has a 4096-bit signing key generated outside the PPA
key updater script and hence is missing an entry in the 'gpgkey' table.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/464961
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:fix-ppa-key-updater-default-ppa-fingerprint-none-or-missing-in-gpgkey-table into launchpad:master.

diff --git a/lib/lp/archivepublisher/archivegpgsigningkey.py b/lib/lp/archivepublisher/archivegpgsigningkey.py
index 34267a4..eeffe71 100644
--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
@@ -357,7 +357,7 @@ class ArchiveGPGSigningKey(SignableArchive):
             self.archive.signing_key_fingerprint
         )
         assert (
-            current_gpg_key.keysize == 1024
+            current_gpg_key and current_gpg_key.keysize == 1024
         ), "Archive already has a 4096-bit RSA signing key."
         default_ppa = self.archive.owner.archive
 
@@ -383,10 +383,19 @@ class ArchiveGPGSigningKey(SignableArchive):
                 IArchiveSigningKeySet
             ).get4096BitRSASigningKey(default_ppa)
             if default_ppa_new_signing_key is None:
-                # Recursively update default_ppa key
-                IArchiveGPGSigningKey(
-                    default_ppa
-                ).generate4096BitRSASigningKey(log=log)
+                try:
+                    # Recursively update default_ppa key
+                    IArchiveGPGSigningKey(
+                        default_ppa
+                    ).generate4096BitRSASigningKey(log=log)
+                except Exception as e:
+                    log.error(
+                        "Error generating 4096-bit RSA signing key "
+                        "for %s - %s",
+                        default_ppa.reference,
+                        e,
+                    )
+                    return
                 # Refresh the default_ppa_new_signing_key with
                 # the newly created one.
                 default_ppa_new_signing_key = getUtility(
diff --git a/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py b/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py
index 1901169..faaf7e6 100644
--- a/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py
+++ b/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py
@@ -416,3 +416,65 @@ class TestPPAKeyUpdater(TestCaseWithFactory, TestWithFixtures):
             "Signing service should be enabled to use this feature.",
         ):
             archive_signing_key.generate4096BitRSASigningKey()
+
+    def testMatchingArchiveButDefaultArchiveHasNoFingerprint(self):
+        self.useFixture(
+            FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"})
+        )
+        owner = self.factory.makePerson()
+        default_archive = self.factory.makeArchive(owner=owner)
+        another_archive = self.factory.makeArchive(owner=owner)
+        fingerprint = self.factory.getUniqueHexString(40).upper()
+        logger = BufferLogger()
+        self.factory.makeGPGKey(
+            owner=owner,
+            keyid=fingerprint[-8:],
+            fingerprint=fingerprint,
+            keysize=1024,
+        )
+        self.factory.makeSigningKey(
+            key_type=SigningKeyType.OPENPGP, fingerprint=fingerprint
+        )
+        another_archive.signing_key_fingerprint = fingerprint
+        archive_signing_key = IArchiveGPGSigningKey(another_archive)
+        archive_signing_key.generate4096BitRSASigningKey(log=logger)
+        self.assertIn(
+            "Error generating 4096-bit RSA signing key for "
+            f"{default_archive.reference} - "
+            "Archive doesn't have an existing signing key to update.",
+            logger.getLogBuffer(),
+        )
+
+    def testPPAMatchingArchiveDefaultArchiveHasSecureExistingKey(self):
+        self.useFixture(
+            FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"})
+        )
+        owner = self.factory.makePerson()
+        default_archive = self.factory.makeArchive(owner=owner)
+        another_archive = self.factory.makeArchive(owner=owner)
+        default_archive.signing_key_fingerprint = (
+            self.factory.getUniqueHexString(40).upper()
+        )
+        another_archive_fingerprint = self.factory.getUniqueHexString(
+            40
+        ).upper()
+        logger = BufferLogger()
+        self.factory.makeGPGKey(
+            owner=owner,
+            keyid=another_archive_fingerprint[-8:],
+            fingerprint=another_archive_fingerprint,
+            keysize=1024,
+        )
+        self.factory.makeSigningKey(
+            key_type=SigningKeyType.OPENPGP,
+            fingerprint=another_archive_fingerprint,
+        )
+        another_archive.signing_key_fingerprint = another_archive_fingerprint
+        archive_signing_key = IArchiveGPGSigningKey(another_archive)
+        archive_signing_key.generate4096BitRSASigningKey(log=logger)
+        self.assertIn(
+            "Error generating 4096-bit RSA signing key for "
+            f"{default_archive.reference} - "
+            "Archive already has a 4096-bit RSA signing key",
+            logger.getLogBuffer(),
+        )