Merge lp:~cjwatson/launchpad/cveimport-requests into lp:launchpad
- cveimport-requests
- Merge into devel
Proposed by
Colin Watson
Status: | Merged |
---|---|
Merged at revision: | 18682 |
Proposed branch: | lp:~cjwatson/launchpad/cveimport-requests |
Merge into: | lp:launchpad |
Prerequisite: | lp:~cjwatson/launchpad/bugs-remote-finders-requests |
Diff against target: |
234 lines (+83/-19) 6 files modified
constraints.txt (+1/-0) lib/lp/bugs/scripts/cveimport.py (+17/-5) lib/lp/services/config/schema-lazr.conf (+3/-0) lib/lp/services/tests/test_timeout.py (+33/-2) lib/lp/services/timeout.py (+28/-12) setup.py (+1/-0) |
To merge this branch: | bzr merge lp:~cjwatson/launchpad/cveimport-requests |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+347201@code.launchpad.net |
Commit message
Convert update-cve to urlfetch with explicit proxy configuration.
Description of the change
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Revision history for this message
Colin Watson (cjwatson) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'constraints.txt' | |||
2 | --- constraints.txt 2018-05-31 10:23:03 +0000 | |||
3 | +++ constraints.txt 2018-06-05 01:51:40 +0000 | |||
4 | @@ -336,6 +336,7 @@ | |||
5 | 336 | PyYAML==3.10 | 336 | PyYAML==3.10 |
6 | 337 | rabbitfixture==0.3.6 | 337 | rabbitfixture==0.3.6 |
7 | 338 | requests==2.7.0 | 338 | requests==2.7.0 |
8 | 339 | requests-file==1.4.3 | ||
9 | 339 | requests-toolbelt==0.6.2 | 340 | requests-toolbelt==0.6.2 |
10 | 340 | responses==0.9.0 | 341 | responses==0.9.0 |
11 | 341 | scandir==1.7 | 342 | scandir==1.7 |
12 | 342 | 343 | ||
13 | === modified file 'lib/lp/bugs/scripts/cveimport.py' | |||
14 | --- lib/lp/bugs/scripts/cveimport.py 2016-08-16 15:33:02 +0000 | |||
15 | +++ lib/lp/bugs/scripts/cveimport.py 2018-06-05 01:51:40 +0000 | |||
16 | @@ -1,4 +1,4 @@ | |||
18 | 1 | # Copyright 2009 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2009-2018 Canonical Ltd. This software is licensed under the |
19 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
20 | 3 | 3 | ||
21 | 4 | """A set of functions related to the ability to parse the XML CVE database, | 4 | """A set of functions related to the ability to parse the XML CVE database, |
22 | @@ -10,9 +10,9 @@ | |||
23 | 10 | import gzip | 10 | import gzip |
24 | 11 | import StringIO | 11 | import StringIO |
25 | 12 | import time | 12 | import time |
26 | 13 | import urllib2 | ||
27 | 14 | import xml.etree.cElementTree as cElementTree | 13 | import xml.etree.cElementTree as cElementTree |
28 | 15 | 14 | ||
29 | 15 | import requests | ||
30 | 16 | from zope.component import getUtility | 16 | from zope.component import getUtility |
31 | 17 | from zope.event import notify | 17 | from zope.event import notify |
32 | 18 | from zope.interface import implementer | 18 | from zope.interface import implementer |
33 | @@ -31,6 +31,10 @@ | |||
34 | 31 | LaunchpadCronScript, | 31 | LaunchpadCronScript, |
35 | 32 | LaunchpadScriptFailure, | 32 | LaunchpadScriptFailure, |
36 | 33 | ) | 33 | ) |
37 | 34 | from lp.services.timeout import ( | ||
38 | 35 | override_timeout, | ||
39 | 36 | urlfetch, | ||
40 | 37 | ) | ||
41 | 34 | 38 | ||
42 | 35 | 39 | ||
43 | 36 | CVEDB_NS = '{http://cve.mitre.org/cve/downloads/1.0}' | 40 | CVEDB_NS = '{http://cve.mitre.org/cve/downloads/1.0}' |
44 | @@ -209,14 +213,22 @@ | |||
45 | 209 | elif self.options.cveurl is not None: | 213 | elif self.options.cveurl is not None: |
46 | 210 | self.logger.info("Downloading CVE database from %s..." % | 214 | self.logger.info("Downloading CVE database from %s..." % |
47 | 211 | self.options.cveurl) | 215 | self.options.cveurl) |
48 | 216 | proxies = {} | ||
49 | 217 | if config.launchpad.http_proxy: | ||
50 | 218 | proxies['http'] = config.launchpad.http_proxy | ||
51 | 219 | proxies['https'] = config.launchpad.http_proxy | ||
52 | 212 | try: | 220 | try: |
55 | 213 | url = urllib2.urlopen(self.options.cveurl) | 221 | with override_timeout(config.cveupdater.timeout): |
56 | 214 | except (urllib2.HTTPError, urllib2.URLError): | 222 | # Command-line options are trusted, so allow file:// |
57 | 223 | # URLs to ease testing. | ||
58 | 224 | response = urlfetch( | ||
59 | 225 | self.options.cveurl, proxies=proxies, allow_file=True) | ||
60 | 226 | except requests.RequestException: | ||
61 | 215 | raise LaunchpadScriptFailure( | 227 | raise LaunchpadScriptFailure( |
62 | 216 | 'Unable to connect for CVE database %s' | 228 | 'Unable to connect for CVE database %s' |
63 | 217 | % self.options.cveurl) | 229 | % self.options.cveurl) |
64 | 218 | 230 | ||
66 | 219 | cve_db_gz = url.read() | 231 | cve_db_gz = response.content |
67 | 220 | self.logger.info("%d bytes downloaded." % len(cve_db_gz)) | 232 | self.logger.info("%d bytes downloaded." % len(cve_db_gz)) |
68 | 221 | cve_db = gzip.GzipFile( | 233 | cve_db = gzip.GzipFile( |
69 | 222 | fileobj=StringIO.StringIO(cve_db_gz)).read() | 234 | fileobj=StringIO.StringIO(cve_db_gz)).read() |
70 | 223 | 235 | ||
71 | === modified file 'lib/lp/services/config/schema-lazr.conf' | |||
72 | --- lib/lp/services/config/schema-lazr.conf 2018-05-31 12:47:31 +0000 | |||
73 | +++ lib/lp/services/config/schema-lazr.conf 2018-06-05 01:51:40 +0000 | |||
74 | @@ -490,6 +490,9 @@ | |||
75 | 490 | # datatype: string | 490 | # datatype: string |
76 | 491 | cve_db_url: https://cve.mitre.org/data/downloads/allitems.xml.gz | 491 | cve_db_url: https://cve.mitre.org/data/downloads/allitems.xml.gz |
77 | 492 | 492 | ||
78 | 493 | # datatype: integer | ||
79 | 494 | timeout: 30 | ||
80 | 495 | |||
81 | 493 | 496 | ||
82 | 494 | [database] | 497 | [database] |
83 | 495 | # Connection strings, format as per the PQconnectdb connection string as | 498 | # Connection strings, format as per the PQconnectdb connection string as |
84 | 496 | 499 | ||
85 | === modified file 'lib/lp/services/tests/test_timeout.py' | |||
86 | --- lib/lp/services/tests/test_timeout.py 2018-06-05 01:31:47 +0000 | |||
87 | +++ lib/lp/services/tests/test_timeout.py 2018-06-05 01:51:40 +0000 | |||
88 | @@ -15,14 +15,22 @@ | |||
89 | 15 | import threading | 15 | import threading |
90 | 16 | import xmlrpclib | 16 | import xmlrpclib |
91 | 17 | 17 | ||
93 | 18 | from fixtures import MonkeyPatch | 18 | from fixtures import ( |
94 | 19 | MonkeyPatch, | ||
95 | 20 | TempDir, | ||
96 | 21 | ) | ||
97 | 19 | from requests import Response | 22 | from requests import Response |
98 | 20 | from requests.exceptions import ( | 23 | from requests.exceptions import ( |
99 | 21 | ConnectionError, | 24 | ConnectionError, |
100 | 22 | InvalidSchema, | 25 | InvalidSchema, |
101 | 23 | ) | 26 | ) |
103 | 24 | from testtools.matchers import MatchesStructure | 27 | from testtools.matchers import ( |
104 | 28 | ContainsDict, | ||
105 | 29 | Equals, | ||
106 | 30 | MatchesStructure, | ||
107 | 31 | ) | ||
108 | 25 | 32 | ||
109 | 33 | from lp.services.osutils import write_file | ||
110 | 26 | from lp.services.timeout import ( | 34 | from lp.services.timeout import ( |
111 | 27 | default_timeout, | 35 | default_timeout, |
112 | 28 | get_default_timeout_function, | 36 | get_default_timeout_function, |
113 | @@ -368,6 +376,17 @@ | |||
114 | 368 | self.assertEqual( | 376 | self.assertEqual( |
115 | 369 | "No connection adapters were found for '%s'" % url, str(e)) | 377 | "No connection adapters were found for '%s'" % url, str(e)) |
116 | 370 | 378 | ||
117 | 379 | def test_urlfetch_does_not_support_file_urls_by_default(self): | ||
118 | 380 | """urlfetch() does not support file urls by default.""" | ||
119 | 381 | set_default_timeout_function(lambda: 1) | ||
120 | 382 | self.addCleanup(set_default_timeout_function, None) | ||
121 | 383 | test_path = self.useFixture(TempDir()).join('file') | ||
122 | 384 | write_file(test_path, '') | ||
123 | 385 | url = 'file://' + test_path | ||
124 | 386 | e = self.assertRaises(InvalidSchema, urlfetch, url) | ||
125 | 387 | self.assertEqual( | ||
126 | 388 | "No connection adapters were found for '%s'" % url, str(e)) | ||
127 | 389 | |||
128 | 371 | def test_urlfetch_no_proxy_by_default(self): | 390 | def test_urlfetch_no_proxy_by_default(self): |
129 | 372 | """urlfetch does not use a proxy by default.""" | 391 | """urlfetch does not use a proxy by default.""" |
130 | 373 | self.pushConfig('launchpad', http_proxy='http://proxy.example:3128/') | 392 | self.pushConfig('launchpad', http_proxy='http://proxy.example:3128/') |
131 | @@ -399,6 +418,18 @@ | |||
132 | 399 | {scheme: proxy for scheme in ('http', 'https')}, | 418 | {scheme: proxy for scheme in ('http', 'https')}, |
133 | 400 | fake_send.calls[0][1]['proxies']) | 419 | fake_send.calls[0][1]['proxies']) |
134 | 401 | 420 | ||
135 | 421 | def test_urlfetch_supports_file_urls_if_allow_file(self): | ||
136 | 422 | """urlfetch() supports file urls if explicitly asked to do so.""" | ||
137 | 423 | set_default_timeout_function(lambda: 1) | ||
138 | 424 | self.addCleanup(set_default_timeout_function, None) | ||
139 | 425 | test_path = self.useFixture(TempDir()).join('file') | ||
140 | 426 | write_file(test_path, 'Success.') | ||
141 | 427 | url = 'file://' + test_path | ||
142 | 428 | self.assertThat(urlfetch(url, allow_file=True), MatchesStructure( | ||
143 | 429 | status_code=Equals(200), | ||
144 | 430 | headers=ContainsDict({'Content-Length': Equals(8)}), | ||
145 | 431 | content=Equals('Success.'))) | ||
146 | 432 | |||
147 | 402 | def test_xmlrpc_transport(self): | 433 | def test_xmlrpc_transport(self): |
148 | 403 | """ Another use case for timeouts is communicating with external | 434 | """ Another use case for timeouts is communicating with external |
149 | 404 | systems using XMLRPC. In order to allow timeouts using XMLRPC we | 435 | systems using XMLRPC. In order to allow timeouts using XMLRPC we |
150 | 405 | 436 | ||
151 | === modified file 'lib/lp/services/timeout.py' | |||
152 | --- lib/lp/services/timeout.py 2018-06-05 01:31:47 +0000 | |||
153 | +++ lib/lp/services/timeout.py 2018-06-05 01:51:40 +0000 | |||
154 | @@ -40,6 +40,7 @@ | |||
155 | 40 | ) | 40 | ) |
156 | 41 | from requests.packages.urllib3.exceptions import ClosedPoolError | 41 | from requests.packages.urllib3.exceptions import ClosedPoolError |
157 | 42 | from requests.packages.urllib3.poolmanager import PoolManager | 42 | from requests.packages.urllib3.poolmanager import PoolManager |
158 | 43 | from requests_file import FileAdapter | ||
159 | 43 | from six import reraise | 44 | from six import reraise |
160 | 44 | 45 | ||
161 | 45 | from lp.services.config import config | 46 | from lp.services.config import config |
162 | @@ -318,25 +319,38 @@ | |||
163 | 318 | class URLFetcher: | 319 | class URLFetcher: |
164 | 319 | """Object fetching remote URLs with a time out.""" | 320 | """Object fetching remote URLs with a time out.""" |
165 | 320 | 321 | ||
169 | 321 | @staticmethod | 322 | def __init__(self): |
170 | 322 | def _makeSession(trust_env=None): | 323 | self.session = None |
171 | 323 | session = Session() | 324 | |
172 | 325 | @with_timeout(cleanup='cleanup') | ||
173 | 326 | def fetch(self, url, trust_env=None, use_proxy=False, allow_file=False, | ||
174 | 327 | **request_kwargs): | ||
175 | 328 | """Fetch the URL using a custom HTTP handler supporting timeout. | ||
176 | 329 | |||
177 | 330 | :param url: The URL to fetch. | ||
178 | 331 | :param trust_env: If not None, set the session's trust_env to this | ||
179 | 332 | to determine whether it fetches proxy configuration from the | ||
180 | 333 | environment. | ||
181 | 334 | :param use_proxy: If True, use Launchpad's configured proxy. | ||
182 | 335 | :param allow_file: If True, allow file:// URLs. (Be careful to only | ||
183 | 336 | pass this if the URL is trusted.) | ||
184 | 337 | :param request_kwargs: Additional keyword arguments passed on to | ||
185 | 338 | `Session.request`. | ||
186 | 339 | """ | ||
187 | 340 | self.session = Session() | ||
188 | 324 | if trust_env is not None: | 341 | if trust_env is not None: |
190 | 325 | session.trust_env = trust_env | 342 | self.session.trust_env = trust_env |
191 | 326 | # Mount our custom adapters. | 343 | # Mount our custom adapters. |
195 | 327 | session.mount("https://", CleanableHTTPAdapter()) | 344 | self.session.mount("https://", CleanableHTTPAdapter()) |
196 | 328 | session.mount("http://", CleanableHTTPAdapter()) | 345 | self.session.mount("http://", CleanableHTTPAdapter()) |
197 | 329 | return session | 346 | if allow_file: |
198 | 347 | self.session.mount("file://", FileAdapter()) | ||
199 | 330 | 348 | ||
200 | 331 | @with_timeout(cleanup='cleanup') | ||
201 | 332 | def fetch(self, url, trust_env=None, use_proxy=False, **request_kwargs): | ||
202 | 333 | """Fetch the URL using a custom HTTP handler supporting timeout.""" | ||
203 | 334 | request_kwargs.setdefault("method", "GET") | 349 | request_kwargs.setdefault("method", "GET") |
204 | 335 | if use_proxy and config.launchpad.http_proxy: | 350 | if use_proxy and config.launchpad.http_proxy: |
205 | 336 | request_kwargs.setdefault("proxies", {}) | 351 | request_kwargs.setdefault("proxies", {}) |
206 | 337 | request_kwargs["proxies"]["http"] = config.launchpad.http_proxy | 352 | request_kwargs["proxies"]["http"] = config.launchpad.http_proxy |
207 | 338 | request_kwargs["proxies"]["https"] = config.launchpad.http_proxy | 353 | request_kwargs["proxies"]["https"] = config.launchpad.http_proxy |
208 | 339 | self.session = self._makeSession(trust_env=trust_env) | ||
209 | 340 | response = self.session.request(url=url, **request_kwargs) | 354 | response = self.session.request(url=url, **request_kwargs) |
210 | 341 | response.raise_for_status() | 355 | response.raise_for_status() |
211 | 342 | # Make sure the content has been consumed before returning. | 356 | # Make sure the content has been consumed before returning. |
212 | @@ -345,7 +359,9 @@ | |||
213 | 345 | 359 | ||
214 | 346 | def cleanup(self): | 360 | def cleanup(self): |
215 | 347 | """Reset the connection when the operation timed out.""" | 361 | """Reset the connection when the operation timed out.""" |
217 | 348 | self.session.close() | 362 | if self.session is not None: |
218 | 363 | self.session.close() | ||
219 | 364 | self.session = None | ||
220 | 349 | 365 | ||
221 | 350 | 366 | ||
222 | 351 | def urlfetch(url, trust_env=None, **request_kwargs): | 367 | def urlfetch(url, trust_env=None, **request_kwargs): |
223 | 352 | 368 | ||
224 | === modified file 'setup.py' | |||
225 | --- setup.py 2018-05-31 10:23:03 +0000 | |||
226 | +++ setup.py 2018-06-05 01:51:40 +0000 | |||
227 | @@ -207,6 +207,7 @@ | |||
228 | 207 | 'PyYAML', | 207 | 'PyYAML', |
229 | 208 | 'rabbitfixture', | 208 | 'rabbitfixture', |
230 | 209 | 'requests', | 209 | 'requests', |
231 | 210 | 'requests-file', | ||
232 | 210 | 'requests-toolbelt', | 211 | 'requests-toolbelt', |
233 | 211 | 'responses', | 212 | 'responses', |
234 | 212 | 'scandir', | 213 | 'scandir', |