Merge lp:~cjwatson/launchpad/better-upload-error-notifications into lp:launchpad
- better-upload-error-notifications
- Merge into devel
Proposed by
Colin Watson
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 18472 | ||||
Proposed branch: | lp:~cjwatson/launchpad/better-upload-error-notifications | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
438 lines (+129/-76) 11 files modified
lib/lp/archiveuploader/changesfile.py (+16/-10) lib/lp/archiveuploader/dscfile.py (+10/-6) lib/lp/archiveuploader/nascentupload.py (+3/-1) lib/lp/archiveuploader/tests/nascentupload.txt (+19/-23) lib/lp/archiveuploader/tests/nascentuploadfile.txt (+4/-0) lib/lp/archiveuploader/tests/test_changesfile.py (+19/-13) lib/lp/archiveuploader/tests/test_nascentuploadfile.py (+3/-1) lib/lp/archiveuploader/tests/test_sync_notification.py (+2/-1) lib/lp/archiveuploader/tests/test_uploadprocessor.py (+40/-2) lib/lp/archiveuploader/uploadprocessor.py (+12/-17) lib/lp/soyuz/doc/distroseriesqueue-translations.txt (+1/-2) |
||||
To merge this branch: | bzr merge lp:~cjwatson/launchpad/better-upload-error-notifications | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+311179@code.launchpad.net |
Commit message
Send proper email notifications about most failures to parse the .changes file.
Description of the change
We can now send email notifications to the signer as long as we have a signature, even if the signing key is deactivated. If the .changes file is sufficiently parseable then we'll notify other relevant people as well.
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/archiveuploader/changesfile.py' | |||
2 | --- lib/lp/archiveuploader/changesfile.py 2017-03-29 09:28:09 +0000 | |||
3 | +++ lib/lp/archiveuploader/changesfile.py 2017-09-17 15:23:26 +0000 | |||
4 | @@ -84,6 +84,11 @@ | |||
5 | 84 | files = None | 84 | files = None |
6 | 85 | 85 | ||
7 | 86 | def __init__(self, filepath, policy, logger): | 86 | def __init__(self, filepath, policy, logger): |
8 | 87 | self.filepath = filepath | ||
9 | 88 | self.policy = policy | ||
10 | 89 | self.logger = logger | ||
11 | 90 | |||
12 | 91 | def parseChanges(self): | ||
13 | 87 | """Process the given changesfile. | 92 | """Process the given changesfile. |
14 | 88 | 93 | ||
15 | 89 | Does: | 94 | Does: |
16 | @@ -93,24 +98,25 @@ | |||
17 | 93 | * Checks name of changes file | 98 | * Checks name of changes file |
18 | 94 | * Checks signature of changes file | 99 | * Checks signature of changes file |
19 | 95 | 100 | ||
23 | 96 | If any of these checks fail, UploadError is raised, and it's | 101 | If any of these checks fail, UploadError is yielded, and it should |
24 | 97 | considered a fatal error (no subsequent processing of the upload | 102 | be considered a fatal error (no subsequent processing of the upload |
25 | 98 | will be done). | 103 | should be done). |
26 | 99 | 104 | ||
27 | 100 | Logger and Policy are instances built in uploadprocessor.py passed | 105 | Logger and Policy are instances built in uploadprocessor.py passed |
28 | 101 | via NascentUpload class. | 106 | via NascentUpload class. |
29 | 102 | """ | 107 | """ |
35 | 103 | self.filepath = filepath | 108 | try: |
36 | 104 | self.policy = policy | 109 | self.parse(verify_signature=not self.policy.unsigned_changes_ok) |
37 | 105 | self.logger = logger | 110 | except UploadError as e: |
38 | 106 | 111 | yield e | |
39 | 107 | self.parse(verify_signature=not policy.unsigned_changes_ok) | 112 | return |
40 | 108 | 113 | ||
41 | 109 | for field in self.mandatory_fields: | 114 | for field in self.mandatory_fields: |
42 | 110 | if field not in self._dict: | 115 | if field not in self._dict: |
44 | 111 | raise UploadError( | 116 | yield UploadError( |
45 | 112 | "Unable to find mandatory field '%s' in the changes " | 117 | "Unable to find mandatory field '%s' in the changes " |
46 | 113 | "file." % field) | 118 | "file." % field) |
47 | 119 | return | ||
48 | 114 | 120 | ||
49 | 115 | try: | 121 | try: |
50 | 116 | format = float(self._dict["Format"]) | 122 | format = float(self._dict["Format"]) |
51 | @@ -119,7 +125,7 @@ | |||
52 | 119 | format = 1.5 | 125 | format = 1.5 |
53 | 120 | 126 | ||
54 | 121 | if format < 1.5 or format > 2.0: | 127 | if format < 1.5 or format > 2.0: |
56 | 122 | raise UploadError( | 128 | yield UploadError( |
57 | 123 | "Format out of acceptable range for changes file. Range " | 129 | "Format out of acceptable range for changes file. Range " |
58 | 124 | "1.5 - 2.0, format %g" % format) | 130 | "1.5 - 2.0, format %g" % format) |
59 | 125 | 131 | ||
60 | 126 | 132 | ||
61 | === modified file 'lib/lp/archiveuploader/dscfile.py' | |||
62 | --- lib/lp/archiveuploader/dscfile.py 2017-07-31 11:45:32 +0000 | |||
63 | +++ lib/lp/archiveuploader/dscfile.py 2017-09-17 15:23:26 +0000 | |||
64 | @@ -142,8 +142,16 @@ | |||
65 | 142 | "Unable to read %s: %s" % (self.filename, error)) | 142 | "Unable to read %s: %s" % (self.filename, error)) |
66 | 143 | 143 | ||
67 | 144 | if verify_signature: | 144 | if verify_signature: |
69 | 145 | self.signingkey, self.parsed_content = self.verifySignature( | 145 | # We set self.signingkey regardless of whether the key is |
70 | 146 | # deactivated, since a deactivated key is still good enough for | ||
71 | 147 | # determining whom to notify, and raising UploadError is enough | ||
72 | 148 | # to prevent the upload being accepted. | ||
73 | 149 | self.signingkey, self.parsed_content = self._verifySignature( | ||
74 | 146 | self.raw_content, self.filepath) | 150 | self.raw_content, self.filepath) |
75 | 151 | if not self.signingkey.active: | ||
76 | 152 | raise UploadError("File %s is signed with a deactivated key %s" | ||
77 | 153 | % (self.filepath, | ||
78 | 154 | self.signingkey.fingerprint)) | ||
79 | 147 | else: | 155 | else: |
80 | 148 | self.logger.debug("%s can be unsigned." % self.filename) | 156 | self.logger.debug("%s can be unsigned." % self.filename) |
81 | 149 | self.parsed_content = self.raw_content | 157 | self.parsed_content = self.raw_content |
82 | @@ -154,7 +162,7 @@ | |||
83 | 154 | raise UploadError( | 162 | raise UploadError( |
84 | 155 | "Unable to parse %s: %s" % (self.filename, error)) | 163 | "Unable to parse %s: %s" % (self.filename, error)) |
85 | 156 | 164 | ||
87 | 157 | def verifySignature(self, content, filename): | 165 | def _verifySignature(self, content, filename): |
88 | 158 | """Verify the signature on the file content. | 166 | """Verify the signature on the file content. |
89 | 159 | 167 | ||
90 | 160 | Raise UploadError if the signing key cannot be found in launchpad | 168 | Raise UploadError if the signing key cannot be found in launchpad |
91 | @@ -179,10 +187,6 @@ | |||
92 | 179 | raise UploadError("Signing key %s not registered in launchpad." | 187 | raise UploadError("Signing key %s not registered in launchpad." |
93 | 180 | % sig.fingerprint) | 188 | % sig.fingerprint) |
94 | 181 | 189 | ||
95 | 182 | if key.active == False: | ||
96 | 183 | raise UploadError("File %s is signed with a deactivated key %s" | ||
97 | 184 | % (filename, key.fingerprint)) | ||
98 | 185 | |||
99 | 186 | return (key, sig.plain_data) | 190 | return (key, sig.plain_data) |
100 | 187 | 191 | ||
101 | 188 | def parseAddress(self, addr, fieldname="Maintainer"): | 192 | def parseAddress(self, addr, fieldname="Maintainer"): |
102 | 189 | 193 | ||
103 | === modified file 'lib/lp/archiveuploader/nascentupload.py' | |||
104 | --- lib/lp/archiveuploader/nascentupload.py 2017-04-21 15:21:27 +0000 | |||
105 | +++ lib/lp/archiveuploader/nascentupload.py 2017-09-17 15:23:26 +0000 | |||
106 | @@ -142,6 +142,8 @@ | |||
107 | 142 | policy = self.policy | 142 | policy = self.policy |
108 | 143 | self.logger.debug("Beginning processing.") | 143 | self.logger.debug("Beginning processing.") |
109 | 144 | 144 | ||
110 | 145 | self.run_and_reject_on_error(self.changes.parseChanges) | ||
111 | 146 | |||
112 | 145 | try: | 147 | try: |
113 | 146 | policy.setDistroSeriesAndPocket(self.changes.suite_name) | 148 | policy.setDistroSeriesAndPocket(self.changes.suite_name) |
114 | 147 | except NotFoundError: | 149 | except NotFoundError: |
115 | @@ -780,7 +782,7 @@ | |||
116 | 780 | IDistributionSet)['ubuntu'].currentseries | 782 | IDistributionSet)['ubuntu'].currentseries |
117 | 781 | return distroseries.createQueueEntry( | 783 | return distroseries.createQueueEntry( |
118 | 782 | PackagePublishingPocket.RELEASE, | 784 | PackagePublishingPocket.RELEASE, |
120 | 783 | distroseries.main_archive, self.changes.filename, | 785 | self.policy.archive, self.changes.filename, |
121 | 784 | self.changes.raw_content, signing_key=self.changes.signingkey) | 786 | self.changes.raw_content, signing_key=self.changes.signingkey) |
122 | 785 | else: | 787 | else: |
123 | 786 | return distroseries.createQueueEntry( | 788 | return distroseries.createQueueEntry( |
124 | 787 | 789 | ||
125 | === modified file 'lib/lp/archiveuploader/tests/nascentupload.txt' | |||
126 | --- lib/lp/archiveuploader/tests/nascentupload.txt 2016-01-26 15:47:37 +0000 | |||
127 | +++ lib/lp/archiveuploader/tests/nascentupload.txt 2017-09-17 15:23:26 +0000 | |||
128 | @@ -39,38 +39,34 @@ | |||
129 | 39 | ... name='anything', distro='ubuntu', distroseries='hoary') | 39 | ... name='anything', distro='ubuntu', distroseries='hoary') |
130 | 40 | 40 | ||
131 | 41 | 41 | ||
139 | 42 | Constructing a NascentUpload object | 42 | NascentUpload Processing |
140 | 43 | ----------------------------------- | 43 | ------------------------ |
141 | 44 | 44 | ||
142 | 45 | Constructing a NascentUpload instance verifies that the changes file | 45 | Processing a NascentUpload consists of building files objects for each |
143 | 46 | specified exists and tries to build a ChangesFile (see | 46 | specified file in the upload, executing all their specific checks and |
144 | 47 | doc/nascentuploadfile.txt) object based on that. If anything goes | 47 | collect all errors that may be generated. (see doc/nascentuploadfile.txt) |
145 | 48 | wrong during this process UploadError is raised: | 48 | |
146 | 49 | First, NascentUpload verifies that the changes file specified exist, and | ||
147 | 50 | tries to build a ChangesFile (see doc/nascentuploadfile.txt) object based | ||
148 | 51 | on that. | ||
149 | 49 | 52 | ||
150 | 50 | >>> from lp.services.log.logger import DevNullLogger, FakeLogger | 53 | >>> from lp.services.log.logger import DevNullLogger, FakeLogger |
152 | 51 | >>> NascentUpload.from_changesfile_path( | 54 | |
153 | 55 | >>> nonexistent = NascentUpload.from_changesfile_path( | ||
154 | 52 | ... datadir("DOES-NOT-EXIST"), buildd_policy, FakeLogger()) | 56 | ... datadir("DOES-NOT-EXIST"), buildd_policy, FakeLogger()) |
155 | 57 | >>> nonexistent.process() | ||
156 | 53 | Traceback (most recent call last): | 58 | Traceback (most recent call last): |
157 | 54 | ... | 59 | ... |
161 | 55 | UploadError:... | 60 | EarlyReturnUploadError: An error occurred that prevented further |
162 | 56 | 61 | processing. | |
163 | 57 | Otherwise a ChangesFile object is ready to use. | 62 | >>> nonexistent.is_rejected |
164 | 63 | True | ||
165 | 64 | >>> print nonexistent.rejection_message | ||
166 | 65 | Unable to read DOES-NOT-EXIST: ... | ||
167 | 58 | 66 | ||
168 | 59 | >>> quodlibet = NascentUpload.from_changesfile_path( | 67 | >>> quodlibet = NascentUpload.from_changesfile_path( |
169 | 60 | ... datadir("quodlibet_0.13.1-1_i386.changes"), | 68 | ... datadir("quodlibet_0.13.1-1_i386.changes"), |
170 | 61 | ... anything_policy, DevNullLogger()) | 69 | ... anything_policy, DevNullLogger()) |
171 | 62 | |||
172 | 63 | >>> quodlibet.changes | ||
173 | 64 | <lp.archiveuploader.changesfile.ChangesFile ...> | ||
174 | 65 | |||
175 | 66 | |||
176 | 67 | NascentUpload Processing | ||
177 | 68 | ------------------------ | ||
178 | 69 | |||
179 | 70 | Processing a NascentUpload consists of building files objects for each | ||
180 | 71 | specified file in the upload, executing all their specific checks and | ||
181 | 72 | collect all errors that may be generated. (see doc/nascentuploadfile.txt) | ||
182 | 73 | |||
183 | 74 | >>> quodlibet.process() | 70 | >>> quodlibet.process() |
184 | 75 | >>> for f in quodlibet.changes.files: | 71 | >>> for f in quodlibet.changes.files: |
185 | 76 | ... print f.filename, f | 72 | ... print f.filename, f |
186 | 77 | 73 | ||
187 | === modified file 'lib/lp/archiveuploader/tests/nascentuploadfile.txt' | |||
188 | --- lib/lp/archiveuploader/tests/nascentuploadfile.txt 2017-08-03 14:26:40 +0000 | |||
189 | +++ lib/lp/archiveuploader/tests/nascentuploadfile.txt 2017-09-17 15:23:26 +0000 | |||
190 | @@ -72,10 +72,14 @@ | |||
191 | 72 | >>> ed_binary_changes = ChangesFile( | 72 | >>> ed_binary_changes = ChangesFile( |
192 | 73 | ... datadir('ed_0.2-20_i386.changes.binary-only'), | 73 | ... datadir('ed_0.2-20_i386.changes.binary-only'), |
193 | 74 | ... modified_insecure_policy, DevNullLogger()) | 74 | ... modified_insecure_policy, DevNullLogger()) |
194 | 75 | >>> len(list(ed_binary_changes.parseChanges())) | ||
195 | 76 | 0 | ||
196 | 75 | 77 | ||
197 | 76 | >>> ed_source_changes = ChangesFile( | 78 | >>> ed_source_changes = ChangesFile( |
198 | 77 | ... datadir('ed_0.2-20_source.changes'), | 79 | ... datadir('ed_0.2-20_source.changes'), |
199 | 78 | ... modified_insecure_policy, DevNullLogger()) | 80 | ... modified_insecure_policy, DevNullLogger()) |
200 | 81 | >>> len(list(ed_source_changes.parseChanges())) | ||
201 | 82 | 0 | ||
202 | 79 | 83 | ||
203 | 80 | Make sure we are not getting any exceptions due to a malformed changes | 84 | Make sure we are not getting any exceptions due to a malformed changes |
204 | 81 | file name. | 85 | file name. |
205 | 82 | 86 | ||
206 | === modified file 'lib/lp/archiveuploader/tests/test_changesfile.py' | |||
207 | --- lib/lp/archiveuploader/tests/test_changesfile.py 2017-03-29 09:28:09 +0000 | |||
208 | +++ lib/lp/archiveuploader/tests/test_changesfile.py 2017-09-17 15:23:26 +0000 | |||
209 | @@ -182,7 +182,10 @@ | |||
210 | 182 | changes.dump(changes_fd) | 182 | changes.dump(changes_fd) |
211 | 183 | finally: | 183 | finally: |
212 | 184 | changes_fd.close() | 184 | changes_fd.close() |
214 | 185 | return ChangesFile(path, self.policy, self.logger) | 185 | changesfile = ChangesFile(path, self.policy, self.logger) |
215 | 186 | for error in changesfile.parseChanges(): | ||
216 | 187 | raise error | ||
217 | 188 | return changesfile | ||
218 | 186 | 189 | ||
219 | 187 | def getBaseChanges(self): | 190 | def getBaseChanges(self): |
220 | 188 | contents = Changes() | 191 | contents = Changes() |
221 | @@ -364,36 +367,39 @@ | |||
222 | 364 | import_public_test_keys() | 367 | import_public_test_keys() |
223 | 365 | 368 | ||
224 | 366 | def test_valid_signature_accepted(self): | 369 | def test_valid_signature_accepted(self): |
226 | 367 | # A correctly signed changes file is excepted, and all its | 370 | # A correctly signed changes file is accepted, and all its |
227 | 368 | # content is parsed. | 371 | # content is parsed. |
228 | 369 | path = datadir('signatures/signed.changes') | 372 | path = datadir('signatures/signed.changes') |
230 | 370 | parsed = ChangesFile(path, InsecureUploadPolicy(), BufferLogger()) | 373 | changesfile = ChangesFile(path, InsecureUploadPolicy(), BufferLogger()) |
231 | 374 | self.assertEqual([], list(changesfile.parseChanges())) | ||
232 | 371 | self.assertEqual( | 375 | self.assertEqual( |
233 | 372 | getUtility(IPersonSet).getByEmail('foo.bar@canonical.com'), | 376 | getUtility(IPersonSet).getByEmail('foo.bar@canonical.com'), |
235 | 373 | parsed.signer) | 377 | changesfile.signer) |
236 | 374 | expected = "\AFormat: 1.7\n.*foo_1.0-1.diff.gz\Z" | 378 | expected = "\AFormat: 1.7\n.*foo_1.0-1.diff.gz\Z" |
237 | 375 | self.assertTextMatchesExpressionIgnoreWhitespace( | 379 | self.assertTextMatchesExpressionIgnoreWhitespace( |
238 | 376 | expected, | 380 | expected, |
240 | 377 | parsed.parsed_content) | 381 | changesfile.parsed_content) |
241 | 378 | 382 | ||
242 | 379 | def test_no_signature_rejected(self): | 383 | def test_no_signature_rejected(self): |
243 | 380 | # An unsigned changes file is rejected. | 384 | # An unsigned changes file is rejected. |
244 | 381 | path = datadir('signatures/unsigned.changes') | 385 | path = datadir('signatures/unsigned.changes') |
248 | 382 | self.assertRaises( | 386 | changesfile = ChangesFile(path, InsecureUploadPolicy(), BufferLogger()) |
249 | 383 | UploadError, | 387 | errors = list(changesfile.parseChanges()) |
250 | 384 | ChangesFile, path, InsecureUploadPolicy(), BufferLogger()) | 388 | self.assertIsInstance(errors[0], UploadError) |
251 | 389 | self.assertEqual(1, len(errors)) | ||
252 | 385 | 390 | ||
253 | 386 | def test_prefix_ignored(self): | 391 | def test_prefix_ignored(self): |
254 | 387 | # A signed changes file with an unsigned prefix has only the | 392 | # A signed changes file with an unsigned prefix has only the |
255 | 388 | # signed part parsed. | 393 | # signed part parsed. |
256 | 389 | path = datadir('signatures/prefixed.changes') | 394 | path = datadir('signatures/prefixed.changes') |
258 | 390 | parsed = ChangesFile(path, InsecureUploadPolicy(), BufferLogger()) | 395 | changesfile = ChangesFile(path, InsecureUploadPolicy(), BufferLogger()) |
259 | 396 | self.assertEqual([], list(changesfile.parseChanges())) | ||
260 | 391 | self.assertEqual( | 397 | self.assertEqual( |
261 | 392 | getUtility(IPersonSet).getByEmail('foo.bar@canonical.com'), | 398 | getUtility(IPersonSet).getByEmail('foo.bar@canonical.com'), |
263 | 393 | parsed.signer) | 399 | changesfile.signer) |
264 | 394 | expected = "\AFormat: 1.7\n.*foo_1.0-1.diff.gz\Z" | 400 | expected = "\AFormat: 1.7\n.*foo_1.0-1.diff.gz\Z" |
265 | 395 | self.assertTextMatchesExpressionIgnoreWhitespace( | 401 | self.assertTextMatchesExpressionIgnoreWhitespace( |
266 | 396 | expected, | 402 | expected, |
270 | 397 | parsed.parsed_content) | 403 | changesfile.parsed_content) |
271 | 398 | self.assertEqual("breezy", parsed.suite_name) | 404 | self.assertEqual("breezy", changesfile.suite_name) |
272 | 399 | self.assertNotIn("evil", parsed.changes_comment) | 405 | self.assertNotIn("evil", changesfile.changes_comment) |
273 | 400 | 406 | ||
274 | === modified file 'lib/lp/archiveuploader/tests/test_nascentuploadfile.py' | |||
275 | --- lib/lp/archiveuploader/tests/test_nascentuploadfile.py 2016-11-08 20:22:59 +0000 | |||
276 | +++ lib/lp/archiveuploader/tests/test_nascentuploadfile.py 2017-09-17 15:23:26 +0000 | |||
277 | @@ -208,7 +208,9 @@ | |||
278 | 208 | path = os.path.join(tempdir, filename) | 208 | path = os.path.join(tempdir, filename) |
279 | 209 | with open(path, "w") as changes_fd: | 209 | with open(path, "w") as changes_fd: |
280 | 210 | changes.dump(changes_fd) | 210 | changes.dump(changes_fd) |
282 | 211 | return ChangesFile(path, self.policy, self.logger) | 211 | changesfile = ChangesFile(path, self.policy, self.logger) |
283 | 212 | self.assertEqual([], list(changesfile.parseChanges())) | ||
284 | 213 | return changesfile | ||
285 | 212 | 214 | ||
286 | 213 | 215 | ||
287 | 214 | class DSCFileTests(PackageUploadFileTestCase): | 216 | class DSCFileTests(PackageUploadFileTestCase): |
288 | 215 | 217 | ||
289 | === modified file 'lib/lp/archiveuploader/tests/test_sync_notification.py' | |||
290 | --- lib/lp/archiveuploader/tests/test_sync_notification.py 2015-08-26 13:41:21 +0000 | |||
291 | +++ lib/lp/archiveuploader/tests/test_sync_notification.py 2017-09-17 15:23:26 +0000 | |||
292 | @@ -1,4 +1,4 @@ | |||
294 | 1 | # Copyright 2012-2015 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012-2016 Canonical Ltd. This software is licensed under the |
295 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
296 | 3 | 3 | ||
297 | 4 | """Test notification behaviour for cross-distro package syncs.""" | 4 | """Test notification behaviour for cross-distro package syncs.""" |
298 | @@ -55,6 +55,7 @@ | |||
299 | 55 | self.raw_content = open(file_path).read() | 55 | self.raw_content = open(file_path).read() |
300 | 56 | self.signingkey = None | 56 | self.signingkey = None |
301 | 57 | 57 | ||
302 | 58 | parseChanges = FakeMethod([]) | ||
303 | 58 | checkFileName = FakeMethod([]) | 59 | checkFileName = FakeMethod([]) |
304 | 59 | processAddresses = FakeMethod([]) | 60 | processAddresses = FakeMethod([]) |
305 | 60 | processFiles = FakeMethod([]) | 61 | processFiles = FakeMethod([]) |
306 | 61 | 62 | ||
307 | === modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py' | |||
308 | --- lib/lp/archiveuploader/tests/test_uploadprocessor.py 2017-04-21 15:21:27 +0000 | |||
309 | +++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2017-09-17 15:23:26 +0000 | |||
310 | @@ -51,6 +51,7 @@ | |||
311 | 51 | IBuildFarmJobBehaviour, | 51 | IBuildFarmJobBehaviour, |
312 | 52 | ) | 52 | ) |
313 | 53 | from lp.registry.interfaces.distribution import IDistributionSet | 53 | from lp.registry.interfaces.distribution import IDistributionSet |
314 | 54 | from lp.registry.interfaces.gpg import IGPGKeySet | ||
315 | 54 | from lp.registry.interfaces.person import IPersonSet | 55 | from lp.registry.interfaces.person import IPersonSet |
316 | 55 | from lp.registry.interfaces.pocket import PackagePublishingPocket | 56 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
317 | 56 | from lp.registry.interfaces.series import SeriesStatus | 57 | from lp.registry.interfaces.series import SeriesStatus |
318 | @@ -240,7 +241,7 @@ | |||
319 | 240 | excName = excClass.__name__ | 241 | excName = excClass.__name__ |
320 | 241 | else: | 242 | else: |
321 | 242 | excName = str(excClass) | 243 | excName = str(excClass) |
323 | 243 | raise self.failureException, "%s not raised" % excName | 244 | raise self.failureException("%s not raised" % excName) |
324 | 244 | 245 | ||
325 | 245 | def setupBreezy(self, name="breezy", permitted_formats=None): | 246 | def setupBreezy(self, name="breezy", permitted_formats=None): |
326 | 246 | """Create a fresh distroseries in ubuntu. | 247 | """Create a fresh distroseries in ubuntu. |
327 | @@ -1955,10 +1956,47 @@ | |||
328 | 1955 | 1956 | ||
329 | 1956 | self.assertEqual(UploadStatusEnum.REJECTED, result) | 1957 | self.assertEqual(UploadStatusEnum.REJECTED, result) |
330 | 1957 | self.assertLogContains( | 1958 | self.assertLogContains( |
332 | 1958 | "INFO Failed to parse changes file") | 1959 | "INFO Not sending rejection notice without a signing key.") |
333 | 1959 | self.assertEmailQueueLength(0) | 1960 | self.assertEmailQueueLength(0) |
334 | 1960 | self.assertEqual([], self.oopses) | 1961 | self.assertEqual([], self.oopses) |
335 | 1961 | 1962 | ||
336 | 1963 | def test_deactivated_key_upload_sends_mail(self): | ||
337 | 1964 | # An upload signed with a deactivated key does not OOPS and sends a | ||
338 | 1965 | # rejection email. | ||
339 | 1966 | self.switchToAdmin() | ||
340 | 1967 | fingerprint = "340CA3BB270E2716C9EE0B768E7EB7086C64A8C5" | ||
341 | 1968 | gpgkeyset = getUtility(IGPGKeySet) | ||
342 | 1969 | gpgkeyset.deactivate(gpgkeyset.getByFingerprint(fingerprint)) | ||
343 | 1970 | self.switchToUploader() | ||
344 | 1971 | |||
345 | 1972 | uploadprocessor = self.setupBreezyAndGetUploadProcessor() | ||
346 | 1973 | upload_dir = self.queueUpload("netapplet_1.0-1-signed") | ||
347 | 1974 | |||
348 | 1975 | [result] = self.processUpload(uploadprocessor, upload_dir) | ||
349 | 1976 | |||
350 | 1977 | self.assertEqual(UploadStatusEnum.REJECTED, result) | ||
351 | 1978 | base_contents = [ | ||
352 | 1979 | "Subject: [ubuntu] netapplet_1.0-1_source.changes (Rejected)", | ||
353 | 1980 | "File %s/netapplet_1.0-1-signed/netapplet_1.0-1_source.changes " | ||
354 | 1981 | "is signed with a deactivated key %s" % ( | ||
355 | 1982 | self.incoming_folder, fingerprint), | ||
356 | 1983 | ] | ||
357 | 1984 | expected = [] | ||
358 | 1985 | expected.append({ | ||
359 | 1986 | "contents": base_contents + [ | ||
360 | 1987 | "You are receiving this email because you are the most " | ||
361 | 1988 | "recent person", | ||
362 | 1989 | "listed in this package's changelog."], | ||
363 | 1990 | "recipient": "daniel.silverstone@canonical.com", | ||
364 | 1991 | }) | ||
365 | 1992 | expected.append({ | ||
366 | 1993 | "contents": base_contents + [ | ||
367 | 1994 | "You are receiving this email because you made this upload."], | ||
368 | 1995 | "recipient": "foo.bar@canonical.com", | ||
369 | 1996 | }) | ||
370 | 1997 | self.assertEmails(expected) | ||
371 | 1998 | self.assertEqual([], self.oopses) | ||
372 | 1999 | |||
373 | 1962 | def test_ddeb_upload_overrides(self): | 2000 | def test_ddeb_upload_overrides(self): |
374 | 1963 | # DDEBs should always be overridden to the same values as their | 2001 | # DDEBs should always be overridden to the same values as their |
375 | 1964 | # counterpart DEB's. | 2002 | # counterpart DEB's. |
376 | 1965 | 2003 | ||
377 | === modified file 'lib/lp/archiveuploader/uploadprocessor.py' | |||
378 | --- lib/lp/archiveuploader/uploadprocessor.py 2017-01-10 15:18:48 +0000 | |||
379 | +++ lib/lp/archiveuploader/uploadprocessor.py 2017-09-17 15:23:26 +0000 | |||
380 | @@ -338,22 +338,8 @@ | |||
381 | 338 | # The path we want for NascentUpload is the path to the folder | 338 | # The path we want for NascentUpload is the path to the folder |
382 | 339 | # containing the changes file (and the other files referenced by it). | 339 | # containing the changes file (and the other files referenced by it). |
383 | 340 | changesfile_path = os.path.join(self.upload_path, changes_file) | 340 | changesfile_path = os.path.join(self.upload_path, changes_file) |
400 | 341 | try: | 341 | upload = NascentUpload.from_changesfile_path( |
401 | 342 | upload = NascentUpload.from_changesfile_path( | 342 | changesfile_path, policy, self.processor.log) |
386 | 343 | changesfile_path, policy, self.processor.log) | ||
387 | 344 | except UploadError as e: | ||
388 | 345 | # We failed to parse the changes file, so we have no key or | ||
389 | 346 | # Changed-By to notify of the rejection. Just log it and | ||
390 | 347 | # move on. | ||
391 | 348 | # XXX wgrant 2011-09-29 bug=499438: With some refactoring we | ||
392 | 349 | # could do better here: if we have a signature then we have | ||
393 | 350 | # somebody to email, even if the rest of the file is | ||
394 | 351 | # corrupt. | ||
395 | 352 | logger.info( | ||
396 | 353 | "Failed to parse changes file '%s': %s" % ( | ||
397 | 354 | os.path.join(self.upload_path, changes_file), | ||
398 | 355 | str(e))) | ||
399 | 356 | return UploadStatusEnum.REJECTED | ||
402 | 357 | 343 | ||
403 | 358 | # Reject source upload to buildd upload paths. | 344 | # Reject source upload to buildd upload paths. |
404 | 359 | first_path = relative_path.split(os.path.sep)[0] | 345 | first_path = relative_path.split(os.path.sep)[0] |
405 | @@ -411,7 +397,16 @@ | |||
406 | 411 | notify = False | 397 | notify = False |
407 | 412 | if upload.is_rejected: | 398 | if upload.is_rejected: |
408 | 413 | result = UploadStatusEnum.REJECTED | 399 | result = UploadStatusEnum.REJECTED |
410 | 414 | upload.do_reject(notify) | 400 | if upload.changes.parsed_content is not None: |
411 | 401 | # We got past the point of checking any required | ||
412 | 402 | # signature, so we can do a proper rejection. | ||
413 | 403 | upload.do_reject(notify) | ||
414 | 404 | else: | ||
415 | 405 | # The upload required a signature and either didn't have | ||
416 | 406 | # one or we failed to verify it, so we have nobody to | ||
417 | 407 | # notify. Just log it and move on. | ||
418 | 408 | logger.info( | ||
419 | 409 | "Not sending rejection notice without a signing key.") | ||
420 | 415 | self.processor.ztm.abort() | 410 | self.processor.ztm.abort() |
421 | 416 | else: | 411 | else: |
422 | 417 | successful = self._acceptUpload(upload, notify) | 412 | successful = self._acceptUpload(upload, notify) |
423 | 418 | 413 | ||
424 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue-translations.txt' | |||
425 | --- lib/lp/soyuz/doc/distroseriesqueue-translations.txt 2015-09-04 12:19:07 +0000 | |||
426 | +++ lib/lp/soyuz/doc/distroseriesqueue-translations.txt 2017-09-17 15:23:26 +0000 | |||
427 | @@ -87,10 +87,9 @@ | |||
428 | 87 | >>> pmount_upload = NascentUpload.from_changesfile_path( | 87 | >>> pmount_upload = NascentUpload.from_changesfile_path( |
429 | 88 | ... datadir('pmount_0.9.7-2ubuntu2_amd64.changes'), | 88 | ... datadir('pmount_0.9.7-2ubuntu2_amd64.changes'), |
430 | 89 | ... buildd_policy, FakeLogger()) | 89 | ... buildd_policy, FakeLogger()) |
431 | 90 | DEBUG pmount_0.9.7-2ubuntu2_amd64.changes can be unsigned. | ||
432 | 91 | |||
433 | 92 | >>> pmount_upload.process(build=build) | 90 | >>> pmount_upload.process(build=build) |
434 | 93 | DEBUG Beginning processing. | 91 | DEBUG Beginning processing. |
435 | 92 | DEBUG pmount_0.9.7-2ubuntu2_amd64.changes can be unsigned. | ||
436 | 94 | DEBUG Verifying the changes file. | 93 | DEBUG Verifying the changes file. |
437 | 95 | DEBUG Verifying files in upload. | 94 | DEBUG Verifying files in upload. |
438 | 96 | DEBUG Verifying binary pmount_0.9.7-2ubuntu2_amd64.deb | 95 | DEBUG Verifying binary pmount_0.9.7-2ubuntu2_amd64.deb |