Launchpad itself

Merge ~lgp171188/launchpad:fix-ppa-key-updater-default-ppa-fingerprint-none-or-missing-in-gpgkey-table into launchpad:master

  1. Git
  2. lp:~lgp171188/launchpad
  3. fix-ppa-key-updater-default-ppa-fingerprint-none-or-missing-in-gpgkey-table
  4. Merge into master
Proposed by Guruprasad
Status: Merged
Approved by: Simone Pelosi
Approved revision: 5a660625b1b335341d85bd6fa4eeed85dfbc2fb5
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~lgp171188/launchpad:fix-ppa-key-updater-default-ppa-fingerprint-none-or-missing-in-gpgkey-table
Merge into: launchpad:master
Diff against target: 107 lines (+76/-5)
2 files modified
lib/lp/archivepublisher/archivegpgsigningkey.py (+14/-5)
lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py (+62/-0)
Reviewer Review Type Date Requested Status
Simone Pelosi Approve
Review via email: mp+464961@code.launchpad.net

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.

To post a comment you must log in.
Revision history for this message
Simone Pelosi (pelpsi) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/archivepublisher/archivegpgsigningkey.py b/lib/lp/archivepublisher/archivegpgsigningkey.py
2index 34267a4..eeffe71 100644
3--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
4+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
5@@ -357,7 +357,7 @@ class ArchiveGPGSigningKey(SignableArchive):
6 self.archive.signing_key_fingerprint
7 )
8 assert (
9- current_gpg_key.keysize == 1024
10+ current_gpg_key and current_gpg_key.keysize == 1024
11 ), "Archive already has a 4096-bit RSA signing key."
12 default_ppa = self.archive.owner.archive
13
14@@ -383,10 +383,19 @@ class ArchiveGPGSigningKey(SignableArchive):
15 IArchiveSigningKeySet
16 ).get4096BitRSASigningKey(default_ppa)
17 if default_ppa_new_signing_key is None:
18- # Recursively update default_ppa key
19- IArchiveGPGSigningKey(
20- default_ppa
21- ).generate4096BitRSASigningKey(log=log)
22+ try:
23+ # Recursively update default_ppa key
24+ IArchiveGPGSigningKey(
25+ default_ppa
26+ ).generate4096BitRSASigningKey(log=log)
27+ except Exception as e:
28+ log.error(
29+ "Error generating 4096-bit RSA signing key "
30+ "for %s - %s",
31+ default_ppa.reference,
32+ e,
33+ )
34+ return
35 # Refresh the default_ppa_new_signing_key with
36 # the newly created one.
37 default_ppa_new_signing_key = getUtility(
38diff --git a/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py b/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py
39index 1901169..faaf7e6 100644
40--- a/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py
41+++ b/lib/lp/soyuz/scripts/tests/test_ppakeyupdater.py
42@@ -416,3 +416,65 @@ class TestPPAKeyUpdater(TestCaseWithFactory, TestWithFixtures):
43 "Signing service should be enabled to use this feature.",
44 ):
45 archive_signing_key.generate4096BitRSASigningKey()
46+
47+ def testMatchingArchiveButDefaultArchiveHasNoFingerprint(self):
48+ self.useFixture(
49+ FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"})
50+ )
51+ owner = self.factory.makePerson()
52+ default_archive = self.factory.makeArchive(owner=owner)
53+ another_archive = self.factory.makeArchive(owner=owner)
54+ fingerprint = self.factory.getUniqueHexString(40).upper()
55+ logger = BufferLogger()
56+ self.factory.makeGPGKey(
57+ owner=owner,
58+ keyid=fingerprint[-8:],
59+ fingerprint=fingerprint,
60+ keysize=1024,
61+ )
62+ self.factory.makeSigningKey(
63+ key_type=SigningKeyType.OPENPGP, fingerprint=fingerprint
64+ )
65+ another_archive.signing_key_fingerprint = fingerprint
66+ archive_signing_key = IArchiveGPGSigningKey(another_archive)
67+ archive_signing_key.generate4096BitRSASigningKey(log=logger)
68+ self.assertIn(
69+ "Error generating 4096-bit RSA signing key for "
70+ f"{default_archive.reference} - "
71+ "Archive doesn't have an existing signing key to update.",
72+ logger.getLogBuffer(),
73+ )
74+
75+ def testPPAMatchingArchiveDefaultArchiveHasSecureExistingKey(self):
76+ self.useFixture(
77+ FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"})
78+ )
79+ owner = self.factory.makePerson()
80+ default_archive = self.factory.makeArchive(owner=owner)
81+ another_archive = self.factory.makeArchive(owner=owner)
82+ default_archive.signing_key_fingerprint = (
83+ self.factory.getUniqueHexString(40).upper()
84+ )
85+ another_archive_fingerprint = self.factory.getUniqueHexString(
86+ 40
87+ ).upper()
88+ logger = BufferLogger()
89+ self.factory.makeGPGKey(
90+ owner=owner,
91+ keyid=another_archive_fingerprint[-8:],
92+ fingerprint=another_archive_fingerprint,
93+ keysize=1024,
94+ )
95+ self.factory.makeSigningKey(
96+ key_type=SigningKeyType.OPENPGP,
97+ fingerprint=another_archive_fingerprint,
98+ )
99+ another_archive.signing_key_fingerprint = another_archive_fingerprint
100+ archive_signing_key = IArchiveGPGSigningKey(another_archive)
101+ archive_signing_key.generate4096BitRSASigningKey(log=logger)
102+ self.assertIn(
103+ "Error generating 4096-bit RSA signing key for "
104+ f"{default_archive.reference} - "
105+ "Archive already has a 4096-bit RSA signing key",
106+ logger.getLogBuffer(),
107+ )

Subscribers

People subscribed via source and target branches

to status/vote changes: