Launchpad itself

Merge ~cjwatson/launchpad:gpg-log-inject into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. gpg-log-inject
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: aee97d7ed51fcd36acecda265c517689c7f7cdcc
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:gpg-log-inject
Merge into: launchpad:master
Diff against target: 132 lines (+21/-9)
6 files modified
lib/lp/archivepublisher/archivegpgsigningkey.py (+3/-2)
lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py (+2/-1)
lib/lp/archivepublisher/tests/archive-signing.txt (+1/-1)
lib/lp/services/gpg/handler.py (+5/-1)
lib/lp/services/gpg/tests/test_gpghandler.py (+9/-3)
lib/lp/soyuz/scripts/ppakeygenerator.py (+1/-1)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+390252@code.launchpad.net

Commit message

Log injection of GPG keys into signing service

Description of the change

It's otherwise not very obvious that injection has actually happened.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) 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 7439a32..685ba04 100644
3--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
4+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
5@@ -188,7 +188,7 @@ class ArchiveGPGSigningKey(SignableArchive):
6 with open(export_path, 'w') as export_file:
7 export_file.write(key.export())
8
9- def generateSigningKey(self):
10+ def generateSigningKey(self, log=None):
11 """See `IArchiveGPGSigningKey`."""
12 assert self.archive.signing_key is None, (
13 "Cannot override signing_keys.")
14@@ -208,7 +208,8 @@ class ArchiveGPGSigningKey(SignableArchive):
15
16 key_displayname = (
17 "Launchpad PPA for %s" % self.archive.owner.displayname)
18- secret_key = getUtility(IGPGHandler).generateKey(key_displayname)
19+ secret_key = getUtility(IGPGHandler).generateKey(
20+ key_displayname, logger=log)
21 self._setupSigningKey(secret_key)
22
23 def setSigningKey(self, key_path, async_keyserver=False):
24diff --git a/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py b/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py
25index 85d3bfb..2efffb6 100644
26--- a/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py
27+++ b/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py
28@@ -98,7 +98,7 @@ class IArchiveGPGSigningKey(ISignableArchive):
29 :raises AssertionError: if the given key is public.
30 """
31
32- def generateSigningKey():
33+ def generateSigningKey(log=None):
34 """Generate a new GPG secret/public key pair.
35
36 For named-ppas, the existing signing-key for the default PPA
37@@ -112,6 +112,7 @@ class IArchiveGPGSigningKey(ISignableArchive):
38 * Store a reference for the public key in GPGKey table, which
39 is set as the context archive 'signing_key'.
40
41+ :param log: an optional logger.
42 :raises AssertionError: if the context archive already has a
43 `signing_key`.
44 :raises GPGUploadFailure: if the just-generated key could not be
45diff --git a/lib/lp/archivepublisher/tests/archive-signing.txt b/lib/lp/archivepublisher/tests/archive-signing.txt
46index f2dfca9..a487a3b 100644
47--- a/lib/lp/archivepublisher/tests/archive-signing.txt
48+++ b/lib/lp/archivepublisher/tests/archive-signing.txt
49@@ -333,7 +333,7 @@ Then modify the GPGHandler utility to return a sampledata key instead
50 of generating a new one, mainly for running the test faster and for
51 printing the context the key is generated.
52
53- >>> def mock_key_generator(name):
54+ >>> def mock_key_generator(name, logger=None):
55 ... print 'Generating:', name
56 ... key_path = os.path.join(gpgkeysdir, 'sign.only@canonical.com.sec')
57 ... return gpghandler.importSecretKey(open(key_path).read())
58diff --git a/lib/lp/services/gpg/handler.py b/lib/lp/services/gpg/handler.py
59index 965282a..e90538c 100644
60--- a/lib/lp/services/gpg/handler.py
61+++ b/lib/lp/services/gpg/handler.py
62@@ -334,7 +334,7 @@ class GPGHandler:
63 SigningKeyType.OPENPGP, secret_key, public_key, key.uids[0].name,
64 now)
65
66- def generateKey(self, name):
67+ def generateKey(self, name, logger=None):
68 """See `IGPGHandler`."""
69 context = get_gpgme_context()
70
71@@ -368,6 +368,10 @@ class GPGHandler:
72 'The key does not seem to exist in the local keyring.')
73
74 if getFeatureFlag(GPG_INJECT):
75+ if logger is not None:
76+ logger.info(
77+ "Injecting key_type %s '%s' into signing service",
78+ SigningKeyType.OPENPGP, name)
79 try:
80 self._injectKeyPair(key)
81 except Exception:
82diff --git a/lib/lp/services/gpg/tests/test_gpghandler.py b/lib/lp/services/gpg/tests/test_gpghandler.py
83index 162f39a..5d8651e 100644
84--- a/lib/lp/services/gpg/tests/test_gpghandler.py
85+++ b/lib/lp/services/gpg/tests/test_gpghandler.py
86@@ -362,7 +362,7 @@ class TestGPGHandler(TestCase):
87 "op=index&search=0x%s" % fingerprint,
88 self.gpg_handler.getURLForKeyInServer(fingerprint, public=True))
89
90- def assertGeneratesKey(self):
91+ def assertGeneratesKey(self, logger=None):
92 # We don't test real key generation because it depends on machine
93 # entropy that we may not have in buildbot, and in general may be
94 # slow. The sample key on disk was generated by running the code
95@@ -378,7 +378,8 @@ class TestGPGHandler(TestCase):
96 # export_file.write(new_key.export())
97 self.useFixture(FakeGenerateKey("ppa-sample@canonical.com.sec"))
98 new_key = self.gpg_handler.generateKey(
99- u"Launchpad PPA for Celso \xe1\xe9\xed\xf3\xfa Providelo")
100+ u"Launchpad PPA for Celso \xe1\xe9\xed\xf3\xfa Providelo",
101+ logger=logger)
102 # generateKey currently only generates passwordless sign-only keys,
103 # i.e. they can sign content but cannot encrypt. The generated key
104 # contains a single UID and only its "name" term is set.
105@@ -437,8 +438,13 @@ class TestGPGHandler(TestCase):
106 mock_datetime = self.useFixture(MockPatch(
107 "lp.services.gpg.handler.datetime")).mock
108 mock_datetime.now = lambda: now
109- new_key = self.assertGeneratesKey()
110+ logger = BufferLogger()
111+ new_key = self.assertGeneratesKey(logger=logger)
112 new_public_key = self.gpg_handler.retrieveKey(new_key.fingerprint)
113+ self.assertEqual(
114+ u"INFO Injecting key_type OpenPGP 'Launchpad PPA for Celso "
115+ u"\xe1\xe9\xed\xf3\xfa Providelo' into signing service\n",
116+ logger.getLogBuffer())
117 self.assertEqual(1, signing_service_client.inject.call_count)
118 self.assertThat(
119 signing_service_client.inject.call_args[0],
120diff --git a/lib/lp/soyuz/scripts/ppakeygenerator.py b/lib/lp/soyuz/scripts/ppakeygenerator.py
121index c1aa3f6..190b4a0 100644
122--- a/lib/lp/soyuz/scripts/ppakeygenerator.py
123+++ b/lib/lp/soyuz/scripts/ppakeygenerator.py
124@@ -33,7 +33,7 @@ class PPAKeyGenerator(LaunchpadCronScript):
125 "Generating signing key for %s (%s)" %
126 (archive.reference, archive.displayname))
127 archive_signing_key = IArchiveGPGSigningKey(archive)
128- archive_signing_key.generateSigningKey()
129+ archive_signing_key.generateSigningKey(log=self.logger)
130 self.logger.info("Key %s" % archive.signing_key.fingerprint)
131
132 def main(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: